From owner-svn-src-all@freebsd.org Wed Aug 19 17:48:11 2020 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id A64743C59E2 for ; Wed, 19 Aug 2020 17:48:11 +0000 (UTC) (envelope-from shawn.webb@hardenedbsd.org) Received: from mail-qv1-xf44.google.com (mail-qv1-xf44.google.com [IPv6:2607:f8b0:4864:20::f44]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BWwHH1Mgvz4Lvx for ; Wed, 19 Aug 2020 17:48:10 +0000 (UTC) (envelope-from shawn.webb@hardenedbsd.org) Received: by mail-qv1-xf44.google.com with SMTP id y11so11675885qvl.4 for ; Wed, 19 Aug 2020 10:48:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=hardenedbsd.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=C9JnkyYAHLWKrYzBwzZwmnP23jaZj4tDM0XSErSklGo=; b=jZmMKIgLRyLHh6pBxKy9t2Ic/55uvni+DeGhm3eOkqHFjjjMgfc7ne4ouoNiKHUdTj FCGI+HdGGiV3JUmTwOEPY+x5ZOqFXaDhzEuWMP5RkEP86Xa38i3WZCU9v4dMxry9kwOP vXdPj1PiTaZZDkLWr0EOubrOvfG0CqXxFNNIYtCIGLflSlZ8hxqWom2VgRzK8QwqvYyK Qv6BsDBJ6h4sCnz0YPQjpfEQk58TROoil7epSS9dQoZMZJncm47xXlFe/ioBZGBIDiiQ Ahy3vSCS4b9tg52mfUBzKObInI26Ty8sbVyEByOWe/JZuHG4tKrAM2YKcPiRhxRZbBev tzbg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=C9JnkyYAHLWKrYzBwzZwmnP23jaZj4tDM0XSErSklGo=; b=Xb2JQ2ARVHrZcNloZmbP2mC3jbzPk007u8bDOSZtWj5SxOQ/qoxS+LhiBfURxcoi9F Cv9ZxF/DfXTlZMGFtb/fjl/IhihRVm8px66QRY84KmJOFwwdyJz/zgs7ROLYuYg1KH1J nREiglF2Kb8P8G0zJqNrDBxDIk6RcmrGJEqBhKt/5UtGa93x3cZ2y164tUqofLImCPJJ zh8P2hAJLbjqjt+3LXD37ylBRdXIhauERmVmYcdsaH18WOLaxOPYdWnjpfuGFKxqSUQo /h4vZIAjwytFAl4fZjTBrxOZL+Kx8EJpRXxT8a9+roqgglm124g+DPZys3Cb2D8sZKWW GlDw== X-Gm-Message-State: AOAM531C76OEV7eBLvqXJEZs6iI7cIgIQ4HZCZsmVhl/RQr2mdvlfdG7 6WgPhS35FFj/Vf17EyoTdcrb8Q== X-Google-Smtp-Source: ABdhPJz5S/ybNBXCovbNcRTlTeoUxJxLUkDbbeH1I8HL8vsvPYNSN3674eD3JDgtc2fySmzWx/Yy4A== X-Received: by 2002:a0c:bd8d:: with SMTP id n13mr24676201qvg.199.1597859289350; Wed, 19 Aug 2020 10:48:09 -0700 (PDT) Received: from mutt-hbsd (pool-100-16-231-224.bltmmd.fios.verizon.net. [100.16.231.224]) by smtp.gmail.com with ESMTPSA id p34sm30561620qte.79.2020.08.19.10.48.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 19 Aug 2020 10:48:08 -0700 (PDT) Date: Wed, 19 Aug 2020 13:48:08 -0400 From: Shawn Webb To: Warner Losh Cc: Warner Losh , src-committers , svn-src-all , svn-src-head Subject: Re: svn commit: r364402 - head/sys/kern Message-ID: <20200819174808.ts3i72vzqsx3sq5o@mutt-hbsd> X-Operating-System: FreeBSD mutt-hbsd 13.0-CURRENT-HBSD FreeBSD 13.0-CURRENT-HBSD X-PGP-Key: http://pgp.mit.edu/pks/lookup?op=vindex&search=0xFF2E67A277F8E1FA References: <202008191710.07JHA5Rk008764@repo.freebsd.org> <20200819172613.vdyutsn6a4w5fbqr@mutt-hbsd> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="fir3ktbk4os5uyld" Content-Disposition: inline In-Reply-To: X-Rspamd-Queue-Id: 4BWwHH1Mgvz4Lvx X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=hardenedbsd.org header.s=google header.b=jZmMKIgL; dmarc=none; spf=pass (mx1.freebsd.org: domain of shawn.webb@hardenedbsd.org designates 2607:f8b0:4864:20::f44 as permitted sender) smtp.mailfrom=shawn.webb@hardenedbsd.org X-Spamd-Result: default: False [-2.33 / 15.00]; RCVD_VIA_SMTP_AUTH(0.00)[]; ARC_NA(0.00)[]; R_DKIM_ALLOW(-0.20)[hardenedbsd.org:s=google]; FROM_HAS_DN(0.00)[]; R_SPF_ALLOW(-0.20)[+ip6:2607:f8b0:4000::/36]; MIME_GOOD(-0.20)[multipart/signed,text/plain]; PREVIOUSLY_DELIVERED(0.00)[svn-src-all@freebsd.org]; DMARC_NA(0.00)[hardenedbsd.org]; RCPT_COUNT_FIVE(0.00)[5]; RCVD_COUNT_THREE(0.00)[3]; TO_MATCH_ENVRCPT_SOME(0.00)[]; TO_DN_ALL(0.00)[]; DKIM_TRACE(0.00)[hardenedbsd.org:+]; NEURAL_HAM_SHORT(-0.23)[-0.233]; RCVD_IN_DNSWL_NONE(0.00)[2607:f8b0:4864:20::f44:from]; SIGNED_PGP(-2.00)[]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+,1:+,2:~]; MID_RHS_NOT_FQDN(0.50)[]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; RCVD_TLS_ALL(0.00)[]; MAILMAN_DEST(0.00)[svn-src-all]; RECEIVED_SPAMHAUS_PBL(0.00)[100.16.231.224:received] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Aug 2020 17:48:11 -0000 --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 > 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 > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -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--