svn commit: r337887 - head/usr.sbin/bhyve
Rodney W. Grimes
freebsd at pdx.rh.CN85.dnsmgr.net
Thu Aug 16 14:34:19 UTC 2018
> 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?
> Keep in mind, though, that ATA is 100% irrelevant to NVMe,
> since the NVMe standard specifies everything.
I was using that as a case that has been seen where
an assumption about there always being a null in the SN
would be certain that strings are null terminated,
not saying that ATA applied to NVMe.
>
> I've sent a followup to marcelo though about the 8-bit and NUL details,
> however, since I have conflicting info about that.
>
> Warner
--
Rod Grimes rgrimes at freebsd.org
More information about the svn-src-all
mailing list