Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751238AbdFCTtO (ORCPT ); Sat, 3 Jun 2017 15:49:14 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:47459 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750991AbdFCTtM (ORCPT ); Sat, 3 Jun 2017 15:49:12 -0400 Message-ID: <1496519338.3992.38.camel@decadent.org.uk> Subject: Re: [PATCH 3.16 144/212] batman-adv: Fix double free during fragment merge error From: Ben Hutchings To: Sven Eckelmann Cc: linux-kernel@vger.kernel.org, stable@vger.kernel.org, akpm@linux-foundation.org, Simon Wunderlich Date: Sat, 03 Jun 2017 20:48:58 +0100 In-Reply-To: <2107568.RkcaoAHtPv@sven-edge> References: <2107568.RkcaoAHtPv@sven-edge> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-WuT1mpc9GPQDsyiTCXAp" X-Mailer: Evolution 3.22.6-1 Mime-Version: 1.0 X-SA-Exim-Connect-IP: 2a02:8011:400e:2:6f00:88c8:c921:d332 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 Content-Length: 3591 Lines: 93 --=-WuT1mpc9GPQDsyiTCXAp Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2017-06-01 at 18:44 +0200, Sven Eckelmann wrote: > On Donnerstag, 1. Juni 2017 16:43:16 CEST Ben Hutchings wrote: > > 3.16.44-rc1 review patch.=C2=A0=C2=A0If anyone has any objections, plea= se let me know. >=20 > It looks to me like there are problems with this backport. The surroundin= g=C2=A0 > code has to be adjusted slightly further to avoid additional problems. Thanks for the review. [...] > It is not really easy to see but this change will introduce a new double = free=C2=A0 > for kernels older than v4.10. The relevant commit is b91a2543b4c1 ("batma= n- > adv: Consume skb in receive handlers"). This was discussed in following g= luon=C2=A0 > ticket https://github.com/freifunk-gluon/gluon/issues/1083 (just in case = you=C2=A0 > are interested about the details) >=20 > Following change must therefore be added to this patch on older kernels: >=20 > =C2=A0=C2=A0=C2=A0=C2=A0--- a/net/batman-adv/routing.c > =C2=A0=C2=A0=C2=A0=C2=A0+++ b/net/batman-adv/routing.c > =C2=A0=C2=A0=C2=A0=C2=A0@@ -961,6 +961,12 @@ int batadv_recv_frag_packet(= struct sk_buff *skb, > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 batadv_inc_counter(bat_priv, BATADV_CNT_FR= AG_RX); > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 batadv_add_counter(bat_priv, BATADV_CNT_FR= AG_RX_BYTES, skb->len); > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 > =C2=A0=C2=A0=C2=A0=C2=A0+ /* batadv_frag_skb_buffer will always consume t= he skb and > =C2=A0=C2=A0=C2=A0=C2=A0+ =C2=A0* the caller should therefore never try t= o free the > =C2=A0=C2=A0=C2=A0=C2=A0+ =C2=A0* skb after this point > =C2=A0=C2=A0=C2=A0=C2=A0+ =C2=A0*/ > =C2=A0=C2=A0=C2=A0=C2=A0+ ret =3D NET_RX_SUCCESS; > =C2=A0=C2=A0=C2=A0=C2=A0+ > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* Add fragment to buffer and merge if pos= sible. */ > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (!batadv_frag_skb_buffer(&skb, orig_nod= e_src)) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 goto out; >=20 > You can also remove the same instruction which appears later in this func= tion. OK, I'll squash this into the patch. Ben. --=20 Ben Hutchings Experience is directly proportional to the value of equipment destroyed. =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=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- Carolyn Scheppner --=-WuT1mpc9GPQDsyiTCXAp Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEErCspvTSmr92z9o8157/I7JWGEQkFAlkzEqoACgkQ57/I7JWG EQnB0g//R84sBPYYOVj8fGfSusaPw6GA0HWdCtK5ZKznbV7n1bKPXGq8aFASTPay igl76qOrfd4nUVBsPmge1WKxioIbmZIHfJqGDcExgV6KOYAbwzZgH2LLSEYyEpbB BOxjGx2SZwcCap7hbxn5xz1LGK2LukKAyJQmEklu3ZHz3Gn58TbFoiagpYapfYS/ Tf0yK0+/Q2dUyaJb0C7OswzNSpXEyGFw22AHUesDNAXl9Xsbwu5h810F2Xs9Im6G 9uIL0Z5QUHv7ZvHNGh8S99Kd+84+wboxL8kB8G9Do6Tca1BN5ToKP74eqopd2kPj Ks9jqKiUrTawo9dWtYRd1K6xUvVcX0pO0GQhUKYdt8q6dnE60edrRqsOAPQPqIVJ tiDQNentpOfmMtvO8d5Tty2m2sfevFmxUcQVPBjRO8kHLrU/QLGXIBgzDD3MP5sf Ym8+Rr6uu4pVsG55jQnGsrepVVvG5Ob6lj6boLMaI6aAY7yTv6lcNcK5p+HN6lDw SmAg0vzHrpaW8sYTCEY1TOa/cEmTZ3R6Ctxyf0cAAlc9oeoRiiXTEyH/YwOyAK5C IQByaKOlWiJb5cb0ezdx9ude32RkSAmFHbuc6lu+2X6SOuksgpbU1I5A75DUKQ5H SF0RUoHUEbZeWbOag/8w6T3vPcHw05vQli037z3Q6tPzAFU+AIw= =Eywk -----END PGP SIGNATURE----- --=-WuT1mpc9GPQDsyiTCXAp--