Author: mjg
Date: Sat Aug 22 06:56:04 2020
New Revision: 364478
URL: https://svnweb.freebsd.org/changeset/base/364478
Log:
vfs: add a work around for vp_crossmp bug to realpath
The actual bug is not yet addressed as it will get much easier after other
problems are addressed (most notably rename contract).
The only affected in-tree consumer is realpath. Everyone else happens to be
performing lookups within a mount point, having a side effect of ni_dvp being
set to mount point's root vnode in the worst case.
Reported by: pho
Modified:
head/sys/kern/vfs_cache.c
head/sys/kern/vfs_lookup.c
Modified: head/sys/kern/vfs_cache.c
==============================================================================
--- head/sys/kern/vfs_cache.c Sat Aug 22 04:07:44 2020 (r364477)
+++ head/sys/kern/vfs_cache.c Sat Aug 22 06:56:04 2020 (r364478)
@@ -2811,6 +2811,7 @@ vn_fullpath_hardlink(struct thread *td, struct nameida
size_t addend;
int error;
bool slash_prefixed;
+ enum vtype type;
if (*buflen < 2)
return (EINVAL);
@@ -2824,7 +2825,31 @@ vn_fullpath_hardlink(struct thread *td, struct nameida
addend = 0;
vp = ndp->ni_vp;
- if (vp->v_type != VDIR) {
+ /*
+ * Check for VBAD to work around the vp_crossmp bug in lookup().
+ *
+ * For example consider tmpfs on /tmp and realpath /tmp. ni_vp will be
+ * set to mount point's root vnode while ni_dvp will be vp_crossmp.
+ * If the type is VDIR (like in this very case) we can skip looking
+ * at ni_dvp in the first place. However, since vnodes get passed here
+ * unlocked the target may transition to doomed state (type == VBAD)
+ * before we get to evaluate the condition. If this happens, we will
+ * populate part of the buffer and descend to vn_fullpath_dir with
+ * vp == vp_crossmp. Prevent the problem by checking for VBAD.
+ *
+ * This should be atomic_load(&vp->v_type) but it is ilegal to take
+ * an address of a bit field, even if said field is sized to char.
+ * Work around the problem by reading the value into a full-sized enum
+ * and then re-reading it with atomic_load which will still prevent
+ * the compiler from re-reading down the road.
+ */
+ type = vp->v_type;
+ type = atomic_load_int(&type);
+ if (type == VBAD) {
+ error = ENOENT;
+ goto out_bad;
+ }
+ if (type != VDIR) {
cnp = &ndp->ni_cnd;
addend = cnp->cn_namelen + 2;
if (*buflen < addend) {
Modified: head/sys/kern/vfs_lookup.c
==============================================================================
--- head/sys/kern/vfs_lookup.c Sat Aug 22 04:07:44 2020 (r364477)
+++ head/sys/kern/vfs_lookup.c Sat Aug 22 06:56:04 2020 (r364478)
@@ -1209,6 +1209,11 @@ nextname:
VOP_UNLOCK(dp);
success:
/*
+ * FIXME: for lookups which only cross a mount point to fetch the
+ * root vnode, ni_dvp will be set to vp_crossmp. This can be a problem
+ * if either WANTPARENT or LOCKPARENT is set.
+ */
+ /*
* Because of shared lookup we may have the vnode shared locked, but
* the caller may want it to be exclusively locked.
*/
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
