dup2 return value mismatch

dup2 return value mismatch

[Linliangjie]

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 

Re: dup2 return value mismatch

[Al Viro]

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);

Re: dup2 return value mismatch

[Linus Torvalds]

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

Re: dup2 return value mismatch

[Al Viro]

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);

Re: dup2 return value mismatch

[Al Viro]

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