Re: kern/45700: /chroot/proc/mounts exposes out-of-chroot pathnames

[Taylor R Campbell <campbell+netbsd%mumble.net@localhost>]

The following reply was made to PR kern/45700; it has been noted by GNATS.

From: Taylor R Campbell <campbell+netbsd%mumble.net@localhost>
To: matthew green <mrg%eterna.com.au@localhost>
Cc: gnats-bugs%NetBSD.org@localhost, kern-bug-people%netbsd.org@localhost, 
gnats-admin%netbsd.org@localhost,
        netbsd-bugs%netbsd.org@localhost
Subject: Re: kern/45700: /chroot/proc/mounts exposes out-of-chroot pathnames
Date: Sun, 11 Dec 2011 00:13:28 +0000

    Date: Fri, 9 Dec 2011 09:54:36 +0000
    From: Taylor R Campbell <campbell%mumble.net@localhost>
 
    Might work just to make procfs_domounts use dostatvfs rather than
    reading from mp->mnt_stat directly.  I'll look into this at some point
    if nobody beats me to it.
 
 The following patch seems to almost work.  It doesn't fake an entry
 for the root file system if appropriate.  Not sure why not, but I
 don't want to reboot again at the moment to futz with it; I'll peer
 closer at it later.
 
 Index: procfs_linux.c
 ===================================================================
 RCS file: /cvsroot/src/sys/miscfs/procfs/procfs_linux.c,v
 retrieving revision 1.61
 diff -p -u -r1.61 procfs_linux.c
 --- procfs_linux.c     4 Sep 2011 17:32:10 -0000       1.61
 +++ procfs_linux.c     11 Dec 2011 00:07:23 -0000
 @@ -54,6 +54,7 @@ __KERNEL_RCSID(0, "$NetBSD: procfs_linux
  #include <sys/mount.h>
  #include <sys/conf.h>
  #include <sys/sysctl.h>
 +#include <sys/filedesc.h>
  
  #include <miscfs/procfs/procfs.h>
  
 @@ -556,53 +557,81 @@ out:
        return error;
  }
  
 +
 +/*
 + * Append a formatted mount entry for the mount point mp to *mtab
 + * starting at *mtabsz, growing it and adding the length of the entry
 + * to *mtabsz.  sfs is mp's vfs statistics structure; bf is a temporary
 + * buffer of size LBFSZ.
 + */
 +static void
 +procfs_format_mount(struct mount *mp, struct statvfs *sfs, char *bf,
 +    char **mtab, size_t *mtabsz)
 +{
 +      const char *fsname;
 +      size_t len;
 +
 +      /* Linux uses different names for some filesystems.  */
 +      fsname = sfs->f_fstypename;
 +      if (strcmp(fsname, "procfs") == 0)
 +              fsname = "proc";
 +      else if (strcmp(fsname, "ext2fs") == 0)
 +              fsname = "ext2";
 +
 +      len = snprintf(bf, LBFSZ, "%s %s %s %s%s%s%s%s%s 0 0\n",
 +          sfs->f_mntfromname,
 +          sfs->f_mntonname,
 +          fsname,
 +          (mp->mnt_flag & MNT_RDONLY) ? "ro" : "rw",
 +          (mp->mnt_flag & MNT_NOSUID) ? ",nosuid" : "",
 +          (mp->mnt_flag & MNT_NOEXEC) ? ",noexec" : "",
 +          (mp->mnt_flag & MNT_NODEV) ? ",nodev" : "",
 +          (mp->mnt_flag & MNT_SYNCHRONOUS) ? ",sync" : "",
 +          (mp->mnt_flag & MNT_NOATIME) ? ",noatime" : "");
 +
 +      *mtab = realloc(*mtab, *mtabsz + len, M_TEMP, M_WAITOK);
 +      memcpy(*mtab + *mtabsz, bf, len);
 +      *mtabsz += len;
 +}
 +
  int
  procfs_domounts(struct lwp *curl, struct proc *p,
      struct pfsnode *pfs, struct uio *uio)
  {
        char *bf, *mtab = NULL;
 -      const char *fsname;
 -      size_t len, mtabsz = 0;
 +      size_t mtabsz = 0;
        struct mount *mp, *nmp;
 -      struct statvfs *sfs;
 +      struct statvfs sfs;
        int error = 0;
 +      bool root = false;
  
        bf = malloc(LBFSZ, M_TEMP, M_WAITOK);
 +
        mutex_enter(&mountlist_lock);
        for (mp = CIRCLEQ_FIRST(&mountlist); mp != (void *)&mountlist;
             mp = nmp) {
 -              if (vfs_busy(mp, &nmp)) {
 +              if (vfs_busy(mp, &nmp))
                        continue;
 -              }
 -
 -              sfs = &mp->mnt_stat;
  
 -              /* Linux uses different names for some filesystems */
 -              fsname = sfs->f_fstypename;
 -              if (strcmp(fsname, "procfs") == 0)
 -                      fsname = "proc";
 -              else if (strcmp(fsname, "ext2fs") == 0)
 -                      fsname = "ext2";
 -
 -              len = snprintf(bf, LBFSZ, "%s %s %s %s%s%s%s%s%s 0 0\n",
 -                      sfs->f_mntfromname,
 -                      sfs->f_mntonname,
 -                      fsname,
 -                      (mp->mnt_flag & MNT_RDONLY) ? "ro" : "rw",
 -                      (mp->mnt_flag & MNT_NOSUID) ? ",nosuid" : "",
 -                      (mp->mnt_flag & MNT_NOEXEC) ? ",noexec" : "",
 -                      (mp->mnt_flag & MNT_NODEV) ? ",nodev" : "",
 -                      (mp->mnt_flag & MNT_SYNCHRONOUS) ? ",sync" : "",
 -                      (mp->mnt_flag & MNT_NOATIME) ? ",noatime" : ""
 -                      );
 -
 -              mtab = realloc(mtab, mtabsz + len, M_TEMP, M_WAITOK);
 -              memcpy(mtab + mtabsz, bf, len);
 -              mtabsz += len;
 +              if (dostatvfs(mp, &sfs, curl, MNT_NOWAIT, 0) == 0) {
 +                      procfs_format_mount(mp, &sfs, bf, &mtab, &mtabsz);
 +                      root |= (strcmp(sfs.f_mntonname, "/") == 0);
 +              }
  
                vfs_unbusy(mp, false, &nmp);
        }
        mutex_exit(&mountlist_lock);
 +
 +      /*
 +       * If we are inside a chroot that is not itself a mount point,
 +       * fake a root entry.
 +       */
 +      if (!root && p->p_cwdi->cwdi_rdir) {
 +              mp = p->p_cwdi->cwdi_rdir->v_mount;
 +              if (dostatvfs(mp, &sfs, curl, MNT_NOWAIT, 1) == 0)
 +                      procfs_format_mount(mp, &sfs, bf, &mtab, &mtabsz);
 +      }
 +
        free(bf, M_TEMP);
  
        if (mtabsz > 0) {