d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Remove redundant checks from fcntl()'s F_DUPFD.

Browse Source 
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().

Approved by:	philip (mentor)
This commit is contained in:
Ed Schouten 2008-05-28 20:25:19 +00:00
parent 4cef699e97
commit cc8945d204
1 changed files with 16 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
sys/kern/kern_descrip.c
View File

@ -91,10 +91,11 @@ static MALLOC_DEFINE(M_SIGIO, "sigio", "sigio structures");
static uma_zone_t file_zone; static uma_zone_t file_zone;
/* How to treat 'new' parameter when allocating a fd for do_dup(). */ /* Flags for do_dup() */
enum dup_type { DUP_VARIABLE, DUP_FIXED }; #define DUP_FIXED 0x1 /* Force fixed allocation */
#define DUP_FCNTL 0x2 /* fcntl()-style errors */
static int do_dup(struct thread *td, enum dup_type type, int old, int new, static int do_dup(struct thread *td, int flags, int old, int new,
register_t *retval); register_t *retval);
static int fd_first_free(struct filedesc *, int, int); static int fd_first_free(struct filedesc *, int, int);
static int fd_last_used(struct filedesc *, int, int); static int fd_last_used(struct filedesc *, int, int);
@ -302,7 +303,7 @@ int
dup(struct thread *td, struct dup_args *uap) dup(struct thread *td, struct dup_args *uap)
{ {
return (do_dup(td, DUP_VARIABLE, (int)uap->fd, 0, td->td_retval)); return (do_dup(td, 0, (int)uap->fd, 0, td->td_retval));
} }
/* /*
@ -405,7 +406,6 @@ kern_fcntl(struct thread *td, int fd, int cmd, intptr_t arg)
struct proc *p; struct proc *p;
char *pop; char *pop;
struct vnode *vp; struct vnode *vp;
u_int newmin;
int error, flg, tmp; int error, flg, tmp;
int vfslocked; int vfslocked;
@ -417,23 +417,8 @@ kern_fcntl(struct thread *td, int fd, int cmd, intptr_t arg)
switch (cmd) { switch (cmd) {
case F_DUPFD: case F_DUPFD:
FILEDESC_SLOCK(fdp); tmp = arg;
if ((fp = fdtofp(fd, fdp)) == NULL) { error = do_dup(td, DUP_FCNTL, fd, tmp, td->td_retval);
FILEDESC_SUNLOCK(fdp);
error = EBADF;
break;
}
FILEDESC_SUNLOCK(fdp);
newmin = arg;
PROC_LOCK(p);
if (newmin >= lim_cur(p, RLIMIT_NOFILE) ||
newmin >= maxfilesperproc) {
PROC_UNLOCK(p);
error = EINVAL;
break;
}
PROC_UNLOCK(p);
error = do_dup(td, DUP_VARIABLE, fd, newmin, td->td_retval);
break; break;
case F_DUP2FD: case F_DUP2FD:
@ -700,7 +685,7 @@ kern_fcntl(struct thread *td, int fd, int cmd, intptr_t arg)
* Common code for dup, dup2, fcntl(F_DUPFD) and fcntl(F_DUP2FD). * Common code for dup, dup2, fcntl(F_DUPFD) and fcntl(F_DUP2FD).
*/ */
static int static int
do_dup(struct thread *td, enum dup_type type, int old, int new, do_dup(struct thread *td, int flags, int old, int new,
register_t *retval) register_t *retval)
{ {
struct filedesc *fdp; struct filedesc *fdp;
@ -709,30 +694,30 @@ do_dup(struct thread *td, enum dup_type type, int old, int new,
struct file *delfp; struct file *delfp;
int error, holdleaders, maxfd; int error, holdleaders, maxfd;
KASSERT((type == DUP_VARIABLE || type == DUP_FIXED),
("invalid dup type %d", type));
p = td->td_proc; p = td->td_proc;
fdp = p->p_fd; fdp = p->p_fd;
/* /*
* Verify we have a valid descriptor to dup from and possibly to * Verify we have a valid descriptor to dup from and possibly to
* dup to. * dup to. Unlike dup() and dup2(), fcntl()'s F_DUPFD should
* return EINVAL when the new descriptor is out of bounds.
*/ */
if (old < 0 || new < 0) if (old < 0)
return (EBADF); return (EBADF);
if (new < 0)
return (flags & DUP_FCNTL ? EINVAL : EBADF);
PROC_LOCK(p); PROC_LOCK(p);
maxfd = min((int)lim_cur(p, RLIMIT_NOFILE), maxfilesperproc); maxfd = min((int)lim_cur(p, RLIMIT_NOFILE), maxfilesperproc);
PROC_UNLOCK(p); PROC_UNLOCK(p);
if (new >= maxfd) if (new >= maxfd)
return (EMFILE); return (flags & DUP_FCNTL ? EINVAL : EMFILE);
FILEDESC_XLOCK(fdp); FILEDESC_XLOCK(fdp);
if (old >= fdp->fd_nfiles || fdp->fd_ofiles[old] == NULL) { if (old >= fdp->fd_nfiles || fdp->fd_ofiles[old] == NULL) {
FILEDESC_XUNLOCK(fdp); FILEDESC_XUNLOCK(fdp);
return (EBADF); return (EBADF);
} }
if (type == DUP_FIXED && old == new) { if (flags & DUP_FIXED && old == new) {
*retval = new; *retval = new;
FILEDESC_XUNLOCK(fdp); FILEDESC_XUNLOCK(fdp);
return (0); return (0);
@ -747,7 +732,7 @@ do_dup(struct thread *td, enum dup_type type, int old, int new,
* lock may be temporarily dropped in the process, we have to look * lock may be temporarily dropped in the process, we have to look
* out for a race. * out for a race.
*/ */
if (type == DUP_FIXED) { if (flags & DUP_FIXED) {
if (new >= fdp->fd_nfiles) if (new >= fdp->fd_nfiles)
fdgrowtable(fdp, new + 1); fdgrowtable(fdp, new + 1);
if (fdp->fd_ofiles[new] == NULL) if (fdp->fd_ofiles[new] == NULL)