Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 20 Nov 2018 19:22:54 -0500
From:      Shawn Webb <shawn.webb@hardenedbsd.org>
To:        Marcelo Araujo <araujo@FreeBSD.org>
Cc:        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:  <20181121002254.efitgf45bzajh5sj@mutt-hbsd>
In-Reply-To: <201811202221.wAKMLJ3W068166@repo.freebsd.org>
References:  <201811202221.wAKMLJ3W068166@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help

--qf5lp2runf5q3qgd
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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
>=20
> 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).
>  =20
>   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.
>=20
> Modified:
>   head/usr.sbin/bhyve/pci_ahci.c
>   head/usr.sbin/bhyve/pci_virtio_block.c
>=20
> 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
> =20
>  /*
> @@ -119,6 +119,8 @@ static FILE *dbg;
>  #endif
>  #define WPRINTF(format, arg...) printf(format, ##arg)
> =20
> +#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]);
> =20
>=20
> 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	(r340=
706)
> +++ head/usr.sbin/bhyve/pci_virtio_block.c	Tue Nov 20 22:21:19 2018	(r340=
707)
> @@ -61,7 +61,7 @@ __FBSDID("$FreeBSD$");
>  #define VTBLK_S_IOERR	1
>  #define	VTBLK_S_UNSUPP	2
> =20
> -#define	VTBLK_BLK_ID_BYTES	20
> +#define	VTBLK_BLK_ID_BYTES	20 + 1
> =20
>  /* Capability bits */
>  #define	VTBLK_F_SEG_MAX		(1 << 2)	/* Maximum request segments */
> @@ -344,7 +344,8 @@ pci_vtblk_init(struct vmctx *ctx, struct pci_devinst *
>  	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]);
> =20
>  	/* 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.

Thanks,

--=20
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

--qf5lp2runf5q3qgd
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEKrq2ve9q9Ia+iT2eaoRlj1JFbu4FAlv0pV0ACgkQaoRlj1JF
bu7sEQ//RqaPN3xVSxteX4+el9RGjOEHxqV2/yaSJZbgOvmM9cbByivnEL59v4q/
8t9O8KbrPrnBmLPqtNpwQCnEHxafBRhboSmzSiIrrsR2tfPkZ6h13l/0jChQMxXV
AyutHMbyROJ6/d4vjGQEy1oiswBjcKWQGy+9qt5bjOmMgjDDZvvfHdsEQlJ5ZXEe
0umTnhX6wR/mt6JKlvOnF110ceZn6O5Y/TiTn8s22L6PjNOZmOfVsDHhQZ5Dmc2A
TWzNIY7KiHQF+K42jLlQF0QUNAczeuLnjAopWsVF5uganBw/g7qmlFAo5Dru/HFz
wGLY0wjsfbOMq6VnFq97UuPUakYxIaNXbOd1UX6SpHm0wGT8MTifBQG5BoKL5JNI
pAOHOj38yMCrRt2TcmMGia3++OpSU6nTiEgNjdazHfdj4zceLCuYYZUZX9gYLFeK
Gn747fbRUKeRvZ43DK0TlOV5lmuiafLO4B9aGbMFqYB6Ty33IbzhiFr1yj8XBKL7
oj/V8gBEajTSux+/2X0rXaKsCIbMPwOV23ZYqO/2iLldssompbZGzn6aFSvctSMP
SzlcJ7jXfvSCbZ1+AC0DSH96ADkRLTp+IziRTg88LwZZt4Uu9r4ljTwgM5yNt5z2
oT5ij4B8t/kyTqeWhBuZQXbodJq4m+OFKG7Tp9UHDcsmCCCAHEc=
=mQ8k
-----END PGP SIGNATURE-----

--qf5lp2runf5q3qgd--



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