Date: Thu, 24 Jan 2008 18:31:27 GMT From: Robert Watson <rwatson@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 134020 for review Message-ID: <200801241831.m0OIVRLn094536@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=134020 Change 134020 by rwatson@rwatson_freebsd_capabilities on 2008/01/24 18:30:30 Rework file descriptor close to better understand capabilities: notifications of close(fd) to file locking and mqueue notification must happen with the underlying file descriptor and not the capability. Properly drop the extra thread-owned reference to a new capability once cap_new() is done. Move capability_close() above various implementations of panic() for fileops. Affected files ... .. //depot/projects/trustedbsd/capabilities/src/sys/kern/kern_descrip.c#5 edit .. //depot/projects/trustedbsd/capabilities/src/sys/kern/sys_capability.c#10 edit Differences ... ==== //depot/projects/trustedbsd/capabilities/src/sys/kern/kern_descrip.c#5 (text+ko) ==== @@ -1011,7 +1011,7 @@ int fd; { struct filedesc *fdp; - struct file *fp; + struct file *fp, *fp_object; int error; int holdleaders; @@ -1046,8 +1046,14 @@ * added, and deleteing a knote for the new fd. */ knote_fdclose(td, fd); - if (fp->f_type == DTYPE_MQUEUE) - mq_fdclose(td, fd, fp); + + /* + * When we're closing an fd with a capability, we need to notify + * mqueue if the underlying object is of type mqueue. + */ + (void)cap_fextract(fp, 0, &fp_object); + if (fp_object->f_type == DTYPE_MQUEUE) + mq_fdclose(td, fd, fp_object); FILEDESC_XUNLOCK(fdp); error = closef(fp, td); @@ -1882,6 +1888,7 @@ struct flock lf; struct filedesc_to_leader *fdtol; struct filedesc *fdp; + struct file *fp_object; /* * POSIX record locking dictates that any close releases ALL @@ -1894,11 +1901,15 @@ * NULL thread pointer when there really is no owning * context that might have locks, or the locks will be * leaked. + * + * If this is a capability, we do lock processing under the + * underyling vnode, not the capability. */ - if (fp->f_type == DTYPE_VNODE && td != NULL) { + (void)cap_fextract(fp, 0, &fp_object); + if (fp_object->f_type == DTYPE_VNODE && td != NULL) { int vfslocked; - vp = fp->f_vnode; + vp = fp_object->f_vnode; vfslocked = VFS_LOCK_GIANT(vp->v_mount); if ((td->td_proc->p_leader->p_flag & P_ADVLOCK) != 0) { lf.l_whence = SEEK_SET; @@ -1928,7 +1939,7 @@ lf.l_start = 0; lf.l_len = 0; lf.l_type = F_UNLCK; - vp = fp->f_vnode; + vp = fp_object->f_vnode; (void) VOP_ADVLOCK(vp, (caddr_t)fdtol->fdl_leader, F_UNLCK, &lf, F_POSIX); @@ -2189,6 +2200,9 @@ /* * Handle the last reference to a file being closed. + * + * No special capability handling here, as the capability's fo_close will run + * instead of the object here, and perform any necessary drop on the object. */ int _fdrop(struct file *fp, struct thread *td) ==== //depot/projects/trustedbsd/capabilities/src/sys/kern/sys_capability.c#10 (text+ko) ==== @@ -59,7 +59,7 @@ */ #include <sys/cdefs.h> -__FBSDID("$P4: //depot/projects/trustedbsd/capabilities/src/sys/kern/sys_capability.c#9 $"); +__FBSDID("$P4: //depot/projects/trustedbsd/capabilities/src/sys/kern/sys_capability.c#10 $"); #include <sys/param.h> #include <sys/capability.h> @@ -263,6 +263,7 @@ finit(fp_cap, fp->f_flag, DTYPE_CAPABILITY, c, &capability_ops); td->td_retval[0] = fd_cap; fdrop(fp, td); + fdrop(fp_cap, td); return (0); fail2: @@ -292,9 +293,31 @@ } /* + * When a capability is closed, simply drop the reference on the underlying + * object and free the capability. fdrop() will handle the case where the + * underlying object also needs to close, and the caller will have already + * performed any object-specific lock or mqueue handling. + */ +static int +capability_close(struct file *fp, struct thread *td) +{ + struct capability *c; + struct file *fp_object; + + KASSERT(fp->f_type == DTYPE_CAPABILITY, + ("capability_close: !capability")); + c = fp->f_data; + fp->f_ops = &badfileops; + fp->f_data = NULL; + fp_object = c->cap_file; + uma_zfree(capability_zone, c); + return (fdrop(fp_object, td)); +} + +/* * In general, file descriptor operations should never make it to the - * capability, only the underlying file descriptor operation vector, so with - * the exception of close(), panic if we do turn up here. + * capability, only the underlying file descriptor operation vector, so panic + * if we do turn up here. */ static int capability_read(struct file *fp, struct uio *uio, struct ucred *active_cred, @@ -350,19 +373,3 @@ panic("capability_stat"); } - -static int -capability_close(struct file *fp, struct thread *td) -{ - struct capability *c; - struct file *fp_object; - - KASSERT(fp->f_type == DTYPE_CAPABILITY, - ("capability_close: !capability")); - c = fp->f_data; - fp->f_ops = &badfileops; - fp->f_data = NULL; - fp_object = c->cap_file; - uma_zfree(capability_zone, c); - return (fdrop(fp_object, td)); -}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200801241831.m0OIVRLn094536>