From 877d24ac8ac9ccc82bf610d00beacfcb80dca686 Mon Sep 17 00:00:00 2001 From: Konstantin Belousov Date: Fri, 28 Sep 2012 11:25:02 +0000 Subject: [PATCH] Fix the mis-handling of the VV_TEXT on the nullfs vnodes. If you have a binary on a filesystem which is also mounted over by nullfs, you could execute the binary from the lower filesystem, or from the nullfs mount. When executed from lower filesystem, the lower vnode gets VV_TEXT flag set, and the file cannot be modified while the binary is active. But, if executed as the nullfs alias, only the nullfs vnode gets VV_TEXT set, and you still can open the lower vnode for write. Add a set of VOPs for the VV_TEXT query, set and clear operations, which are correctly bypassed to lower vnode. Tested by: pho (previous version) MFC after: 2 weeks --- sys/compat/linux/linux_misc.c | 2 +- sys/fs/coda/coda_subr.c | 4 ++-- sys/fs/nfsserver/nfs_nfsdport.c | 2 +- sys/kern/imgact_elf.c | 2 +- sys/kern/kern_exec.c | 13 +++++-------- sys/kern/vfs_default.c | 30 ++++++++++++++++++++++++++++++ sys/kern/vfs_vnops.c | 2 +- sys/kern/vnode_if.src | 18 ++++++++++++++++++ sys/nfsserver/nfs_serv.c | 2 +- sys/vm/vm_object.c | 2 +- sys/vm/vnode_pager.c | 2 +- 11 files changed, 62 insertions(+), 17 deletions(-) diff --git a/sys/compat/linux/linux_misc.c b/sys/compat/linux/linux_misc.c index 4365b10bdabe..50e21759ff2a 100644 --- a/sys/compat/linux/linux_misc.c +++ b/sys/compat/linux/linux_misc.c @@ -386,7 +386,7 @@ linux_uselib(struct thread *td, struct linux_uselib_args *args) * XXX: Note that if any of the VM operations fail below we don't * clear this flag. */ - vp->v_vflag |= VV_TEXT; + VOP_SET_TEXT(vp); /* * Lock no longer needed diff --git a/sys/fs/coda/coda_subr.c b/sys/fs/coda/coda_subr.c index ecdd927d3743..a7ae3549aace 100644 --- a/sys/fs/coda/coda_subr.c +++ b/sys/fs/coda/coda_subr.c @@ -486,7 +486,7 @@ handleDownCall(struct coda_mntinfo *mnt, int opcode, union outputArgs *out) cache_purge(CTOV(cp)); cp->c_flags &= ~(C_VATTR | C_ACCCACHE); ASSERT_VOP_LOCKED(CTOV(cp), "coda HandleDownCall"); - if (CTOV(cp)->v_vflag & VV_TEXT) + if (VOP_IS_TEXT(CTOV(cp))) error = coda_vmflush(cp); CODADEBUG(CODA_ZAPFILE, myprintf(("zapfile: fid = %s, refcnt = %d, error = " @@ -532,7 +532,7 @@ handleDownCall(struct coda_mntinfo *mnt, int opcode, union outputArgs *out) cp->c_flags &= ~(C_VATTR | C_ACCCACHE); ASSERT_VOP_LOCKED(CTOV(cp), "coda HandleDownCall"); if (!(IS_DIR(out->coda_purgefid.Fid)) - && (CTOV(cp)->v_vflag & VV_TEXT)) + && VOP_IS_TEXT(CTOV(cp))) error = coda_vmflush(cp); CODADEBUG(CODA_PURGEFID, myprintf(("purgefid: fid " "= %s, refcnt = %d, error = %d\n", diff --git a/sys/fs/nfsserver/nfs_nfsdport.c b/sys/fs/nfsserver/nfs_nfsdport.c index 4729859edf83..21d184b94068 100644 --- a/sys/fs/nfsserver/nfs_nfsdport.c +++ b/sys/fs/nfsserver/nfs_nfsdport.c @@ -252,7 +252,7 @@ nfsvno_accchk(struct vnode *vp, accmode_t accmode, struct ucred *cred, * the inode, try to free it up once. If * we fail, we can't allow writing. */ - if ((vp->v_vflag & VV_TEXT) != 0 && error == 0) + if (VOP_IS_TEXT(vp) && error == 0) error = ETXTBSY; } if (error != 0) { diff --git a/sys/kern/imgact_elf.c b/sys/kern/imgact_elf.c index b6266101c195..1fcd995ef2a7 100644 --- a/sys/kern/imgact_elf.c +++ b/sys/kern/imgact_elf.c @@ -647,7 +647,7 @@ __elfN(load_file)(struct proc *p, const char *file, u_long *addr, * Also make certain that the interpreter stays the same, so set * its VV_TEXT flag, too. */ - nd->ni_vp->v_vflag |= VV_TEXT; + VOP_SET_TEXT(nd->ni_vp); imgp->object = nd->ni_vp->v_object; diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c index 90f7311b8209..eff0f832b01d 100644 --- a/sys/kern/kern_exec.c +++ b/sys/kern/kern_exec.c @@ -473,9 +473,8 @@ do_execve(td, args, mac_p) * Remember if this was set before and unset it in case this is not * actually an executable image. */ - textset = imgp->vp->v_vflag & VV_TEXT; - ASSERT_VOP_ELOCKED(imgp->vp, "vv_text"); - imgp->vp->v_vflag |= VV_TEXT; + textset = VOP_IS_TEXT(imgp->vp); + VOP_SET_TEXT(imgp->vp); error = exec_map_first_page(imgp); if (error) @@ -506,10 +505,8 @@ do_execve(td, args, mac_p) if (error) { if (error == -1) { - if (textset == 0) { - ASSERT_VOP_ELOCKED(imgp->vp, "vv_text"); - imgp->vp->v_vflag &= ~VV_TEXT; - } + if (textset == 0) + VOP_UNSET_TEXT(imgp->vp); error = ENOEXEC; } goto exec_fail_dealloc; @@ -527,7 +524,7 @@ do_execve(td, args, mac_p) * VV_TEXT will be set. The vnode lock is held over this * entire period so nothing should illegitimately be blocked. */ - imgp->vp->v_vflag &= ~VV_TEXT; + VOP_UNSET_TEXT(imgp->vp); /* free name buffer and old vnode */ if (args->fname != NULL) NDFREE(&nd, NDF_ONLY_PNBUF); diff --git a/sys/kern/vfs_default.c b/sys/kern/vfs_default.c index b807db892bd9..31a650c48986 100644 --- a/sys/kern/vfs_default.c +++ b/sys/kern/vfs_default.c @@ -78,6 +78,10 @@ static int dirent_exists(struct vnode *vp, const char *dirname, #define DIRENT_MINSIZE (sizeof(struct dirent) - (MAXNAMLEN+1) + 4) +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); + /* * This vnode table stores what we want to do if the filesystem doesn't * implement a particular VOP. @@ -126,6 +130,9 @@ struct vop_vector default_vnodeops = { .vop_unp_bind = vop_stdunp_bind, .vop_unp_connect = vop_stdunp_connect, .vop_unp_detach = vop_stdunp_detach, + .vop_is_text = vop_stdis_text, + .vop_set_text = vop_stdset_text, + .vop_unset_text = vop_stdunset_text, }; /* @@ -1073,6 +1080,29 @@ vop_stdunp_detach(struct vop_unp_detach_args *ap) return (0); } +static int +vop_stdis_text(struct vop_is_text_args *ap) +{ + + return ((ap->a_vp->v_vflag & VV_TEXT) != 0); +} + +static int +vop_stdset_text(struct vop_set_text_args *ap) +{ + + ap->a_vp->v_vflag |= VV_TEXT; + return (0); +} + +static int +vop_stdunset_text(struct vop_unset_text_args *ap) +{ + + ap->a_vp->v_vflag &= ~VV_TEXT; + 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 fec55f941ffb..49535cc7c15e 100644 --- a/sys/kern/vfs_vnops.c +++ b/sys/kern/vfs_vnops.c @@ -342,7 +342,7 @@ vn_writechk(vp) * the vnode, try to free it up once. If * we fail, we can't allow writing. */ - if (vp->v_vflag & VV_TEXT) + if (VOP_IS_TEXT(vp)) return (ETXTBSY); return (0); diff --git a/sys/kern/vnode_if.src b/sys/kern/vnode_if.src index 5e3fa1118ef9..4c86ff6c8936 100644 --- a/sys/kern/vnode_if.src +++ b/sys/kern/vnode_if.src @@ -660,6 +660,24 @@ vop_unp_detach { IN struct vnode *vp; }; +%% is_text vp L L L + +vop_is_text { + IN struct vnode *vp; +}; + +%% set_text vp E E E + +vop_set_text { + IN struct vnode *vp; +}; + +%% vop_unset_text vp E E E + +vop_unset_text { + IN struct vnode *vp; +}; + # 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/nfsserver/nfs_serv.c b/sys/nfsserver/nfs_serv.c index e51c761eee7f..ae8f6d6becee 100644 --- a/sys/nfsserver/nfs_serv.c +++ b/sys/nfsserver/nfs_serv.c @@ -3896,7 +3896,7 @@ nfsrv_access(struct vnode *vp, accmode_t accmode, struct ucred *cred, * If there's shared text associated with * the inode, we can't allow writing. */ - if (vp->v_vflag & VV_TEXT) + if (VOP_IS_TEXT(vp)) return (ETXTBSY); } diff --git a/sys/vm/vm_object.c b/sys/vm/vm_object.c index 8157649330d8..abf7689174a5 100644 --- a/sys/vm/vm_object.c +++ b/sys/vm/vm_object.c @@ -456,7 +456,7 @@ vm_object_vndeallocate(vm_object_t object) VOP_UNLOCK(vp, 0); } else { if (object->ref_count == 0) - vp->v_vflag &= ~VV_TEXT; + VOP_UNSET_TEXT(vp); VM_OBJECT_UNLOCK(object); vput(vp); } diff --git a/sys/vm/vnode_pager.c b/sys/vm/vnode_pager.c index b0139ce07f42..cc9063b00826 100644 --- a/sys/vm/vnode_pager.c +++ b/sys/vm/vnode_pager.c @@ -277,7 +277,7 @@ vnode_pager_dealloc(object) __func__, vp, vp->v_writecount); } vp->v_object = NULL; - vp->v_vflag &= ~VV_TEXT; + VOP_UNSET_TEXT(vp); VM_OBJECT_UNLOCK(object); while (refs-- > 0) vunref(vp);