Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 19 Aug 2020 13:26:13 -0400
From:      Shawn Webb <shawn.webb@hardenedbsd.org>
To:        Warner Losh <imp@FreeBSD.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r364402 - head/sys/kern
Message-ID:  <20200819172613.vdyutsn6a4w5fbqr@mutt-hbsd>
In-Reply-To: <202008191710.07JHA5Rk008764@repo.freebsd.org>
References:  <202008191710.07JHA5Rk008764@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--wvgkifyvole67pl3
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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
>=20
> Log:
>   Add VFS FS events for mount and unmount to devctl/devd
>  =20
>   Report when a filesystem is mounted, remounted or unmounted via devd, a=
long with
>   details about the mount point and mount options.
>  =20
>   Discussed with:	kib@
>   Reviewed by: kirk@ (prior version)
>   Sponsored by: Netflix
>   Diffential Revision: https://reviews.freebsd.org/D25969
>=20
> Modified:
>   head/sys/kern/vfs_mount.c
>=20
> 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	(r364401)
> +++ head/sys/kern/vfs_mount.c	Wed Aug 19 17:10:04 2020	(r364402)
> @@ -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);
> =20
> +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;
> =20
> +	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 threa=
d *
>  	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, ...)
> =20
>  	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 vfsoptlis=
t *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.

> +	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?

Thanks,

--=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

--wvgkifyvole67pl3
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEA6TL67gupaZ9nzhT/y5nonf44foFAl89YLMACgkQ/y5nonf4
4fqzKRAAn3OmJQmJXPPTrgG1i0pAgikhacZzJtB1yUUrsCnCYoWZVyvdthe3Ax9V
176AHgIMth/mbU8OgBmoQEc4wtQ1wEJnbH/I3x1jr0SaakMxWe5hI+4pfxKpm996
pYjSA5K6wrdHG0Q3mB7HLUyjRplqyJbPIaSlkt+vD9932nCMCApP/9D5fmTp/LmB
jzQqFSWi6ZR9OHnZnUq6iMzy4cJT9vUUpNMUGn9cqIZ8aJkVlxgr3BWNchCotipj
tPFc7Cq41VoChfKPk/uJV9allHawzColoATxf/2pF5rcP62l7RyoTnLdzxVF3VkP
IQlhapIPd6kLNP8/EJyLrtVIubPzChOxog3XAps+KBNrdVV0GkDR82qBPF0DMJW7
XoiW9b7Jz6DPjNoxZarN82g0dua0SVcw6cX9hD6hg2AA2Rnqkuzf6Y/fiGN5VWrs
yuaxXJfR/cCBmb/doGMmunuBTofPjr1tBeUlAgEr3mXxeT1sZi6KbEsdw6BUZd5B
wRKrm2b/LcaB7UD0ItTB0ypeV4ls7EMgxd1/frG5vY6gLSg1f4mM10ijChDyT4P0
LPG12C7IBodEyPiA3XBpOjrPek9uJx5VvwZBMskvU6PUtheGG3fEmJAnrDmQg1XF
ecXbm9lRoZihAGWR4CTiRzmssemSGAudRbw97aDrS5nyPH7des8=
=mvvo
-----END PGP SIGNATURE-----

--wvgkifyvole67pl3--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20200819172613.vdyutsn6a4w5fbqr>