VFS: better dupfrom(2) deadlock detection

Change-Id: I29f00075698888c7c8ca60b47ab82fba8c606f4e
This commit is contained in:
David van Moolenbroek 2013-10-05 12:50:44 +02:00 committed by Lionel Sambuc
parent 9e3e032c26
commit 50685cbec3
4 changed files with 19 additions and 16 deletions

View File

@ -560,9 +560,13 @@ int do_ioctl(message *UNUSED(m_out))
if (r == OK) { if (r == OK) {
dev = (dev_t) vp->v_sdev; dev = (dev_t) vp->v_sdev;
if (S_ISBLK(vp->v_mode)) if (S_ISBLK(vp->v_mode)) {
f->filp_ioctl_fp = fp;
r = bdev_ioctl(dev, who_e, ioctlrequest, argx); r = bdev_ioctl(dev, who_e, ioctlrequest, argx);
else
f->filp_ioctl_fp = NULL;
} else
r = cdev_io(CDEV_IOCTL, dev, who_e, argx, 0, ioctlrequest, r = cdev_io(CDEV_IOCTL, dev, who_e, argx, 0, ioctlrequest,
f->filp_flags); f->filp_flags);
} }

View File

@ -15,6 +15,9 @@ EXTERN struct filp {
struct fproc *filp_softlock; /* if not NULL; this filp didn't lock the struct fproc *filp_softlock; /* if not NULL; this filp didn't lock the
* vnode. Another filp already holds a lock * vnode. Another filp already holds a lock
* for this thread */ * for this thread */
struct fproc *filp_ioctl_fp; /* if not NULL, this filp is locked by the
* process for a currently ongoing IOCTL call
*/
/* the following fields are for select() and are owned by the generic /* the following fields are for select() and are owned by the generic
* select() code (i.e., fd-type-specific select() code can't touch these). * select() code (i.e., fd-type-specific select() code can't touch these).

View File

@ -131,6 +131,7 @@ int get_fd(struct fproc *rfp, int start, mode_t bits, int *k, struct filp **fpt)
f->filp_flags = 0; f->filp_flags = 0;
f->filp_select_flags = 0; f->filp_select_flags = 0;
f->filp_softlock = NULL; f->filp_softlock = NULL;
f->filp_ioctl_fp = NULL;
*fpt = f; *fpt = f;
return(OK); return(OK);
} }
@ -628,7 +629,6 @@ int do_dupfrom(message *UNUSED(m_out))
*/ */
struct fproc *rfp; struct fproc *rfp;
struct filp *rfilp; struct filp *rfilp;
struct vnode *vp;
endpoint_t endpt; endpoint_t endpt;
int r, fd, slot; int r, fd, slot;
@ -647,19 +647,15 @@ int do_dupfrom(message *UNUSED(m_out))
if ((rfilp = get_filp2(rfp, fd, VNODE_NONE)) == NULL) if ((rfilp = get_filp2(rfp, fd, VNODE_NONE)) == NULL)
return(err_code); return(err_code);
/* For now, we do not allow remote duplication of device nodes. In practice, /* If the filp is involved in an IOCTL by the user process, locking the filp
* only a block-special file can cause a deadlock for the caller (currently * here would result in a deadlock. This would happen if a user process
* only the VND driver). This would happen if a user process passes in the * passes in the file descriptor to the device node on which it is performing
* file descriptor to the device node on which it is performing the IOCTL. * the IOCTL. We do not allow manipulation of such device nodes. In
* This would cause two VFS threads to deadlock on the same filp. Since the * practice, this only applies to block-special files (and thus VND), because
* VND driver does not allow device nodes to be used anyway, this somewhat * character-special files (as used by UDS) are unlocked during the IOCTL.
* rudimentary check eliminates such deadlocks. A better solution would be
* to check if the given endpoint holds a lock to the target filp, but we
* currently do not have this information within VFS.
*/ */
vp = rfilp->filp_vno; if (rfilp->filp_ioctl_fp == rfp)
if (S_ISCHR(vp->v_mode) || S_ISBLK(vp->v_mode)) return(EBADF);
return(EINVAL);
/* Now we can safely lock the filp, copy it, and unlock it again. */ /* Now we can safely lock the filp, copy it, and unlock it again. */
lock_filp(rfilp, VNODE_READ); lock_filp(rfilp, VNODE_READ);

View File

@ -42,7 +42,7 @@ main(int argc, char **argv)
return EXIT_FAILURE; return EXIT_FAILURE;
} }
if (errno != EINVAL) { if (errno != EBADF) {
perror("ioctl(VNDIOCSET)"); perror("ioctl(VNDIOCSET)");
return EXIT_FAILURE; return EXIT_FAILURE;
} }