From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51431) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c7nDH-0003YF-6j for qemu-devel@nongnu.org; Fri, 18 Nov 2016 12:44:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c7nDG-0003a6-8I for qemu-devel@nongnu.org; Fri, 18 Nov 2016 12:44:35 -0500 Date: Fri, 18 Nov 2016 18:41:09 +0100 From: Olaf Hering Message-ID: <20161118174108.GF5717@aepfle.de> References: <20161118102452.5779-1-olaf@aepfle.de> <42ca6186-ca47-3c63-d2e0-54f2ed9f4be7@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="wtjvnLv0o8UUzur2" Content-Disposition: inline In-Reply-To: <42ca6186-ca47-3c63-d2e0-54f2ed9f4be7@redhat.com> Subject: Re: [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-block@nongnu.org, Kevin Wolf , Stefano Stabellini , "open list:All patches CC here" , Max Reitz , "open list:X86" , Anthony Perard --wtjvnLv0o8UUzur2 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 18, Eric Blake wrote: > On 11/18/2016 04:24 AM, Olaf Hering wrote: > > + /* Overflowing byte limit? */ > > + if ((sec_start + sec_count) > ((INT64_MAX + INT_MAX) >> BDRV_SECTO= R_BITS)) { > This is undefined. INT64_MAX + anything non-negative overflows int64, The expanded value used to be stored into a uint64_t before it was used here. A "cleanup" introduced this error. Thanks for spotting. > If you are trying to detect guests that make a request that would cover > more than INT64_MAX bytes, you can simplify. Besides, for as much > storage as there is out there, I seriously doubt ANYONE will ever have > 2^63 bytes addressable through a single device. Why not just write it as: >=20 > if ((INT64_MAX >> BDRV_SECTOR_BITS) - sec_count < sec_start) { That would always be false I think. I will resubmit with this: if ((sec_start + sec_count) > (INT64_MAX >> BDRV_SECTOR_BITS)) { Regarding the cast for ->req, it has type blkif_request_t, but the pointer needs to be assigned to type blkif_request_discard_t. Olaf --wtjvnLv0o8UUzur2 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iEYEARECAAYFAlgvPS8ACgkQXUKg+qaYNn79pQCgj1ttlLZ5lSopXrd2g3yEJf8r G9gAoNTAYYG5S0PTo09IIebfKnHxQNVC =E1ga -----END PGP SIGNATURE----- --wtjvnLv0o8UUzur2-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olaf Hering Subject: Re: [Qemu-devel] [PATCH] xen_disk: convert discard input to byte ranges Date: Fri, 18 Nov 2016 18:41:09 +0100 Message-ID: <20161118174108.GF5717@aepfle.de> References: <20161118102452.5779-1-olaf@aepfle.de> <42ca6186-ca47-3c63-d2e0-54f2ed9f4be7@redhat.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4663720786576701003==" Return-path: In-Reply-To: <42ca6186-ca47-3c63-d2e0-54f2ed9f4be7@redhat.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Eric Blake Cc: Kevin Wolf , "open list:X86" , qemu-block@nongnu.org, "open list:All patches CC here" , Max Reitz , Stefano Stabellini , Anthony Perard List-Id: xen-devel@lists.xenproject.org --===============4663720786576701003== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="wtjvnLv0o8UUzur2" Content-Disposition: inline --wtjvnLv0o8UUzur2 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 18, Eric Blake wrote: > On 11/18/2016 04:24 AM, Olaf Hering wrote: > > + /* Overflowing byte limit? */ > > + if ((sec_start + sec_count) > ((INT64_MAX + INT_MAX) >> BDRV_SECTO= R_BITS)) { > This is undefined. INT64_MAX + anything non-negative overflows int64, The expanded value used to be stored into a uint64_t before it was used here. A "cleanup" introduced this error. Thanks for spotting. > If you are trying to detect guests that make a request that would cover > more than INT64_MAX bytes, you can simplify. Besides, for as much > storage as there is out there, I seriously doubt ANYONE will ever have > 2^63 bytes addressable through a single device. Why not just write it as: >=20 > if ((INT64_MAX >> BDRV_SECTOR_BITS) - sec_count < sec_start) { That would always be false I think. I will resubmit with this: if ((sec_start + sec_count) > (INT64_MAX >> BDRV_SECTOR_BITS)) { Regarding the cast for ->req, it has type blkif_request_t, but the pointer needs to be assigned to type blkif_request_discard_t. Olaf --wtjvnLv0o8UUzur2 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iEYEARECAAYFAlgvPS8ACgkQXUKg+qaYNn79pQCgj1ttlLZ5lSopXrd2g3yEJf8r G9gAoNTAYYG5S0PTo09IIebfKnHxQNVC =E1ga -----END PGP SIGNATURE----- --wtjvnLv0o8UUzur2-- --===============4663720786576701003== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============4663720786576701003==--