From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42961) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YL7YM-0005xV-Bk for qemu-devel@nongnu.org; Tue, 10 Feb 2015 04:56:23 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YL7YH-0002cv-5S for qemu-devel@nongnu.org; Tue, 10 Feb 2015 04:56:22 -0500 Received: from mail-we0-x22a.google.com ([2a00:1450:400c:c03::22a]:35368) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YL7YG-0002cW-VD for qemu-devel@nongnu.org; Tue, 10 Feb 2015 04:56:17 -0500 Received: by mail-we0-f170.google.com with SMTP id q59so23623447wes.1 for ; Tue, 10 Feb 2015 01:56:16 -0800 (PST) Date: Tue, 10 Feb 2015 09:56:12 +0000 From: Stefan Hajnoczi Message-ID: <20150210095612.GA9229@stefanha-thinkpad.redhat.com> References: <1418780167-16231-1-git-send-email-jsnow@redhat.com> <1418780167-16231-17-git-send-email-jsnow@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Q68bSM7Ycu6FN28Q" Content-Disposition: inline In-Reply-To: <1418780167-16231-17-git-send-email-jsnow@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 16/17] ahci: Recompute cur_cmd on migrate post load List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: kwolf@redhat.com, mst@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com --Q68bSM7Ycu6FN28Q Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Dec 16, 2014 at 08:36:06PM -0500, John Snow wrote: > When the AHCI HBA device is migrated, all of the information that > led to the request being created is stored in the AHCIDevice > structures, except for pointers into guest data where return > information needs to be stored. >=20 > The "cur_cmd" field is usually responsible for this. >=20 > To rebuild the cur_cmd pointer post-migration, we can utilize > the busy_slot index to figure out where the command header > we are still processing is. >=20 > This allows a machine in a halted state from rerror=3Dstop or > werror=3Dstop to be migrated and resume operations without issue. >=20 > Signed-off-by: John Snow > --- > hw/ide/ahci.c | 4 ++++ > 1 file changed, 4 insertions(+) >=20 > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c > index c153228..8078d3e 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -1373,6 +1373,10 @@ static int ahci_state_post_load(void *opaque, int = version_id) > */ > if (ad->busy_slot =3D=3D -1) { > check_cmd(s, i); > + } else { > + /* We are in the middle of a command, and may need to access > + * the command header in guest memory again. */ > + ad->cur_cmd =3D &((AHCICmdHdr *)ad->lst)[ad->busy_slot]; Where do we check that ad->busy_slot is within ad->lst[] bounds? If a malicious source sends a bogus value, this patch will lead to out-of-bounds accesses. Stefan --Q68bSM7Ycu6FN28Q Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJU2dW8AAoJEJykq7OBq3PIVa8H/1Htl0p2DqGflcM3PRSkW9nn qHfFHVfOPrFWcBYMAiDcnFf6GcZFGAcQEj6hTIAaV44B200EdGBT5Iu1e98XdXiM o2YCxjtECB/iJlJ6kQtJi5+fUwzTsTavG5VBMLlZZOmoQFbNMV3dT5b8d4pd6MiO UgRtxaJSzEfGurHUUXBxNmMYJ5sxfMf1aVVrGiv6ZOYB3lFpCdi2leTTln+IP4+e uyWb8wjUOukNdXyOAWDrYm91kdZtoq64wiFWDsBNjpEp4VmrogMGl1X7AO++y55l k6UgenVqNVXk4ounIdLGH8Lcpb7LG9Fe+RG34z6rPn1PSnf9xHsTd0PcBaCl2Cc= =lAC0 -----END PGP SIGNATURE----- --Q68bSM7Ycu6FN28Q--