From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40717) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGY7W-0006gK-HR for qemu-devel@nongnu.org; Tue, 05 Jan 2016 15:22:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aGY7S-0007hm-Hf for qemu-devel@nongnu.org; Tue, 05 Jan 2016 15:22:18 -0500 Received: from [2a03:4000:1::4e2f:c7ac:d] (port=34587 helo=mail.weilnetz.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aGY7S-0007hg-7m for qemu-devel@nongnu.org; Tue, 05 Jan 2016 15:22:14 -0500 References: <1452005723-1494-1-git-send-email-ppandit@redhat.com> <1452005723-1494-2-git-send-email-ppandit@redhat.com> From: Stefan Weil Message-ID: <568C25EE.4040109@weilnetz.de> Date: Tue, 5 Jan 2016 21:22:06 +0100 MIME-Version: 1.0 In-Reply-To: <1452005723-1494-2-git-send-email-ppandit@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="7go0cabWpFSI9LNRbJwHWt4DOKotIKqvV" Subject: Re: [Qemu-devel] [PATCH for v2.3.0] fw_cfg: add check to validate current entry value List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: P J P , Qemu devel Cc: Peter Maydell , Prasad J Pandit This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --7go0cabWpFSI9LNRbJwHWt4DOKotIKqvV Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable Am 05.01.2016 um 15:55 schrieb P J P: > From: Prasad J Pandit >=20 > When processing firmware configurations, an OOB r/w access occurs > if 's->cur_entry' is set to be invalid(FW_CFG_INVALID=3D0xffff). > Add a check to validate 's->cur_entry' to avoid such access. >=20 > Reported-by: Donghai Zdh > Signed-off-by: Prasad J Pandit > --- > hw/nvram/fw_cfg.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) >=20 > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c > index 68eff77..ce026bc 100644 > --- a/hw/nvram/fw_cfg.c > +++ b/hw/nvram/fw_cfg.c > @@ -233,12 +233,15 @@ static void fw_cfg_reboot(FWCfgState *s) > static void fw_cfg_write(FWCfgState *s, uint8_t value) > { > int arch =3D !!(s->cur_entry & FW_CFG_ARCH_LOCAL); > - FWCfgEntry *e =3D &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MA= SK]; > + FWCfgEntry *e =3D (s->cur_entry =3D=3D FW_CFG_INVALID) ? NULL : > + &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MAS= K]; > =20 > trace_fw_cfg_write(s, value); > =20 > - if (s->cur_entry & FW_CFG_WRITE_CHANNEL && e->callback && > - s->cur_offset < e->len) { > + if (s->cur_entry !=3D FW_CFG_INVALID > + && s->cur_entry & FW_CFG_WRITE_CHANNEL > + && e->callback > + && s->cur_offset < e->len) { I suggest to test e !=3D NULL instead of s->cur_entry !=3D FW_CFG_INVALID= =2E Of course both variants are equivalent, but e !=3D NULL might be easier to review and make work of static code analyzers easier, too. > e->data[s->cur_offset++] =3D value; > if (s->cur_offset =3D=3D e->len) { > e->callback(e->callback_opaque, e->data); > @@ -267,7 +270,8 @@ static int fw_cfg_select(FWCfgState *s, uint16_t ke= y) > static uint8_t fw_cfg_read(FWCfgState *s) > { > int arch =3D !!(s->cur_entry & FW_CFG_ARCH_LOCAL); > - FWCfgEntry *e =3D &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MA= SK]; > + FWCfgEntry *e =3D (s->cur_entry =3D=3D FW_CFG_INVALID) ? NULL : > + &s->entries[arch][s->cur_entry & FW_CFG_ENTRY_MAS= K]; > uint8_t ret; > =20 > if (s->cur_entry =3D=3D FW_CFG_INVALID || !e->data || s->cur_offse= t >=3D e->len) >=20 --7go0cabWpFSI9LNRbJwHWt4DOKotIKqvV Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJWjCXyAAoJEOCMIdVndFCtC4IP/i8QJ85T81Q/8EDHplxzcVm5 ELj51qT9kwBtSc2+i1A3N6MULYSIqBnvedNkSJXPkAmrsAdF5RjBMecD6UTWJT6h yzyuUDcm5OK9zWRMmDcsZN4SCjGasR/1zj4am3sopPtvYWHTc0HkI8j5phu4nZTd FPyPzo02Nhqpc+GQZWp41QFgQ4pfz0cwGU0LP+50y9ruNFDLBOXWkdgvCKfJK2JM cqTnalONBZg5tuPlT/B0vRQYCQ2sqoCKZjLuW3xDa/EeT8ZShVVcDc0R/zOLBVy8 lNQBObltp0FZvu8XK+kAtxhYB2yfX8imI7TyYJ6i6uaEZ9xHTLJR9TvHMAfQc0S/ xSGaUTciarAPUiaSqXa3EiIwzWxnGfKo0RtzDalTqTIHOZndsY9+PwRLvJp3HTVA jesPMQUuKRN3qG053IzmkPmSi33W+6TOnq2KBNGaf3mdSXWG5X59XDpgLjg3qC9U WO8nQALS9QvYMv9KUBnaoRn5V8bfID+HIO+8zr6NHhPvw6GDgL7U0KGwa8/Zq5GN +lUtLBgEzc+M6fs+8cmQq2jajNfRIuEjCS3ZG9kcdugKm60nprcHajLTrhO1LHe/ w9BTZswLNRHJbaPt1tEJuogTbyzoFcJ1L749N5tZIp0rMs7ytfWMGM47wWBeSszV Bp1loqXHGXci1XwykidC =pk9Z -----END PGP SIGNATURE----- --7go0cabWpFSI9LNRbJwHWt4DOKotIKqvV--