svn commit: r337887 - head/usr.sbin/bhyve

[Warner Losh]

On Thu, Aug 16, 2018 at 8:34 AM, Rodney W. Grimes <
freebsd at pdx.rh.cn85.dnsmgr.net> wrote:

> > On Thu, Aug 16, 2018 at 8:03 AM, Rodney W. Grimes <
> > freebsd at pdx.rh.cn85.dnsmgr.net> wrote:
> >
> > > > Author: araujo
> > > > Date: Thu Aug 16 06:31:54 2018
> > > > New Revision: 337887
> > > > URL: https://svnweb.freebsd.org/changeset/base/337887
> > > >
> > > > Log:
> > > >   Add a comment explaining how the PSN works and why there is no
> need for
> > > >   a null terminator. Also mark CID 1394825 as intentional.
> > > >
> > > >   Reported by:        Coverity
> > > >   CID:                1394825
> > > >   MFC after:  1 week
> > > >   Sponsored by:       iXsystems Inc.
> > > >
> > > > Modified:
> > > >   head/usr.sbin/bhyve/pci_nvme.c
> > > >
> > > > Modified: head/usr.sbin/bhyve/pci_nvme.c
> > > > ============================================================
> > > ==================
> > > > --- head/usr.sbin/bhyve/pci_nvme.c    Thu Aug 16 06:20:25 2018
> > > (r337886)
> > > > +++ head/usr.sbin/bhyve/pci_nvme.c    Thu Aug 16 06:31:54 2018
> > > (r337887)
> > > > @@ -1714,6 +1714,11 @@ pci_nvme_parse_opts(struct pci_nvme_softc *sc,
> > > char *o
> > > >               } else if (!strcmp("sectsz", xopts)) {
> > > >                       sectsz = atoi(config);
> > > >               } else if (!strcmp("ser", xopts)) {
> > > > +                     /*
> > > > +                      * This field indicates the Product Serial
> Number
> > > in
> > > > +                      * 8-bit ASCII, unused bytes should be NULL
> > > characters.
> > > > +                      * Ref: NVM Express Management Interface 1.0a.
> > > > +                      */
> > >
> > > I have seen this before on ATA devices,
> > > if the vendor fills all bytes of PSN,
> > > there well be no unused bytes,
> > > so no null byte at the end,
> > > and you end up with an unterminated string.
> > >
> > > Can you please verify that this edge case is handled correctly?
> > > Thanks,
> > > Rod
> > >
> > > >                       memset(sc->ctrldata.sn, 0, sizeof(sc->
> ctrldata.sn
> > > ));
> > > >                       strncpy(sc->ctrldata.sn, config,
> > > >                               sizeof(sc->ctrldata.sn));
> > > >
> > >
> >
> > strncpy will not NUL terminate when there's exactly sizeof(ctrldata.sn)
> > bytes in the 'config' string. Thus that case where all characters are
> > non-NUL is handled properly (the standard says the string need not be NUL
> > terminated).
>
> I get that, are we certain that all consumers of ctrldata.sn
> obey this, ie it is never attempted to print this string
> with a %s?


Grep says "Yes."

Warner