From f16ea990070abd8c9cb42bfc7001ada604b4df01 Mon Sep 17 00:00:00 2001 From: kib Date: Fri, 2 Nov 2012 13:56:36 +0000 Subject: The r241025 fixed the case when a binary, executed from nullfs mount, was still possible to open for write from the lower filesystem. There is a symmetric situation where the binary could already has file descriptors opened for write, but it can be executed from the nullfs overlay. Handle the issue by passing one v_writecount reference to the lower vnode if nullfs vnode has non-zero v_writecount. Note that only one write reference can be donated, since nullfs only keeps one use reference on the lower vnode. Always use the lower vnode v_writecount for the checks. Introduce the VOP_GET_WRITECOUNT to read v_writecount, which is currently always bypassed to the lower vnode, and VOP_ADD_WRITECOUNT to manipulate the v_writecount value, which manages a single bypass reference to the lower vnode. Caling the VOPs instead of directly accessing v_writecount provide the fix described in the previous paragraph. Tested by: pho MFC after: 3 weeks --- sys/compat/linux/linux_misc.c | 8 +++++--- sys/fs/nullfs/null_vnops.c | 21 +++++++++++++++++++++ sys/fs/unionfs/union_subr.c | 4 ++-- sys/i386/ibcs2/imgact_coff.c | 7 +++++-- sys/kern/kern_exec.c | 7 +++++-- sys/kern/vfs_default.c | 20 ++++++++++++++++++++ sys/kern/vfs_vnops.c | 4 ++-- sys/kern/vnode_if.src | 14 ++++++++++++++ sys/ufs/ufs/ufs_extattr.c | 2 +- sys/vm/vnode_pager.c | 6 +++--- 10 files changed, 78 insertions(+), 15 deletions(-) diff --git a/sys/compat/linux/linux_misc.c b/sys/compat/linux/linux_misc.c index 6da3b19..7587272 100644 --- a/sys/compat/linux/linux_misc.c +++ b/sys/compat/linux/linux_misc.c @@ -246,8 +246,7 @@ linux_uselib(struct thread *td, struct linux_uselib_args *args) unsigned long bss_size; char *library; ssize_t aresid; - int error; - int locked; + int error, locked, writecount; LCONVPATHEXIST(td, args->library, &library); @@ -277,7 +276,10 @@ linux_uselib(struct thread *td, struct linux_uselib_args *args) locked = 1; /* Writable? */ - if (vp->v_writecount) { + error = VOP_GET_WRITECOUNT(vp, &writecount); + if (error != 0) + goto cleanup; + if (writecount != 0) { error = ETXTBSY; goto cleanup; } diff --git a/sys/fs/nullfs/null_vnops.c b/sys/fs/nullfs/null_vnops.c index 94bfea8..f530ed2 100644 --- a/sys/fs/nullfs/null_vnops.c +++ b/sys/fs/nullfs/null_vnops.c @@ -329,6 +329,26 @@ null_bypass(struct vop_generic_args *ap) return (error); } +static int +null_add_writecount(struct vop_add_writecount_args *ap) +{ + struct vnode *lvp, *vp; + int error; + + vp = ap->a_vp; + lvp = NULLVPTOLOWERVP(vp); + KASSERT(vp->v_writecount + ap->a_inc >= 0, ("wrong writecount inc")); + if (vp->v_writecount > 0 && vp->v_writecount + ap->a_inc == 0) + error = VOP_ADD_WRITECOUNT(lvp, -1); + else if (vp->v_writecount == 0 && vp->v_writecount + ap->a_inc > 0) + error = VOP_ADD_WRITECOUNT(lvp, 1); + else + error = 0; + if (error == 0) + vp->v_writecount += ap->a_inc; + return (error); +} + /* * We have to carry on the locking protocol on the null layer vnodes * as we progress through the tree. We also have to enforce read-only @@ -826,4 +846,5 @@ struct vop_vector null_vnodeops = { .vop_unlock = null_unlock, .vop_vptocnp = null_vptocnp, .vop_vptofh = null_vptofh, + .vop_add_writecount = null_add_writecount, }; diff --git a/sys/fs/unionfs/union_subr.c b/sys/fs/unionfs/union_subr.c index d3532027..64bd4ac 100644 --- a/sys/fs/unionfs/union_subr.c +++ b/sys/fs/unionfs/union_subr.c @@ -945,7 +945,7 @@ unionfs_vn_create_on_upper(struct vnode **vpp, struct vnode *udvp, vput(vp); goto unionfs_vn_create_on_upper_free_out1; } - vp->v_writecount++; + VOP_ADD_WRITECOUNT(vp, 1); CTR3(KTR_VFS, "%s: vp %p v_writecount increased to %d", __func__, vp, vp->v_writecount); *vpp = vp; @@ -1082,7 +1082,7 @@ unionfs_copyfile(struct unionfs_node *unp, int docopy, struct ucred *cred, } } VOP_CLOSE(uvp, FWRITE, cred, td); - uvp->v_writecount--; + VOP_ADD_WRITECOUNT(uvp, -1); CTR3(KTR_VFS, "%s: vp %p v_writecount decreased to %d", __func__, uvp, uvp->v_writecount); diff --git a/sys/i386/ibcs2/imgact_coff.c b/sys/i386/ibcs2/imgact_coff.c index a28ba52..b155ef9 100644 --- a/sys/i386/ibcs2/imgact_coff.c +++ b/sys/i386/ibcs2/imgact_coff.c @@ -168,7 +168,7 @@ coff_load_file(struct thread *td, char *name) unsigned long text_offset = 0, text_address = 0, text_size = 0; unsigned long data_offset = 0, data_address = 0, data_size = 0; unsigned long bss_size = 0; - int i; + int i, writecount; NDINIT(&nd, LOOKUP, ISOPEN | LOCKLEAF | FOLLOW | SAVENAME, UIO_SYSSPACE, name, td); @@ -181,7 +181,10 @@ coff_load_file(struct thread *td, char *name) if (vp == NULL) return ENOEXEC; - if (vp->v_writecount) { + error = VOP_GET_WRITECOUNT(vp, &writecount); + if (error != 0) + goto fail; + if (writecount != 0) { error = ETXTBSY; goto fail; } diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c index 0970562..a37b9f1 100644 --- a/sys/kern/kern_exec.c +++ b/sys/kern/kern_exec.c @@ -1376,7 +1376,7 @@ exec_check_permissions(imgp) struct vnode *vp = imgp->vp; struct vattr *attr = imgp->attr; struct thread *td; - int error; + int error, writecount; td = curthread; @@ -1421,7 +1421,10 @@ exec_check_permissions(imgp) * Check number of open-for-writes on the file and deny execution * if there are any. */ - if (vp->v_writecount) + error = VOP_GET_WRITECOUNT(vp, &writecount); + if (error != 0) + return (error); + if (writecount != 0) return (ETXTBSY); /* diff --git a/sys/kern/vfs_default.c b/sys/kern/vfs_default.c index 0112dcb..00d064e 100644 --- a/sys/kern/vfs_default.c +++ b/sys/kern/vfs_default.c @@ -81,6 +81,8 @@ static int dirent_exists(struct vnode *vp, const char *dirname, static int vop_stdis_text(struct vop_is_text_args *ap); static int vop_stdset_text(struct vop_set_text_args *ap); static int vop_stdunset_text(struct vop_unset_text_args *ap); +static int vop_stdget_writecount(struct vop_get_writecount_args *ap); +static int vop_stdadd_writecount(struct vop_add_writecount_args *ap); /* * This vnode table stores what we want to do if the filesystem doesn't @@ -133,6 +135,8 @@ struct vop_vector default_vnodeops = { .vop_is_text = vop_stdis_text, .vop_set_text = vop_stdset_text, .vop_unset_text = vop_stdunset_text, + .vop_get_writecount = vop_stdget_writecount, + .vop_add_writecount = vop_stdadd_writecount, }; /* @@ -1100,6 +1104,22 @@ vop_stdunset_text(struct vop_unset_text_args *ap) return (0); } +static int +vop_stdget_writecount(struct vop_get_writecount_args *ap) +{ + + *ap->a_writecount = ap->a_vp->v_writecount; + return (0); +} + +static int +vop_stdadd_writecount(struct vop_add_writecount_args *ap) +{ + + ap->a_vp->v_writecount += ap->a_inc; + return (0); +} + /* * vfs default ops * used to fill the vfs function table to get reasonable default return values. diff --git a/sys/kern/vfs_vnops.c b/sys/kern/vfs_vnops.c index 402a9d0..ea42f9d 100644 --- a/sys/kern/vfs_vnops.c +++ b/sys/kern/vfs_vnops.c @@ -302,7 +302,7 @@ vn_open_vnode(struct vnode *vp, int fmode, struct ucred *cred, fp->f_flag |= FHASLOCK; } if (fmode & FWRITE) { - vp->v_writecount++; + VOP_ADD_WRITECOUNT(vp, 1); CTR3(KTR_VFS, "%s: vp %p v_writecount increased to %d", __func__, vp, vp->v_writecount); } @@ -355,7 +355,7 @@ vn_close(vp, flags, file_cred, td) if (flags & FWRITE) { VNASSERT(vp->v_writecount > 0, vp, ("vn_close: negative writecount")); - vp->v_writecount--; + VOP_ADD_WRITECOUNT(vp, -1); CTR3(KTR_VFS, "%s: vp %p v_writecount decreased to %d", __func__, vp, vp->v_writecount); } diff --git a/sys/kern/vnode_if.src b/sys/kern/vnode_if.src index 4c86ff6..3c3563c 100644 --- a/sys/kern/vnode_if.src +++ b/sys/kern/vnode_if.src @@ -678,6 +678,20 @@ vop_unset_text { IN struct vnode *vp; }; +%% get_writecount vp L L L + +vop_get_writecount { + IN struct vnode *vp; + OUT int *writecount; +}; + +%% add_writecount vp E E E + +vop_add_writecount { + IN struct vnode *vp; + IN int inc; +}; + # The VOPs below are spares at the end of the table to allow new VOPs to be # added in stable branches without breaking the KBI. New VOPs in HEAD should # be added above these spares. When merging a new VOP to a stable branch, diff --git a/sys/ufs/ufs/ufs_extattr.c b/sys/ufs/ufs/ufs_extattr.c index 48b272e..d52f650 100644 --- a/sys/ufs/ufs/ufs_extattr.c +++ b/sys/ufs/ufs/ufs_extattr.c @@ -334,7 +334,7 @@ ufs_extattr_enable_with_open(struct ufsmount *ump, struct vnode *vp, return (error); } - vp->v_writecount++; + VOP_ADD_WRITECOUNT(vp, 1); CTR3(KTR_VFS, "%s: vp %p v_writecount increased to %d", __func__, vp, vp->v_writecount); diff --git a/sys/vm/vnode_pager.c b/sys/vm/vnode_pager.c index f6848b6..a6d78f4 100644 --- a/sys/vm/vnode_pager.c +++ b/sys/vm/vnode_pager.c @@ -272,7 +272,7 @@ vnode_pager_dealloc(object) ASSERT_VOP_ELOCKED(vp, "vnode_pager_dealloc"); if (object->un_pager.vnp.writemappings > 0) { object->un_pager.vnp.writemappings = 0; - vp->v_writecount--; + VOP_ADD_WRITECOUNT(vp, -1); CTR3(KTR_VFS, "%s: vp %p v_writecount decreased to %d", __func__, vp, vp->v_writecount); } @@ -1212,12 +1212,12 @@ vnode_pager_update_writecount(vm_object_t object, vm_offset_t start, vp = object->handle; if (old_wm == 0 && object->un_pager.vnp.writemappings != 0) { ASSERT_VOP_ELOCKED(vp, "v_writecount inc"); - vp->v_writecount++; + VOP_ADD_WRITECOUNT(vp, 1); CTR3(KTR_VFS, "%s: vp %p v_writecount increased to %d", __func__, vp, vp->v_writecount); } else if (old_wm != 0 && object->un_pager.vnp.writemappings == 0) { ASSERT_VOP_ELOCKED(vp, "v_writecount dec"); - vp->v_writecount--; + VOP_ADD_WRITECOUNT(vp, -1); CTR3(KTR_VFS, "%s: vp %p v_writecount decreased to %d", __func__, vp, vp->v_writecount); } -- cgit v1.1