Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

[PATCH] clean dup2() up a bit

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Al Viro 1b7e190b 1027abe8

+27 -26
+27 -26
fs/fcntl.c
··· 63 63 return -EINVAL; 64 64 65 65 spin_lock(&files->file_lock); 66 - if (!(file = fcheck(oldfd))) 67 - goto out_unlock; 68 - get_file(file); /* We are now finished with oldfd */ 69 - 70 66 err = expand_files(files, newfd); 67 + file = fcheck(oldfd); 68 + if (unlikely(!file)) 69 + goto Ebadf; 71 70 if (unlikely(err < 0)) { 72 71 if (err == -EMFILE) 73 - err = -EBADF; 74 - goto out_fput; 72 + goto Ebadf; 73 + goto out_unlock; 75 74 } 76 - 77 - /* To avoid races with open() and dup(), we will mark the fd as 78 - * in-use in the open-file bitmap throughout the entire dup2() 79 - * process. This is quite safe: do_close() uses the fd array 80 - * entry, not the bitmap, to decide what work needs to be 81 - * done. --sct */ 82 - /* Doesn't work. open() might be there first. --AV */ 83 - 84 - /* Yes. It's a race. In user space. Nothing sane to do */ 75 + /* 76 + * We need to detect attempts to do dup2() over allocated but still 77 + * not finished descriptor. NB: OpenBSD avoids that at the price of 78 + * extra work in their equivalent of fget() - they insert struct 79 + * file immediately after grabbing descriptor, mark it larval if 80 + * more work (e.g. actual opening) is needed and make sure that 81 + * fget() treats larval files as absent. Potentially interesting, 82 + * but while extra work in fget() is trivial, locking implications 83 + * and amount of surgery on open()-related paths in VFS are not. 84 + * FreeBSD fails with -EBADF in the same situation, NetBSD "solution" 85 + * deadlocks in rather amusing ways, AFAICS. All of that is out of 86 + * scope of POSIX or SUS, since neither considers shared descriptor 87 + * tables and this condition does not arise without those. 88 + */ 85 89 err = -EBUSY; 86 90 fdt = files_fdtable(files); 87 91 tofree = fdt->fd[newfd]; 88 92 if (!tofree && FD_ISSET(newfd, fdt->open_fds)) 89 - goto out_fput; 90 - 93 + goto out_unlock; 94 + get_file(file); 91 95 rcu_assign_pointer(fdt->fd[newfd], file); 92 96 FD_SET(newfd, fdt->open_fds); 93 97 if (flags & O_CLOEXEC) ··· 102 98 103 99 if (tofree) 104 100 filp_close(tofree, files); 105 - err = newfd; 106 - out: 107 - return err; 101 + 102 + return newfd; 103 + 104 + Ebadf: 105 + err = -EBADF; 108 106 out_unlock: 109 107 spin_unlock(&files->file_lock); 110 - goto out; 111 - 112 - out_fput: 113 - spin_unlock(&files->file_lock); 114 - fput(file); 115 - goto out; 108 + return err; 116 109 } 117 110 118 111 asmlinkage long sys_dup2(unsigned int oldfd, unsigned int newfd)