From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43550) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XyI8P-00037c-Kk for qemu-devel@nongnu.org; Tue, 09 Dec 2014 05:35:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XyI8J-0005DW-GX for qemu-devel@nongnu.org; Tue, 09 Dec 2014 05:35:13 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38330) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XyI8J-0005B6-9r for qemu-devel@nongnu.org; Tue, 09 Dec 2014 05:35:07 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sB9AZ60x019318 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Tue, 9 Dec 2014 05:35:06 -0500 Date: Tue, 9 Dec 2014 10:35:04 +0000 From: Stefan Hajnoczi Message-ID: <20141209103504.GC18649@stefanha-thinkpad.redhat.com> References: <1417719559-26380-1-git-send-email-jsnow@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="0lnxQi9hkpPO77W3" Content-Disposition: inline In-Reply-To: <1417719559-26380-1-git-send-email-jsnow@redhat.com> Subject: Re: [Qemu-devel] [PATCH] ide: Implement VPD response for ATAPI List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com --0lnxQi9hkpPO77W3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Dec 04, 2014 at 01:59:19PM -0500, John Snow wrote: > diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c > index c63b7e5..d27c34b 100644 > --- a/hw/ide/atapi.c > +++ b/hw/ide/atapi.c > @@ -622,19 +622,98 @@ static void cmd_request_sense(IDEState *s, uint8_t = *buf) > static void cmd_inquiry(IDEState *s, uint8_t *buf) > { > int max_len =3D buf[4]; What guarantees do we have about the size of buf in this function? We can't trust max_len because it comes from the guest. > + int idx =3D 0; > =20 > - buf[0] =3D 0x05; /* CD-ROM */ > - buf[1] =3D 0x80; /* removable */ > - buf[2] =3D 0x00; /* ISO */ > - buf[3] =3D 0x21; /* ATAPI-2 (XXX: put ATAPI-4 ?) */ > - buf[4] =3D 31; /* additional length */ > - buf[5] =3D 0; /* reserved */ > - buf[6] =3D 0; /* reserved */ > - buf[7] =3D 0; /* reserved */ > - padstr8(buf + 8, 8, "QEMU"); > - padstr8(buf + 16, 16, "QEMU DVD-ROM"); > - padstr8(buf + 32, 4, s->version); > - ide_atapi_cmd_reply(s, 36, max_len); > + /* If the EVPD (Enable Vital Product Data) bit is set in byte 1, > + * we are being asked for a specific page of info indicated by byte = 2. */ > + if (buf[1] & 0x01) { > + switch (buf[2]) { > + case 0x00: > + /* Supported Pages */ > + buf[idx++] =3D 0x05; /* CD-ROM */ > + buf[idx++] =3D 0x00; /* Page Code (0x00) */ > + buf[idx++] =3D 0x00; /* reserved */ > + buf[idx++] =3D 0x02; /* Two pages supported: */ > + buf[idx++] =3D 0x00; /* 0x00: Supported Pages, and: */ > + buf[idx++] =3D 0x83; /* 0x83: Device Identification. */ > + break; > + case 0x83: > + /* Device Identification. Each entry is optional, but the en= tries > + * included here are modeled after libata's VPD responses. */ > + buf[idx++] =3D 0x05; /* CD-ROM */ > + buf[idx++] =3D 0x83; /* Page Code */ > + buf[idx++] =3D 0x00; > + buf[idx++] =3D 0x00; /* Length of encapsulated structure: */ > + > + /* Entry 1: Serial */ > + if (idx + 24 > max_len) { > + /* Not enough room for even the first entry: */ > + /* 4 byte header + 20 byte string */ > + ide_atapi_cmd_error(s, ILLEGAL_REQUEST, > + ASC_DATA_PHASE_ERROR); Missing return > + } > + buf[idx++] =3D 0x02; /* Ascii */ > + buf[idx++] =3D 0x00; /* Vendor Specific */ > + buf[idx++] =3D 0x00; > + buf[idx++] =3D 20; /* Remaining length */ > + padstr8(buf + idx, 20, s->drive_serial_str); > + idx +=3D 20; > + > + /* Entry 2: Drive Model and Serial */ > + if (idx + 72 > max_len) { > + /* 4 (header) + 8 (vendor) + 60 (model & serial) */ > + goto out; > + } I guess this time it's okay to return early when there is not enough space left due to optional fields? Do we need to set buf[3] (which is currently 0)? --0lnxQi9hkpPO77W3 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJUhtBYAAoJEJykq7OBq3PI34cH/AqqzBTEeWl0ZGa3zU/DMzDv ceh2PKfxv5X5UOxx2N7TWhLePRjQamHWD3GpPVOTK9csM52aMjjlmRdXv4jFt7RY rdkO4D2DWC+L4SYOqCp7smRS6MZBrxzWtPV20ZJ45O/uv0r0OHeNOdK4+K5rTrQx orYiEEBmTKnRdf9zP9j6bWTFlSEG6Ru6/JZe9mXQoCDecwiRGsqVjRFi9HAR5PZr k0cPPo7SV++jAFcwd7OBGQUhJjWj6QyLt36u0Z0DKEL6oDbbrcS3p30lHGEgisTb pxZZY7I2ln4jJhTe2XjZ76vlT8ws1CPafiEdnLVk3wjua/skz98uZ+jsgH/3Npc= =kZD+ -----END PGP SIGNATURE----- --0lnxQi9hkpPO77W3--