Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 21 Nov 2018 09:32:14 +0800
From:      Marcelo Araujo <araujobsdport@gmail.com>
To:        Shawn Webb <shawn.webb@hardenedbsd.org>
Cc:        Marcelo Araujo <araujo@freebsd.org>, src-committers <src-committers@freebsd.org>,  svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r340707 - head/usr.sbin/bhyve
Message-ID:  <CAOfEmZhpAExGajG5XhGhOMDdXvKrgt8eSL2EpGci5zhEnhdwzQ@mail.gmail.com>
In-Reply-To: <20181121002254.efitgf45bzajh5sj@mutt-hbsd>
References:  <201811202221.wAKMLJ3W068166@repo.freebsd.org> <20181121002254.efitgf45bzajh5sj@mutt-hbsd>

next in thread | previous in thread | raw e-mail | index | archive | help
Em qua, 21 de nov de 2018 =C3=A0s 08:23, Shawn Webb <shawn.webb@hardenedbsd=
.org>
escreveu:

> On Tue, Nov 20, 2018 at 10:21:19PM +0000, Marcelo Araujo wrote:
> > Author: araujo
> > Date: Tue Nov 20 22:21:19 2018
> > New Revision: 340707
> > URL: https://svnweb.freebsd.org/changeset/base/340707
> >
> > Log:
> >   Define AHCI_PORT_IDENT and increase by 1 the VTBLK_BLK_ID_BYTES
> >   to avoid buffer accessed out of bounds, also switch to snprintf(3).
> >
> >   PR:         200859
> >   Submitted by:       Caglar <caglar@10ur.org>
> >   Obtained from:      https://github.com/mist64/xhyve/pull/24
> >   MFC after:  4 weeks
> >   Sponsored by:       iXsystems Inc.
> >
> > Modified:
> >   head/usr.sbin/bhyve/pci_ahci.c
> >   head/usr.sbin/bhyve/pci_virtio_block.c
> >
> > Modified: head/usr.sbin/bhyve/pci_ahci.c
> >
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> > --- head/usr.sbin/bhyve/pci_ahci.c    Tue Nov 20 22:12:10 2018
> (r340706)
> > +++ head/usr.sbin/bhyve/pci_ahci.c    Tue Nov 20 22:21:19 2018
> (r340707)
> > @@ -105,7 +105,7 @@ enum sata_fis_type {
> >   * ATA commands
> >   */
> >  #define      ATA_SF_ENAB_SATA_SF             0x10
> > -#define              ATA_SATA_SF_AN          0x05
> > +#define      ATA_SATA_SF_AN                  0x05
> >  #define      ATA_SF_DIS_SATA_SF              0x90
> >
> >  /*
> > @@ -119,6 +119,8 @@ static FILE *dbg;
> >  #endif
> >  #define WPRINTF(format, arg...) printf(format, ##arg)
> >
> > +#define AHCI_PORT_IDENT 20 + 1
> > +
> >  struct ahci_ioreq {
> >       struct blockif_req io_req;
> >       struct ahci_port *io_pr;
> > @@ -136,7 +138,7 @@ struct ahci_port {
> >       struct pci_ahci_softc *pr_sc;
> >       uint8_t *cmd_lst;
> >       uint8_t *rfis;
> > -     char ident[20 + 1];
> > +     char ident[AHCI_PORT_IDENT];
> >       int port;
> >       int atapi;
> >       int reset;
> > @@ -2374,7 +2376,8 @@ pci_ahci_init(struct vmctx *ctx, struct
> pci_devinst *p
> >               MD5Init(&mdctx);
> >               MD5Update(&mdctx, opts, strlen(opts));
> >               MD5Final(digest, &mdctx);
> > -             sprintf(sc->port[p].ident,
> "BHYVE-%02X%02X-%02X%02X-%02X%02X",
> > +             snprintf(sc->port[p].ident, AHCI_PORT_IDENT,
> > +                 "BHYVE-%02X%02X-%02X%02X-%02X%02X",
> >                   digest[0], digest[1], digest[2], digest[3], digest[4]=
,
> >                   digest[5]);
> >
> >
> > Modified: head/usr.sbin/bhyve/pci_virtio_block.c
> >
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
> > --- head/usr.sbin/bhyve/pci_virtio_block.c    Tue Nov 20 22:12:10 2018
>       (r340706)
> > +++ head/usr.sbin/bhyve/pci_virtio_block.c    Tue Nov 20 22:21:19 2018
>       (r340707)
> > @@ -61,7 +61,7 @@ __FBSDID("$FreeBSD$");
> >  #define VTBLK_S_IOERR        1
> >  #define      VTBLK_S_UNSUPP  2
> >
> > -#define      VTBLK_BLK_ID_BYTES      20
> > +#define      VTBLK_BLK_ID_BYTES      20 + 1
> >
> >  /* Capability bits */
> >  #define      VTBLK_F_SEG_MAX         (1 << 2)        /* Maximum reques=
t
> segments */
> > @@ -344,7 +344,8 @@ pci_vtblk_init(struct vmctx *ctx, struct pci_devins=
t
> *
> >       MD5Init(&mdctx);
> >       MD5Update(&mdctx, opts, strlen(opts));
> >       MD5Final(digest, &mdctx);
> > -     sprintf(sc->vbsc_ident, "BHYVE-%02X%02X-%02X%02X-%02X%02X",
> > +     snprintf(sc->vbsc_ident, VTBLK_BLK_ID_BYTES,
> > +         "BHYVE-%02X%02X-%02X%02X-%02X%02X",
> >           digest[0], digest[1], digest[2], digest[3], digest[4],
> digest[5]);
> >
> >       /* setup virtio block config space */
>
> Hey Marcelo,
>
> Thanks for committing this. Could VTBLK_BLK_ID_BYTES and
> AHCI_PORT_IDENT be merged into the same macro, defined in
> usr.sbin/bhyve/pci_emul.h? Especially since both equate to the same
> value.
>

The macro could be merged, but it is safer to have it in this way, in case
something changes specifically for one of the drivers.
I don't think pci_emul.h would be the right place for that, this file is in
charge of PCI emulation functions and it is pretty much generic among the
other drivers.

Best.


>
> Thanks,
>
> --
> Shawn Webb
> Cofounder and Security Engineer
> HardenedBSD
>
> Tor-ified Signal:    +1 443-546-8752
> Tor+XMPP+OTR:        lattera@is.a.hacker.sx
> GPG Key ID:          0x6A84658F52456EEE
> GPG Key Fingerprint: 2ABA B6BD EF6A F486 BE89  3D9E 6A84 658F 5245 6EEE
>


--=20

--=20
Marcelo Araujo            (__)araujo@FreeBSD.org
\\\'',)http://www.FreeBSD.org <http://www.freebsd.org/>;   \/  \ ^
Power To Server.         .\. /_)



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAOfEmZhpAExGajG5XhGhOMDdXvKrgt8eSL2EpGci5zhEnhdwzQ>