From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Miller Subject: Re: [PATCH bpf 2/2] bpf, x64: fix memleak when not converging on calls Date: Wed, 02 May 2018 14:54:01 -0400 (EDT) Message-ID: <20180502.145401.1482877244354567278.davem@davemloft.net> References: <20180502181223.30613-1-daniel@iogearbox.net> <20180502181223.30613-3-daniel@iogearbox.net> Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: ast@kernel.org, netdev@vger.kernel.org To: daniel@iogearbox.net Return-path: Received: from shards.monkeyblade.net ([184.105.139.130]:36184 "EHLO shards.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751030AbeEBSyC (ORCPT ); Wed, 2 May 2018 14:54:02 -0400 In-Reply-To: <20180502181223.30613-3-daniel@iogearbox.net> Sender: netdev-owner@vger.kernel.org List-ID: From: Daniel Borkmann Date: Wed, 2 May 2018 20:12:23 +0200 > The JIT logic in jit_subprogs() is as follows: for all subprogs we > allocate a bpf_prog_alloc(), populate it (prog->is_func = 1 here), > and pass it to bpf_int_jit_compile(). If a failure occurred during > JIT and prog->jited is not set, then we bail out from attempting to > JIT the whole program, and punt to the interpreter instead. In case > JITing went successful, we fixup BPF call offsets and do another > pass to bpf_int_jit_compile() (extra_pass is true at that point) to > complete JITing calls. Given that requires to pass JIT context around > addrs and jit_data from x86 JIT are freed in the extra_pass in > bpf_int_jit_compile() when calls are involved (if not, they can > be freed immediately). However, if in the original pass, the JIT > image didn't converge then we leak addrs and jit_data since image > itself is NULL, the prog->is_func is set and extra_pass is false > in that case, meaning both will become unreachable and are never > cleaned up, therefore we need to free as well on !image. Only x64 > JIT is affected. > > Fixes: 1c2a088a6626 ("bpf: x64: add JIT support for multi-function programs") > Signed-off-by: Daniel Borkmann > Acked-by: Alexei Starovoitov Acked-by: David S. Miller