* dup2 return value mismatch @ 2012-10-31 2:18 Linliangjie 2012-10-31 3:00 ` Al Viro 0 siblings, 1 reply; 5+ messages in thread From: Linliangjie @ 2012-10-31 2:18 UTC (permalink / raw) To: linux-fsdevel@vger.kernel.org Cc: viro@zeniv.linux.org.uk, Sanil kumar, shyju pv, Nagamani Mantha, Maxiansheng (Max) Test: negative test for dup2 call with maxfd It has been noticed that new changes return directly -EMFILE when the newfd is larger than maxfd in 3.7-rc3 instead of EBADF in 3.6-rc6 file changed : /fs/fcntl.c Kernel Commit id of the change : f33ff9927f42045116d738ee47ff7bc59f739bd7 More details: "https://bugzilla.kernel.org/show_bug.cgi?id=49791" IS this an issue or is there a man page update required? Jack ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: dup2 return value mismatch 2012-10-31 2:18 dup2 return value mismatch Linliangjie @ 2012-10-31 3:00 ` Al Viro 2012-10-31 3:14 ` Linus Torvalds 0 siblings, 1 reply; 5+ messages in thread From: Al Viro @ 2012-10-31 3:00 UTC (permalink / raw) To: Linus Torvalds Cc: linux-fsdevel@vger.kernel.org, Sanil kumar, shyju pv, Nagamani Mantha, Maxiansheng (Max) On Wed, Oct 31, 2012 at 02:18:12AM +0000, Linliangjie wrote: > > Test: > negative test for dup2 call with maxfd > > > It has been noticed that new changes return directly -EMFILE when the newfd is larger than maxfd in 3.7-rc3 instead of EBADF in 3.6-rc6 > file changed : /fs/fcntl.c > Kernel Commit id of the change : f33ff9927f42045116d738ee47ff7bc59f739bd7 > > > More details: "https://bugzilla.kernel.org/show_bug.cgi?id=49791" > > > IS this an issue or is there a man page update required? Umm... After looking at what POSIX actually says... There is an issue, all right. Thanks for spotting. Fix follows: Return the right error value when dup2() or dup3() newfd argument is too large Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- diff --git a/fs/file.c b/fs/file.c index ec20de9..ddf57c5 100644 --- a/fs/file.c +++ b/fs/file.c @@ -894,7 +894,7 @@ int replace_fd(unsigned fd, struct file *file, unsigned flags) return __close_fd(files, fd); if (fd >= rlimit(RLIMIT_NOFILE)) - return -EMFILE; + return -EBADF; spin_lock(&files->file_lock); err = expand_files(files, fd); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: dup2 return value mismatch 2012-10-31 3:00 ` Al Viro @ 2012-10-31 3:14 ` Linus Torvalds 2012-10-31 3:37 ` Al Viro 0 siblings, 1 reply; 5+ messages in thread From: Linus Torvalds @ 2012-10-31 3:14 UTC (permalink / raw) To: Al Viro Cc: linux-fsdevel@vger.kernel.org, Sanil kumar, shyju pv, Nagamani Mantha, Maxiansheng (Max) On Tue, Oct 30, 2012 at 8:00 PM, Al Viro <viro@zeniv.linux.org.uk> wrote: > > Umm... After looking at what POSIX actually says... There is an issue, > all right. Thanks for spotting. Fix follows: > > Return the right error value when dup2() or dup3() newfd argument is too large I don't think this fixes anything. You're fixing replace_fd(), but dup2/dup3 don't actually *use* that. They have their own RLIMIT_NOFILE check and return -EMFILE there. Hmm? Linus ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: dup2 return value mismatch 2012-10-31 3:14 ` Linus Torvalds @ 2012-10-31 3:37 ` Al Viro 2012-10-31 3:39 ` Al Viro 0 siblings, 1 reply; 5+ messages in thread From: Al Viro @ 2012-10-31 3:37 UTC (permalink / raw) To: Linus Torvalds Cc: linux-fsdevel@vger.kernel.org, Sanil kumar, shyju pv, Nagamani Mantha, Maxiansheng (Max) On Tue, Oct 30, 2012 at 08:14:52PM -0700, Linus Torvalds wrote: > On Tue, Oct 30, 2012 at 8:00 PM, Al Viro <viro@zeniv.linux.org.uk> wrote: > > > > Umm... After looking at what POSIX actually says... There is an issue, > > all right. Thanks for spotting. Fix follows: > > > > Return the right error value when dup2() or dup3() newfd argument is too large > > I don't think this fixes anything. > > You're fixing replace_fd(), but dup2/dup3 don't actually *use* that. > They have their own RLIMIT_NOFILE check and return -EMFILE there. Ow... Moral: when :r in vi picks the file you've just scp'ed there from another xterm, it might be the variant you've sent there a couple of minutes prior ;-/ You are right, of course - sys_dup3() gets the same change (sys_dup2() doesn't; dup2(n, n) with n opened and currently beyond rlimit is not worth bothering *and* we'd never failed with EBADF in that case anyway; all other cases are covered by sys_dup3() change). Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> --- diff --git a/fs/file.c b/fs/file.c index ec20de9..603c7b5 100644 --- a/fs/file.c +++ b/fs/file.c @@ -894,7 +894,7 @@ int replace_fd(unsigned fd, struct file *file, unsigned flags) return __close_fd(files, fd); if (fd >= rlimit(RLIMIT_NOFILE)) - return -EMFILE; + return -EBADF; spin_lock(&files->file_lock); err = expand_files(files, fd); @@ -920,7 +920,7 @@ SYSCALL_DEFINE3(dup3, unsigned int, oldfd, unsigned int, newfd, int, flags) return -EINVAL; if (newfd >= rlimit(RLIMIT_NOFILE)) - return -EMFILE; + return -BADF; spin_lock(&files->file_lock); err = expand_files(files, newfd); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: dup2 return value mismatch 2012-10-31 3:37 ` Al Viro @ 2012-10-31 3:39 ` Al Viro 0 siblings, 0 replies; 5+ messages in thread From: Al Viro @ 2012-10-31 3:39 UTC (permalink / raw) To: Linus Torvalds Cc: linux-fsdevel@vger.kernel.org, Sanil kumar, shyju pv, Nagamani Mantha, Maxiansheng (Max) On Wed, Oct 31, 2012 at 03:37:48AM +0000, Al Viro wrote: > On Tue, Oct 30, 2012 at 08:14:52PM -0700, Linus Torvalds wrote: > > On Tue, Oct 30, 2012 at 8:00 PM, Al Viro <viro@zeniv.linux.org.uk> wrote: > > > > > > Umm... After looking at what POSIX actually says... There is an issue, > > > all right. Thanks for spotting. Fix follows: > > > > > > Return the right error value when dup2() or dup3() newfd argument is too large > > > > I don't think this fixes anything. > > > > You're fixing replace_fd(), but dup2/dup3 don't actually *use* that. > > They have their own RLIMIT_NOFILE check and return -EMFILE there. > > Ow... Moral: when :r in vi picks the file you've just scp'ed there from > another xterm, it might be the variant you've sent there a couple of > minutes prior ;-/ You are right, of course - sys_dup3() gets the same > change (sys_dup2() doesn't; dup2(n, n) with n opened and currently beyond > rlimit is not worth bothering *and* we'd never failed with EBADF in that > case anyway; all other cases are covered by sys_dup3() change). > > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> > --- > diff --git a/fs/file.c b/fs/file.c > index ec20de9..603c7b5 100644 > --- a/fs/file.c > +++ b/fs/file.c > @@ -894,7 +894,7 @@ int replace_fd(unsigned fd, struct file *file, unsigned flags) > return __close_fd(files, fd); > > if (fd >= rlimit(RLIMIT_NOFILE)) > - return -EMFILE; > + return -EBADF; > > spin_lock(&files->file_lock); > err = expand_files(files, fd); > @@ -920,7 +920,7 @@ SYSCALL_DEFINE3(dup3, unsigned int, oldfd, unsigned int, newfd, int, flags) > return -EINVAL; > > if (newfd >= rlimit(RLIMIT_NOFILE)) > - return -EMFILE; > + return -BADF; Grrrrr... And testing would also be a good idea. EBADF, of course. Al "I'll go and hide somewhere in shame now" Viro, with apologies ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-10-31 3:39 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-10-31 2:18 dup2 return value mismatch Linliangjie 2012-10-31 3:00 ` Al Viro 2012-10-31 3:14 ` Linus Torvalds 2012-10-31 3:37 ` Al Viro 2012-10-31 3:39 ` Al Viro
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).