d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

vfs: add a work around for vp_crossmp bug to realpath

Browse Source 
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
This commit is contained in:
Mateusz Guzik 2020-08-22 06:56:04 +00:00
parent 71bba112f6
commit 760a430bb3
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=364478
2 changed files with 31 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
sys/kern/vfs_cache.c
View File

@ -2811,6 +2811,7 @@ vn_fullpath_hardlink(struct thread *td, struct nameidata *ndp, char **retbuf,
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 nameidata *ndp, char **retbuf,
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) {

5
sys/kern/vfs_lookup.c
View File

@ -1208,6 +1208,11 @@ lookup(struct nameidata *ndp)
if ((cnp->cn_flags & LOCKLEAF) == 0)
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.