Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752747AbbLMVXF (ORCPT ); Sun, 13 Dec 2015 16:23:05 -0500 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:54054 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752416AbbLMVXD (ORCPT ); Sun, 13 Dec 2015 16:23:03 -0500 Message-ID: <1450041762.3944.4.camel@decadent.org.uk> Subject: Re: [PATCH 3.16.y-ckt 009/126] sit: fix sit0 percpu double allocations From: Ben Hutchings To: Eric Dumazet Cc: Luis Henriques , Eric Dumazet , Steffen Klassert , "David S. Miller" , linux-kernel@vger.kernel.org, stable@vger.kernel.org, kernel-team@lists.ubuntu.com Date: Sun, 13 Dec 2015 21:22:42 +0000 In-Reply-To: <1450039390.9768.30.camel@edumazet-glaptop2.roam.corp.google.com> References: <1449653896-5236-1-git-send-email-luis.henriques@canonical.com> <1449653896-5236-10-git-send-email-luis.henriques@canonical.com> <1449893906.3836.5.camel@decadent.org.uk> <20151213185429.GA31826@charon> <1450038035.3944.2.camel@decadent.org.uk> <1450039390.9768.30.camel@edumazet-glaptop2.roam.corp.google.com> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-XG7lzJXGZ9esCgTpsJlv" X-Mailer: Evolution 3.18.2-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 Content-Length: 3881 Lines: 104 --=-XG7lzJXGZ9esCgTpsJlv Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sun, 2015-12-13 at 12:43 -0800, Eric Dumazet wrote: > On Sun, 2015-12-13 at 20:20 +0000, Ben Hutchings wrote: > > On Sun, 2015-12-13 at 18:54 +0000, Luis Henriques wrote: > > > On Sat, Dec 12, 2015 at 04:18:26AM +0000, Ben Hutchings wrote: > > > > On Wed, 2015-12-09 at 09:36 +0000, Luis Henriques wrote: > > > > > 3.16.7-ckt21 -stable review patch.=C2=A0=C2=A0If anyone has any o= bjections, > > > > > please let me know. > > > > >=20 > > > > > ------------------ > > > > >=20 > > > > > From: Eric Dumazet > > > > >=20 > > > > > commit 4ece9009774596ee3df0acba65a324b7ea79387c upstream. > > > > >=20 > > > > > sit0 device allocates its percpu storage twice : > > > > > - One time in ipip6_tunnel_init() > > > > > - One time in ipip6_fb_tunnel_init() > > > > >=20 > > > > > Thus we leak 48 bytes per possible cpu per network namespace > > > > > dismantle. > > > > >=20 > > > > > ipip6_fb_tunnel_init() can be much simpler and does not > > > > > return an error, and should be called after register_netdev() > > > > [...] > > > >=20 > > > > Doesn't this introduce a race condition when sit is a module?=C2=A0= =C2=A0There > > > > seems to be nothing to prevent access to the partially initialised > > > > device after calling register_netdev(), if sit_init_net() is called > > > > during module loading rather than during namespace creation. > > > >=20 > > >=20 > > > This seems to be an upstream issue, not specific to the 3.16.y-ckt > > > stable kernel.=C2=A0=C2=A0If that is the case, I guess I'll just keep= this patch > > > and later apply the fix.=C2=A0=C2=A0Or do you think this race is real= ly likely > > > to be a worst problem than then issue the patch is trying to fix? > >=20 > > It seems worse than the problem being fixed. >=20 > 1) Sorry Ben, I do not understand the problem you mention. > =C2=A0=C2=A0=C2=A0What is a partially initialized device exactly ? A tunnel device which is registered but hasn't had its private structure fully initialised yet. > 2) I have no idea why this patch is even backported to 3.16, > =C2=A0since it is fixing a problem added in 3.18 : >=20 > # git describe --contains ebe084aafb7e > v3.18-rc5~22^2~42^2~1 >=20 > If your 3.16 kernel survives this loop without consuming memory like > crazy, then the backport is not needed. >=20 > modprobe sit > while : > do > =C2=A0ip netns add ns1 > =C2=A0ip netns delete ns1 > done I can't detect a memory leak when doing this. Ben. --=20 Ben Hutchings Life would be so much easier if we could look at the source code. --=-XG7lzJXGZ9esCgTpsJlv Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIVAwUAVm3houe/yOyVhhEJAQpT/A//am8B2L0FohCf1g/rn2AV67SXE2la1Myz gxzWvf18UJIvaCRa2YvsMO8/1GZccWbsCJwhAxrLAf0J11mG7TnR+WtPJXDDjy/M uduGx2T+AApK0u4LCfaWNyTcEBy+FmwJf+DEvvZQs+Qz5WQOolEJLlzdMWL56Xgl 27UKEarEZxYxFYmpQGqaffVmvmSOpE+mzTES8GjQ1p90rI+huBqNPdZpTr5naHol 7jWQPogYkfao6zRTu/alurhmbuGJitDGjdDELWN2Ng+ndL1dj6VoPnQ4oSSDBji6 n+zcZX54OgcdHlV29mJyBCf7hibeF6P0MBEd9vWShLIyoKxXXG2QaLG6XTquX5yH 5PFFz3JLCj7TiWPWj7dis9JLI9B4mbMVD5ryrMydTg+z6Alj///badb95eyg2Kcl OVTn55eulS4kkVB/nJpEIGMvMR+b+/rLnI71RGasUTA6yX1OQjmYUYqnj7yMy+y6 bGhzuELAyZANfgdcCfFmbFLBJPp3gOUw74P9jrZ14XPDcMqNaTyLl6ROOicao7J6 Gacd3Nxi9VVQpSPb6G1hn7lzOxTWlsiT6c9ixYDpjELWLFShWphbpSfQ1Io0vEzu NBsPcC14v2FSSX+OaH/WrVzPANvOassyuSEjdLvVNHPsKP1enhEuRUHTiA5WoCIh 17n4YEUCKlQ= =5emH -----END PGP SIGNATURE----- --=-XG7lzJXGZ9esCgTpsJlv-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/