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

[Warner Losh]

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). Keep in mind, though, that ATA is 100% irrelevant to NVMe,
since the NVMe standard specifies everything.

I've sent a followup to marcelo though about the 8-bit and NUL details,
however, since I have conflicting info about that.

Warner