cvs commit: src/sys/kern kern_descrip.c

[Ed Schouten]

* Robert Watson <rwatson at FreeBSD.org> wrote:
>
> On Wed, 28 May 2008, Ed Schouten wrote:
>
>>  Remove redundant checks from fcntl()'s F_DUPFD.
>>
>>  Right now we perform some of the checks inside the fcntl()'s F_DUPFD
>>  operation twice. We first validate the `fd' argument. When finished,
>>  we validate the `arg' argument. These checks are also performed inside
>>  do_dup().
>>
>>  The reason we need to do this, is because fcntl() should return different
>>  errno's when the `arg' argument is out of bounds (EINVAL instead of
>>  EBADF). To prevent the redundant locking of the PROC_LOCK and
>>  FILEDESC_SLOCK, patch do_dup() to support the error semantics required
>>  by fcntl().
>
> This sounds like a good candidate for a regression test -- do we have a  
> dup/dup2/F_DUPFD/F_DUP2FD test?  If not, perhaps we should, in light of 
> the opportunity for further bugs and regressions.

It looks like we already have regression tests for dup/dup2/etc --
kern_descrip.c 1.325 mentions them.

I saw FreeBSD also implements F_DUP2FD (which is a non-standard
extension). Right now this command returns EBADF when you do the
following:

	fcntl(0, F_DUP2FD, -1);		// below zero
	fcntl(0, F_DUP2FD, 1000000);	// too high

This is exactly the same as what dup2() does, but is inconsistent with
fcntl() in general. EBADF should be returned if the `fd' argument is
invalid. It should not apply to the argument.

We could consider applying the following patch:

--- sys/kern/kern_descrip.c
+++ sys/kern/kern_descrip.c
@@ -423,7 +423,8 @@
 
 	case F_DUP2FD:
 		tmp = arg;
-		error = do_dup(td, DUP_FIXED, fd, tmp, td->td_retval);
+		error = do_dup(td, DUP_FIXED|DUP_FCNTL, fd, tmp,
+		    td->td_retval);
 		break;
 
 	case F_GETFD:
--- lib/libc/sys/fcntl.2
+++ lib/libc/sys/fcntl.2
@@ -452,14 +452,6 @@
 The argument
 .Fa cmd
 is
-.Dv F_DUP2FD ,
-and
-.Fa arg
-is not a valid file descriptor.
-.Pp
-The argument
-.Fa cmd
-is
 .Dv F_SETLK
 or
 .Dv F_SETLKW ,
@@ -502,6 +494,8 @@
 argument
 is
 .Dv F_DUPFD
+or
+.Dv F_DUP2FD
 and
 .Fa arg
 is negative or greater than the maximum allowable number

Any comments?

-- 
 Ed Schouten <ed at 80386.nl>
 WWW: http://80386.nl/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/cvs-src/attachments/20080529/8720e2bb/attachment.pgp