From owner-p4-projects@FreeBSD.ORG Thu Jan 24 18:31:29 2008 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 71FC316A41A; Thu, 24 Jan 2008 18:31:29 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0A02616A418 for ; Thu, 24 Jan 2008 18:31:29 +0000 (UTC) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id EC81613C4D1 for ; Thu, 24 Jan 2008 18:31:27 +0000 (UTC) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.1/8.14.1) with ESMTP id m0OIVRU6094539 for ; Thu, 24 Jan 2008 18:31:27 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.1/8.14.1/Submit) id m0OIVRLn094536 for perforce@freebsd.org; Thu, 24 Jan 2008 18:31:27 GMT (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Date: Thu, 24 Jan 2008 18:31:27 GMT Message-Id: <200801241831.m0OIVRLn094536@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to bb+lists.freebsd.perforce@cyrus.watson.org using -f From: Robert Watson To: Perforce Change Reviews Cc: Subject: PERFORCE change 134020 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Jan 2008 18:31:29 -0000 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 -__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 #include @@ -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)); -}