From owner-svn-src-head@freebsd.org Wed Aug 19 17:44:56 2020 Return-Path: Delivered-To: svn-src-head@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 086803C5A0F for ; Wed, 19 Aug 2020 17:44:56 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qt1-x82f.google.com (mail-qt1-x82f.google.com [IPv6:2607:f8b0:4864:20::82f]) (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 4BWwCV54hSz4Ljh for ; Wed, 19 Aug 2020 17:44:54 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qt1-x82f.google.com with SMTP id c12so18441546qtn.9 for ; Wed, 19 Aug 2020 10:44:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Md1BhQpmVgkUEi5BR5XyiF3pOMGWzfdGGUFv2GvQAeo=; b=I+EEvGfvoBw5VXgoto2mDjlMxJihcReF/IAVVoEZNRJfmhmTu+PAiHct4Y85zKTAzn KxEYMMv7W8xJu6BZTGuz7lw4LnTETE2bvA+krh+secvCfnF8dn/jHY5JIGX/DqR72gHi P1SAPLLKJJ+4CoWZ9Y8aWf/+EaGU0aSIxEbhX1ZqYlo8wzf7QLppbSTZOnkaYZfEQwAO q5x06mfp+XCGSSQ/xOnzhxvDNYJmnPsI0IAPqDfSHvSwj2/5QP+/l/b65R7i5NxYTAOv wLkNSg3Gd7BXSZZ+iR03IkYmeP+F4Ub5L6yOF84iTjUtPbDkb+3k91rQUglEqXBZIy/6 NFFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Md1BhQpmVgkUEi5BR5XyiF3pOMGWzfdGGUFv2GvQAeo=; b=HuEWn7UeH3VxzBrhwynLFBWztFtlO+PW9NwPpuHaweE9HyQgdZol/Du2Nt6RuKy7N1 yeExtaw9F8fZIBIslgThzHFzGBQqhBvbCGPd/xUesrFChDbxxXvVGOkUoPjwu0H1Ii1o 6KVa5eJHWALuz8ETbOfegHsY4wWZuVjRk4WOLiBtytcyY9fxlT5TGFgrn/EVU0WnBjYM XlgmVNoB8ELgxS5Lk4Pg/qumWW4+HRZct+tLyqIVsEnuTUumaygAs327Bc8ujku+DZXd 76kuk1zgxqoYXWbrSupihqDahvG8GZRAoJ/IyqxfxHOECsesxB32Dqys6y551AtIKLAi OpPA== X-Gm-Message-State: AOAM533LwoC1NMFms9321xdFMMf65evIxU8ucIAAr0vWt4k4HpT7pZKs kDdfUS3HHtF3WAXKZkKf1RntHpdz0o2wcebwcDKlkw== X-Google-Smtp-Source: ABdhPJwzrShB2wcKJ1kw+TPrNH+11wYrzDBLAVgbvwz8n0YpOMxwpCQBPPxcyYQQstiQonlty3fhQp4zAw6V2su3IeE= X-Received: by 2002:ac8:50a:: with SMTP id u10mr23277766qtg.175.1597859093340; Wed, 19 Aug 2020 10:44:53 -0700 (PDT) MIME-Version: 1.0 References: <202008191710.07JHA5Rk008764@repo.freebsd.org> <20200819172613.vdyutsn6a4w5fbqr@mutt-hbsd> In-Reply-To: <20200819172613.vdyutsn6a4w5fbqr@mutt-hbsd> From: Warner Losh Date: Wed, 19 Aug 2020 11:44:42 -0600 Message-ID: Subject: Re: svn commit: r364402 - head/sys/kern To: Shawn Webb Cc: Warner Losh , src-committers , svn-src-all , svn-src-head X-Rspamd-Queue-Id: 4BWwCV54hSz4Ljh X-Spamd-Bar: / Authentication-Results: mx1.freebsd.org; dkim=pass header.d=bsdimp-com.20150623.gappssmtp.com header.s=20150623 header.b=I+EEvGfv; dmarc=none; spf=none (mx1.freebsd.org: domain of wlosh@bsdimp.com has no SPF policy when checking 2607:f8b0:4864:20::82f) smtp.mailfrom=wlosh@bsdimp.com X-Spamd-Result: default: False [-0.01 / 15.00]; RCVD_TLS_ALL(0.00)[]; ARC_NA(0.00)[]; R_DKIM_ALLOW(-0.20)[bsdimp-com.20150623.gappssmtp.com:s=20150623]; FROM_HAS_DN(0.00)[]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; PREVIOUSLY_DELIVERED(0.00)[svn-src-head@freebsd.org]; DMARC_NA(0.00)[bsdimp.com]; RCPT_COUNT_FIVE(0.00)[5]; TO_MATCH_ENVRCPT_SOME(0.00)[]; TO_DN_ALL(0.00)[]; DKIM_TRACE(0.00)[bsdimp-com.20150623.gappssmtp.com:+]; NEURAL_HAM_SHORT(-0.01)[-0.012]; RCVD_IN_DNSWL_NONE(0.00)[2607:f8b0:4864:20::82f:from]; R_SPF_NA(0.00)[no SPF record]; FORGED_SENDER(0.30)[imp@bsdimp.com,wlosh@bsdimp.com]; MIME_TRACE(0.00)[0:+,1:+,2:~]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; FROM_NEQ_ENVFROM(0.00)[imp@bsdimp.com,wlosh@bsdimp.com]; MAILMAN_DEST(0.00)[svn-src-head]; RCVD_COUNT_TWO(0.00)[2] Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.33 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Aug 2020 17:44:56 -0000 On Wed, Aug 19, 2020 at 11:26 AM Shawn Webb wrote: > 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 devd, > 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 > > > ============================================================================== > > --- 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 > > #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 != 0) > > goto end; > > > > + dev_vfs_event("REMOUNT", mp, true); > > if (mp->mnt_opt != NULL) > > vfs_freeopts(mp->mnt_opt); > > mp->mnt_opt = 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 != NULL) { > > coveredvp->v_mountedhere = NULL; > > vn_seqc_write_end(coveredvp); > > @@ -2425,4 +2431,72 @@ kernel_vmount(int flags, ...) > > > > error = kernel_mount(ma, flags); > > return (error); > > +} > > + > > +/* Map from mount options to printable formats. */ > > +static struct mntoptnames optnames[] = { > > + MNTOPT_NAMES > > +}; > > + > > +static void > > +dev_vfs_event_mntopt(struct sbuf *sb, const char *what, struct > vfsoptlist *opts) > > +{ > > + struct vfsopt *opt; > > + > > + if (opts == NULL || TAILQ_EMPTY(opts)) > > + return; > > + sbuf_printf(sb, " %s=\"", what); > > + TAILQ_FOREACH(opt, opts, link) { > > + if (opt->name[0] == '\0' || (opt->len > 0 && *(char > *)opt->value == '\0')) > > + continue; > > + devctl_safe_quote_sb(sb, opt->name); > > + if (opt->len > 0) { > > + sbuf_putc(sb, '='); > > + 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 = &mp->mnt_stat; > > + char *buf; > > + > > + buf = malloc(DEVCTL_LEN, M_MOUNT, M_WAITOK); > > + if (buf == NULL) > > + return; > > buf can't be NULL. > The bug here is that M_NOWAIT should have been specified in the malloc. > > + sbuf_new(&sb, buf, DEVCTL_LEN, SBUF_FIXEDLEN); > > + sbuf_cpy(&sb, "mount-point=\""); > > + devctl_safe_quote_sb(&sb, sfp->f_mntonname); > > + sbuf_cat(&sb, "\" mount-dev=\""); > > + devctl_safe_quote_sb(&sb, sfp->f_mntfromname); > > + sbuf_cat(&sb, "\" mount-type=\""); > > + devctl_safe_quote_sb(&sb, sfp->f_fstypename); > > + sbuf_cat(&sb, "\" fsid=0x"); > > + cp = (const uint8_t *)&sfp->f_fsid.val[0]; > > + for (int i = 0; i < sizeof(sfp->f_fsid); i++) > > + sbuf_printf(&sb, "%02x", cp[i]); > > + sbuf_printf(&sb, " owner=%u flags=\"", sfp->f_owner); > > + for (fp = optnames; fp->o_opt != 0; fp++) { > > + if ((mp->mnt_flag & fp->o_opt) != 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? > 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. Warner