From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:57266 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751084AbdBXNil (ORCPT ); Fri, 24 Feb 2017 08:38:41 -0500 Message-ID: <1487943505.26310.4.camel@decadent.org.uk> Subject: Re: [PATCH 4.4 17/25] USB: serial: digi_acceleport: fix OOB data sanity check From: Ben Hutchings To: Greg Kroah-Hartman , linux-kernel@vger.kernel.org Cc: stable@vger.kernel.org, Johan Hovold Date: Fri, 24 Feb 2017 13:38:25 +0000 In-Reply-To: <20170224082130.561381811@linuxfoundation.org> References: <20170224082128.156304123@linuxfoundation.org> <20170224082130.561381811@linuxfoundation.org> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-q3Wj73cpS3sqcMfoi9o0" Mime-Version: 1.0 Sender: stable-owner@vger.kernel.org List-ID: --=-q3Wj73cpS3sqcMfoi9o0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2017-02-24 at 09:25 +0100, Greg Kroah-Hartman wrote: > 4.4-stable review patch.=C2=A0=C2=A0If anyone has any objections, please = let me know. >=20 > ------------------ >=20 > From: Johan Hovold >=20 > commit 2d380889215fe20b8523345649dee0579821800c upstream. >=20 > Make sure to check for short transfers to avoid underflow in a loop > condition when parsing the receive buffer. >=20 > Also fix an off-by-one error in the incomplete sanity check which could > lead to invalid data being parsed. This appears to *introduce* an off-by-one. Which is not as serious as the underflow, but is still a regression. Suppose we have urb->actual_length =3D=3D 4: [...] > - for (i =3D 0; i < urb->actual_length - 3;) { i < 1 is true, so we would run the loop once. > - opcode =3D ((unsigned char *)urb->transfer_buffer)[i++]; > - line =3D ((unsigned char *)urb->transfer_buffer)[i++]; > - status =3D ((unsigned char *)urb->transfer_buffer)[i++]; > - val =3D ((unsigned char *)urb->transfer_buffer)[i++]; > + for (i =3D 0; i < urb->actual_length - 4; i +=3D 4) { i < 0 is false, so we now skip the loop. > + opcode =3D buf[i]; > + line =3D buf[i + 1]; > + status =3D buf[i + 2]; > + val =3D buf[i + 3]; [...] Ben. --=20 Ben Hutchings All the simple programs have been written, and all the good names taken. --=-q3Wj73cpS3sqcMfoi9o0 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEErCspvTSmr92z9o8157/I7JWGEQkFAliwN1EACgkQ57/I7JWG EQnUkQ/+LuTua9/AthYQuTkSAUmLcoTZDp78AF2aj7ZXWJQBUnPZ3JWCmTWcS38Z W4mFF0kXdKRcR8xma9GZIn7YkUJa15y6eiCWEYgj5PzeyhpP/xnFyMjxTjTkiXJY mnPpwYkrrdOpe0VlWNbi//yC12Djne4TOnnPyRN4zNk+Cfy8PWlGo8uak6LEilxM 8WC21RW4f+xtVA3hGX3Zt0PIuQjJmD9DaPUGE2IrWz/jmTnyVwEEcb7Wa4C7ZZOK 8Alz86y8NFWTsW+14Koe/op8vUlDf3F7iWkpn0LfkCj6YGYMYP92i6rQqPhj8raG y+p68+ZIZyVlrro1EBnDHEJCFIw9iRUF3Hgx4pZCsNDXTY5/xatQb/ePCdV6fZzh RKpY2bPnM9+obutqYOMxJ2H/59zutFNTSBOoIO0yaHW2ZKhfPsfbTUtnxl/LiZl/ UbOgCWAqruAJM3V+zvukNWFZmvXhKRfugGcTEfoIAsUB5hYRKb9yCYt7bvTqu4rk R1gUXT59E9tk8SXsNJK+lcudNp253uMnJw7DV3/tQgldLWEuFais1jv+Tz841Q4D wPMzdv1x7VDI9Loq/iv1RMshl0U11U8IjbYK0Ywg4dWKbLpAPb/wh90gBXxGbt86 GCP0ovy2dRXN8GhcdZXRyjMGfYRDNkMDBm2nXopBlUHXKm9AmLI= =3+Xe -----END PGP SIGNATURE----- --=-q3Wj73cpS3sqcMfoi9o0--