[Bug 258729] linprocfs regression: /compat/linux/proc/*/cwd wrongly points to calling process's cwd for all PIDs

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=258729

Damjan Jovanovic <damjan.jov@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kib@FreeBSD.org,
                   |                            |mjg@FreeBSD.org

--- Comment #1 from Damjan Jovanovic <damjan.jov@gmail.com> ---
This is the commit that caused the regression:


---snip---
commit 8d03b99b9dafe92896f405c79f846667637c0194
Author: Mateusz Guzik <mjg@FreeBSD.org>
Date:   Sun Mar 1 21:53:46 2020 +0000

    fd: move vnodes out of filedesc into a dedicated structure

    The new structure is copy-on-write. With the assumption that path lookups
are
    significantly more frequent than chdirs and chrooting this is a win.

    This provides stable root and jail root vnodes without the need to
reference
    them on lookup, which in turn means less work on globally shared
structures.
    Note this also happens to fix a bug where jail vnode was never referenced,
    meaning subsequent access on lookup could run into use-after-free.

    Reviewed by:    kib
    Differential Revision:  https://reviews.freebsd.org/D23884
---snip---



It changed linprocfs_doproccwd() from returning the cwd of the passed "struct
proc" (p), to returning the cwd of the calling thread (td) instead:

---snip---
 static int
 linprocfs_doproccwd(PFS_FILL_ARGS)
 {
-       struct filedesc *fdp;
-       struct vnode *vp;
+       struct pwd *pwd;
        char *fullpath = "unknown";
        char *freepath = NULL;

-       fdp = p->p_fd;
-       FILEDESC_SLOCK(fdp);
-       vp = fdp->fd_cdir;
-       if (vp != NULL)
-               VREF(vp);
-       FILEDESC_SUNLOCK(fdp);
-       vn_fullpath(td, vp, &fullpath, &freepath);
-       if (vp != NULL)
-               vrele(vp);
+       pwd = pwd_hold(td);
+       vn_fullpath(td, pwd->pwd_cdir, &fullpath, &freepath);
        sbuf_printf(sb, "%s", fullpath);
        if (freepath)
                free(freepath, M_TEMP);
+       pwd_drop(pwd);
        return (0);
 }
---snip---



This patch fixes it (although it still needs proper locking and possibly
security checks):

---snip---
diff --git a/sys/compat/linprocfs/linprocfs.c
b/sys/compat/linprocfs/linprocfs.c
index 79ffc4dfd5a..ee94268a4b6 100644
--- a/sys/compat/linprocfs/linprocfs.c
+++ b/sys/compat/linprocfs/linprocfs.c
@@ -1169,7 +1169,7 @@ linprocfs_doproccwd(PFS_FILL_ARGS)
        char *fullpath = "unknown";
        char *freepath = NULL;

-       pwd = pwd_hold(td);
+       pwd = pwd_hold_pwddesc(p->p_pd);
        vn_fullpath(pwd->pwd_cdir, &fullpath, &freepath);
        sbuf_printf(sb, "%s", fullpath);
        if (freepath)
---snip---



Other functions may be similarly broken, eg. linprocfs_doprocroot() also looks
affected.

Adding author and reviewer to CC.

-- 
You are receiving this mail because:
You are the assignee for the bug.