Date: Wed, 19 Aug 2020 13:48:08 -0400 From: Shawn Webb <shawn.webb@hardenedbsd.org> To: Warner Losh <imp@bsdimp.com> Cc: Warner Losh <imp@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org> Subject: Re: svn commit: r364402 - head/sys/kern Message-ID: <20200819174808.ts3i72vzqsx3sq5o@mutt-hbsd> In-Reply-To: <CANCZdfqg=83f1mqA=ZH1qVqnd7dus09_Nbx2vX98_oK4MoTk1A@mail.gmail.com> References: <202008191710.07JHA5Rk008764@repo.freebsd.org> <20200819172613.vdyutsn6a4w5fbqr@mutt-hbsd> <CANCZdfqg=83f1mqA=ZH1qVqnd7dus09_Nbx2vX98_oK4MoTk1A@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--fir3ktbk4os5uyld Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 19, 2020 at 11:44:42AM -0600, Warner Losh wrote: > On Wed, Aug 19, 2020 at 11:26 AM Shawn Webb <shawn.webb@hardenedbsd.org> > wrote: >=20 > > On Wed, Aug 19, 2020 at 05:10:05PM +0000, Warner Losh wrote: > > > Author: imp > > > Date: Wed Aug 19 17:10:04 2020 > > > New Revision: 364402 > > > URL: https://svnweb.freebsd.org/changeset/base/364402 > > > > > > Log: > > > Add VFS FS events for mount and unmount to devctl/devd > > > > > > Report when a filesystem is mounted, remounted or unmounted via dev= d, > > along with > > > details about the mount point and mount options. > > > > > > Discussed with: kib@ > > > Reviewed by: kirk@ (prior version) > > > Sponsored by: Netflix > > > Diffential Revision: https://reviews.freebsd.org/D25969 > > > > > > Modified: > > > head/sys/kern/vfs_mount.c > > > > > > Modified: head/sys/kern/vfs_mount.c > > > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D > > > --- head/sys/kern/vfs_mount.c Wed Aug 19 17:09:58 2020 (r36440= 1) > > > +++ head/sys/kern/vfs_mount.c Wed Aug 19 17:10:04 2020 (r36440= 2) > > > @@ -42,6 +42,7 @@ __FBSDID("$FreeBSD$"); > > > #include <sys/param.h> > > > #include <sys/conf.h> > > > #include <sys/smp.h> > > > +#include <sys/bus.h> > > > #include <sys/eventhandler.h> > > > #include <sys/fcntl.h> > > > #include <sys/jail.h> > > > @@ -101,6 +102,8 @@ MTX_SYSINIT(mountlist, &mountlist_mtx, "mountlist= ", > > MT > > > EVENTHANDLER_LIST_DEFINE(vfs_mounted); > > > EVENTHANDLER_LIST_DEFINE(vfs_unmounted); > > > > > > +static void dev_vfs_event(const char *type, struct mount *mp, bool > > donew); > > > + > > > /* > > > * Global opts, taken by all filesystems > > > */ > > > @@ -1020,6 +1023,7 @@ vfs_domount_first( > > > VOP_UNLOCK(vp); > > > EVENTHANDLER_DIRECT_INVOKE(vfs_mounted, mp, newdp, td); > > > VOP_UNLOCK(newdp); > > > + dev_vfs_event("MOUNT", mp, false); > > > mountcheckdirs(vp, newdp); > > > vn_seqc_write_end(vp); > > > vn_seqc_write_end(newdp); > > > @@ -1221,6 +1225,7 @@ vfs_domount_update( > > > if (error !=3D 0) > > > goto end; > > > > > > + dev_vfs_event("REMOUNT", mp, true); > > > if (mp->mnt_opt !=3D NULL) > > > vfs_freeopts(mp->mnt_opt); > > > mp->mnt_opt =3D mp->mnt_optnew; > > > @@ -1839,6 +1844,7 @@ dounmount(struct mount *mp, int flags, struct > > thread * > > > TAILQ_REMOVE(&mountlist, mp, mnt_list); > > > mtx_unlock(&mountlist_mtx); > > > EVENTHANDLER_DIRECT_INVOKE(vfs_unmounted, mp, td); > > > + dev_vfs_event("UNMOUNT", mp, false); > > > if (coveredvp !=3D NULL) { > > > coveredvp->v_mountedhere =3D NULL; > > > vn_seqc_write_end(coveredvp); > > > @@ -2425,4 +2431,72 @@ kernel_vmount(int flags, ...) > > > > > > error =3D kernel_mount(ma, flags); > > > return (error); > > > +} > > > + > > > +/* Map from mount options to printable formats. */ > > > +static struct mntoptnames optnames[] =3D { > > > + MNTOPT_NAMES > > > +}; > > > + > > > +static void > > > +dev_vfs_event_mntopt(struct sbuf *sb, const char *what, struct > > vfsoptlist *opts) > > > +{ > > > + struct vfsopt *opt; > > > + > > > + if (opts =3D=3D NULL || TAILQ_EMPTY(opts)) > > > + return; > > > + sbuf_printf(sb, " %s=3D\"", what); > > > + TAILQ_FOREACH(opt, opts, link) { > > > + if (opt->name[0] =3D=3D '\0' || (opt->len > 0 && *(char > > *)opt->value =3D=3D '\0')) > > > + continue; > > > + devctl_safe_quote_sb(sb, opt->name); > > > + if (opt->len > 0) { > > > + sbuf_putc(sb, '=3D'); > > > + devctl_safe_quote_sb(sb, opt->value); > > > + } > > > + sbuf_putc(sb, ';'); > > > + } > > > + sbuf_putc(sb, '"'); > > > +} > > > + > > > +#define DEVCTL_LEN 1024 > > > +static void > > > +dev_vfs_event(const char *type, struct mount *mp, bool donew) > > > +{ > > > + const uint8_t *cp; > > > + struct mntoptnames *fp; > > > + struct sbuf sb; > > > + struct statfs *sfp =3D &mp->mnt_stat; > > > + char *buf; > > > + > > > + buf =3D malloc(DEVCTL_LEN, M_MOUNT, M_WAITOK); > > > + if (buf =3D=3D NULL) > > > + return; > > > > buf can't be NULL. > > >=20 > The bug here is that M_NOWAIT should have been specified in the malloc. >=20 >=20 > > > + sbuf_new(&sb, buf, DEVCTL_LEN, SBUF_FIXEDLEN); > > > + sbuf_cpy(&sb, "mount-point=3D\""); > > > + devctl_safe_quote_sb(&sb, sfp->f_mntonname); > > > + sbuf_cat(&sb, "\" mount-dev=3D\""); > > > + devctl_safe_quote_sb(&sb, sfp->f_mntfromname); > > > + sbuf_cat(&sb, "\" mount-type=3D\""); > > > + devctl_safe_quote_sb(&sb, sfp->f_fstypename); > > > + sbuf_cat(&sb, "\" fsid=3D0x"); > > > + cp =3D (const uint8_t *)&sfp->f_fsid.val[0]; > > > + for (int i =3D 0; i < sizeof(sfp->f_fsid); i++) > > > + sbuf_printf(&sb, "%02x", cp[i]); > > > + sbuf_printf(&sb, " owner=3D%u flags=3D\"", sfp->f_owner); > > > + for (fp =3D optnames; fp->o_opt !=3D 0; fp++) { > > > + if ((mp->mnt_flag & fp->o_opt) !=3D 0) { > > > + sbuf_cat(&sb, fp->o_name); > > > + sbuf_putc(&sb, ';'); > > > + } > > > + } > > > + sbuf_putc(&sb, '"'); > > > + dev_vfs_event_mntopt(&sb, "opt", mp->mnt_opt); > > > + if (donew) > > > + dev_vfs_event_mntopt(&sb, "optnew", mp->mnt_optnew); > > > + sbuf_finish(&sb); > > > + > > > + devctl_notify("VFS", "FS", type, sbuf_data(&sb)); > > > + sbuf_delete(&sb); > > > + free(buf, M_MOUNT); > > > } > > > > I don't really see much attention paid to checking for sbuf overflow. > > Could that cause issues, especially in case of impartial quotation > > termination? Could not performing those checks have security > > implications? Would performing those checks adhere to good code > > development practices? > > >=20 > sbuf doesn't overflow. It is safe from that perspective. The code should > just not send it if there's an overflow... There almost certainly won't = be > one in practice given the buffer size, though. You're right that sbuf can't overflow. However, assuming that it hasn't reached its fixed size specified above and continuing on as if there's still space left could lead to... interesting behavior. --=20 Shawn Webb Cofounder / Security Engineer HardenedBSD GPG Key ID: 0xFF2E67A277F8E1FA GPG Key Fingerprint: D206 BB45 15E0 9C49 0CF9 3633 C85B 0AF8 AB23 0FB2 https://git-01.md.hardenedbsd.org/HardenedBSD/pubkeys/src/branch/master/Sha= wn_Webb/03A4CBEBB82EA5A67D9F3853FF2E67A277F8E1FA.pub.asc --fir3ktbk4os5uyld Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEA6TL67gupaZ9nzhT/y5nonf44foFAl89ZdUACgkQ/y5nonf4 4fraYg//QP4wBsv2QlncpKm+pPOrsZ9CED/Ig3gn57bySS6r5kyj79MkOhuQ5W3k 3mweUsth08K2ikSUm4zPzEuy9an/nsm3g3vp6iTmOD9XyirPqB6gxTaSZp2yAnuQ stvW3pHTPyPuWLlEHdDVv9VA/OIGT7hRHEPj92/tinjnUEkLyqqeOE2rQBtHudyj 37ytQUsz7Je4qyDxPY+p2D6U8jVfgB7xQp/F15Ap3GQOI6rpL+CN96wq5m5Mkzgh p/Jd7+EkbEjCzhyP5/tPbsxaBQU4GUWwKW5d1Q+iIOnMbbMadEUXD49/Ro55QUcg 3RRV6pdMXw9jz8u3b8/RRtTgBMP8tgR3KbEo/iL+oteFImMWyl334iikUWXBthjL Jkhpa0GwKJM9kdwl8VX7Cnh/P0izTyjBtuU9mLPsXnO/dUXcaswgAkzFDEfU4GRx d17QY8JTSubX2f/hKWzeAH4Mnr8v+44PvwFkywDfUmc+tsoxvp/MojdyMQaLotDz kyObsqYKNH5juKWb6fVx/0iJXqvC1GvwUo43AbIHuWTwl47xcZoWUGYn864ADumI PWUnOyOKrDyS5IvxQgbk2Dz58Ged0Acb3AbygjuAJ1FWTvLrjxpgcD907CEaTtgq CeuJTxTROiInCtJXiHuvnSxrrfJQn4rsiSdZh8ST+3hfulF8Kzw= =YLxR -----END PGP SIGNATURE----- --fir3ktbk4os5uyld--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20200819174808.ts3i72vzqsx3sq5o>