From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754839AbcARLuh (ORCPT ); Mon, 18 Jan 2016 06:50:37 -0500 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:33043 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754573AbcARLue (ORCPT ); Mon, 18 Jan 2016 06:50:34 -0500 Message-ID: <1453117824.2519.114.camel@decadent.org.uk> Subject: Re: [PATCH 3.2 42/70] net: possible use after free in dst_release From: Ben Hutchings To: Francesco Ruggeri Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, akpm@linux-foundation.org, "David S. Miller" , Eric Dumazet , Francesco Ruggeri Date: Mon, 18 Jan 2016 11:50:24 +0000 In-Reply-To: References: Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-obJWOIJ3tZzWqsf3pcaD" X-Mailer: Evolution 3.18.3-1 Mime-Version: 1.0 X-SA-Exim-Connect-IP: 192.168.4.247 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-obJWOIJ3tZzWqsf3pcaD Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sun, 2016-01-17 at 19:49 -0800, Francesco Ruggeri wrote: > On Sun, Jan 17, 2016 at 7:18 PM, Ben Hutchings wrot= e: > > 3.2.76-rc1 review patch.=C2=A0=C2=A0If anyone has any objections, pleas= e let me know. > >=20 > > ------------------ > >=20 > > From: Francesco Ruggeri > >=20 > > commit 07a5d38453599052aff0877b16bb9c1585f08609 upstream. > >=20 > > dst_release should not access dst->flags after decrementing > > __refcnt to 0. The dst_entry may be in dst_busy_list and > > dst_gc_task may dst_destroy it before dst_release gets a chance > > to access dst->flags. > >=20 > > Fixes: d69bbf88c8d0 ("net: fix a race in dst_release()") > > Fixes: 27b75c95f10d ("net: avoid RCU for NOCACHE dst") > > Signed-off-by: Francesco Ruggeri > > Acked-by: Eric Dumazet > > Signed-off-by: David S. Miller > > [bwh: Backported to 3.2: adjust context] > > Signed-off-by: Ben Hutchings > > --- > > =C2=A0net/core/dst.c | 3 ++- > > =C2=A01 file changed, 2 insertions(+), 1 deletion(-) > >=20 > > --- a/net/core/dst.c > > +++ b/net/core/dst.c > > @@ -269,10 +269,11 @@ void dst_release(struct dst_entry *dst) > > =C2=A0{ > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (dst) { > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0int newrefcnt; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0unsigned short nocache =3D dst->flags & DST_NOCACHE; >=20 > Only one minor thing. Before 3.6 dst->flags was an int. I noticed that - but no flags outside the 16-bit range are assigned, so I think this is OK. Ben. > Thanks, > Francesco >=20 > >=20 > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0newrefcnt =3D atomic_dec_return(&dst->__refcnt); > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0WARN_ON(newrefcnt < 0); > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0if (!newrefcnt && unlikely(dst->flags & DST_NOCACHE)) = { > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0if (!newrefcnt && unlikely(nocache)) { > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= dst =3D dst_destroy(dst); > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= if (dst) > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0__dst_free(dst); > >=20 --=20 Ben Hutchings A free society is one where it is safe to be unpopular. - Adlai Stevenson --=-obJWOIJ3tZzWqsf3pcaD Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIVAwUAVpzRgOe/yOyVhhEJAQqvKRAAgkPzGyQkcfNHWCt9puuP2SBPX81Wq7W3 z5eQSR8NWXUJ+IPfqm2QuZyJLoC/LkNb892ObeH5ZZFRrKA4d5LhFD5Evxm+pIVe 1v26bKNXUyt0u3BG3ZP+kt9ZZwHZICc5m1voZ1o8MCAROipMyUGPihzGpi/9pNSt ErNLhak7Xs03wcIFnHqBlhse3xlsbPjByJNQhY/Xx+Nqhr9iL2vJP+6L1XR3G1fj VB+9A4p+zeUMqrHe1H4TWxh2G4+JK2Oe7vqwC9O4/JvA4XTKAEYDuO/zP8F8MskP AZsr4MOs9Ui4PhKBLqaGqicv+ZReWXy+XpTiXI/1KbAxbjPDA597amYE6NcuoAap 44bjCMBlpnTXiijeW+TuppzfYFZEOBxC0i2d6uMVEiTzraaKTbq1nVIxijw3Cs1Y bpkB3du5qCCOVnp20Ixn5/2FmRTNNfGFI8V4+67M6qk1/AsHziemTbmXCPM7prSQ rPe6A9mSifqv9S3y0ULU2EkqHUZwO5xltT7+4aBI+0DCovX/0zfuRJtge21lSLrb JOm8WJlKxBf4kPvjeI/nfxITXQj3L8uLTgO6aNmuEvAvnb22n57ao6tdS3yaN609 +77fuhmTeMg7u7Q5PrWaUN1F3OZ0ubQeIZeyeZyHKS7HBHmyFn2oHCMqgrrqraZx y0FG81enn4o= =yraP -----END PGP SIGNATURE----- --=-obJWOIJ3tZzWqsf3pcaD--