svn commit: r295812 - projects/mips64-clang/sys/mips/rmi

[Benjamin Kaduk]

On Sun, Feb 21, 2016 at 8:47 AM, Ian Lepore <ian at freebsd.org> wrote:

> On Fri, 2016-02-19 at 16:37 +0000, Sean Bruno wrote:
> > Author: sbruno
> > Date: Fri Feb 19 16:37:06 2016
> > New Revision: 295812
> > URL: https://svnweb.freebsd.org/changeset/base/295812
> >
> > Log:
> >   Change a static const string to a #define as the strcpy() throws a
> >   warn/error with clang.
> >
> >   /home/sbruno/mips64-clang/sys/mips/rmi/xls_ehci.c:133:25: error:
> >   format string is not a string literal (potentially insecure)
> >       [-Werror,-Wformat-security]
> >           sprintf(sc->sc_vendor, xlr_vendor_desc);
> >
> > Modified:
> >   projects/mips64-clang/sys/mips/rmi/xls_ehci.c
> >
> > Modified: projects/mips64-clang/sys/mips/rmi/xls_ehci.c
> > =====================================================================
> > =========
> > --- projects/mips64-clang/sys/mips/rmi/xls_ehci.c     Fri Feb 19
> > 15:53:08 2016 (r295811)
> > +++ projects/mips64-clang/sys/mips/rmi/xls_ehci.c     Fri Feb 19
> > 16:37:06 2016 (r295812)
> > @@ -73,7 +73,7 @@ static device_attach_t ehci_xls_attach;
> >  static device_detach_t ehci_xls_detach;
> >
> >  static const char *xlr_usb_dev_desc = "RMI XLR USB 2.0 controller";
> > -static const char *xlr_vendor_desc = "RMI Corp";
> > +#define XLR_VENDOR_DESC "RMI Corp";
> >
> >  static int
> >  ehci_xls_probe(device_t self)
> > @@ -130,7 +130,7 @@ ehci_xls_attach(device_t self)
> >       device_set_ivars(sc->sc_bus.bdev, &sc->sc_bus);
> >       device_set_desc(sc->sc_bus.bdev, xlr_usb_dev_desc);
> >
> > -     sprintf(sc->sc_vendor, xlr_vendor_desc);
> > +     sprintf(sc->sc_vendor, XLR_VENDOR_DESC);
> >
> >       err = bus_setup_intr(self, sc->sc_irq_res,
> >           INTR_TYPE_BIO | INTR_MPSAFE, NULL,
> >
>
> Bah.  The compiler should understand that a static const char* is
> equivelent to a string literal for the purposes of this warning.
>
>
Is it?  The compiler would need to check that nothing else in the file
writes to xlr_vendor_desc before making that conclusion; on the other hand,
if it was char const * const, then that alone would suffice.


> That said, a sprintf() is just a strange spelling of strlcpy() here.
>

Almost.  sprintf() is not as good about length checking as strlcpy(), which
is a much better option here, as you note.

-Ben


>
> -- Ian
>
> _______________________________________________
> svn-src-projects at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/svn-src-projects
> To unsubscribe, send any mail to "svn-src-projects-unsubscribe at freebsd.org
> "
>