All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf 0/2] Two x86 BPF JIT fixes
@ 2018-05-02 18:12 Daniel Borkmann
  2018-05-02 18:12 ` [PATCH bpf 1/2] bpf, x64: fix memleak when not converging after image Daniel Borkmann
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Daniel Borkmann @ 2018-05-02 18:12 UTC (permalink / raw)
  To: ast; +Cc: netdev, Daniel Borkmann

Fix two memory leaks in x86 JIT. For details, please see
individual patches in this series. Thanks!

Daniel Borkmann (2):
  bpf, x64: fix memleak when not converging after image
  bpf, x64: fix memleak when not converging on calls

 arch/x86/net/bpf_jit_comp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.9.5

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH bpf 1/2] bpf, x64: fix memleak when not converging after image
  2018-05-02 18:12 [PATCH bpf 0/2] Two x86 BPF JIT fixes Daniel Borkmann
@ 2018-05-02 18:12 ` Daniel Borkmann
  2018-05-02 18:53   ` David Miller
  2018-05-02 18:12 ` [PATCH bpf 2/2] bpf, x64: fix memleak when not converging on calls Daniel Borkmann
  2018-05-02 19:39 ` [PATCH bpf 0/2] Two x86 BPF JIT fixes Alexei Starovoitov
  2 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2018-05-02 18:12 UTC (permalink / raw)
  To: ast; +Cc: netdev, Daniel Borkmann

While reviewing x64 JIT code, I noticed that we leak the prior allocated
JIT image in the case where proglen != oldproglen during the JIT passes.
Prior to the commit e0ee9c12157d ("x86: bpf_jit: fix two bugs in eBPF JIT
compiler") we would just break out of the loop, and using the image as the
JITed prog since it could only shrink in size anyway. After e0ee9c12157d,
we would bail out to out_addrs label where we free addrs and jit_data but
not the image coming from bpf_jit_binary_alloc().

Fixes: e0ee9c12157d ("x86: bpf_jit: fix two bugs in eBPF JIT compiler")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 arch/x86/net/bpf_jit_comp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index abce27c..9ae7b93 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1236,6 +1236,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 	for (pass = 0; pass < 20 || image; pass++) {
 		proglen = do_jit(prog, addrs, image, oldproglen, &ctx);
 		if (proglen <= 0) {
+out_image:
 			image = NULL;
 			if (header)
 				bpf_jit_binary_free(header);
@@ -1246,8 +1247,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 			if (proglen != oldproglen) {
 				pr_err("bpf_jit: proglen=%d != oldproglen=%d\n",
 				       proglen, oldproglen);
-				prog = orig_prog;
-				goto out_addrs;
+				goto out_image;
 			}
 			break;
 		}
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH bpf 2/2] bpf, x64: fix memleak when not converging on calls
  2018-05-02 18:12 [PATCH bpf 0/2] Two x86 BPF JIT fixes Daniel Borkmann
  2018-05-02 18:12 ` [PATCH bpf 1/2] bpf, x64: fix memleak when not converging after image Daniel Borkmann
@ 2018-05-02 18:12 ` Daniel Borkmann
  2018-05-02 18:54   ` David Miller
  2018-05-02 19:39 ` [PATCH bpf 0/2] Two x86 BPF JIT fixes Alexei Starovoitov
  2 siblings, 1 reply; 6+ messages in thread
From: Daniel Borkmann @ 2018-05-02 18:12 UTC (permalink / raw)
  To: ast; +Cc: netdev, Daniel Borkmann

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 <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@kernel.org>
---
 arch/x86/net/bpf_jit_comp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 9ae7b93..263c845 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1283,7 +1283,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 		prog = orig_prog;
 	}
 
-	if (!prog->is_func || extra_pass) {
+	if (!image || !prog->is_func || extra_pass) {
 out_addrs:
 		kfree(addrs);
 		kfree(jit_data);
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH bpf 1/2] bpf, x64: fix memleak when not converging after image
  2018-05-02 18:12 ` [PATCH bpf 1/2] bpf, x64: fix memleak when not converging after image Daniel Borkmann
@ 2018-05-02 18:53   ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2018-05-02 18:53 UTC (permalink / raw)
  To: daniel; +Cc: ast, netdev

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Wed,  2 May 2018 20:12:22 +0200

> While reviewing x64 JIT code, I noticed that we leak the prior allocated
> JIT image in the case where proglen != oldproglen during the JIT passes.
> Prior to the commit e0ee9c12157d ("x86: bpf_jit: fix two bugs in eBPF JIT
> compiler") we would just break out of the loop, and using the image as the
> JITed prog since it could only shrink in size anyway. After e0ee9c12157d,
> we would bail out to out_addrs label where we free addrs and jit_data but
> not the image coming from bpf_jit_binary_alloc().
> 
> Fixes: e0ee9c12157d ("x86: bpf_jit: fix two bugs in eBPF JIT compiler")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH bpf 2/2] bpf, x64: fix memleak when not converging on calls
  2018-05-02 18:12 ` [PATCH bpf 2/2] bpf, x64: fix memleak when not converging on calls Daniel Borkmann
@ 2018-05-02 18:54   ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2018-05-02 18:54 UTC (permalink / raw)
  To: daniel; +Cc: ast, netdev

From: Daniel Borkmann <daniel@iogearbox.net>
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 <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH bpf 0/2] Two x86 BPF JIT fixes
  2018-05-02 18:12 [PATCH bpf 0/2] Two x86 BPF JIT fixes Daniel Borkmann
  2018-05-02 18:12 ` [PATCH bpf 1/2] bpf, x64: fix memleak when not converging after image Daniel Borkmann
  2018-05-02 18:12 ` [PATCH bpf 2/2] bpf, x64: fix memleak when not converging on calls Daniel Borkmann
@ 2018-05-02 19:39 ` Alexei Starovoitov
  2 siblings, 0 replies; 6+ messages in thread
From: Alexei Starovoitov @ 2018-05-02 19:39 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: ast, netdev

On Wed, May 02, 2018 at 08:12:21PM +0200, Daniel Borkmann wrote:
> Fix two memory leaks in x86 JIT. For details, please see
> individual patches in this series. Thanks!

Applied to bpf tree, Thanks Daniel.

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-05-02 19:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-02 18:12 [PATCH bpf 0/2] Two x86 BPF JIT fixes Daniel Borkmann
2018-05-02 18:12 ` [PATCH bpf 1/2] bpf, x64: fix memleak when not converging after image Daniel Borkmann
2018-05-02 18:53   ` David Miller
2018-05-02 18:12 ` [PATCH bpf 2/2] bpf, x64: fix memleak when not converging on calls Daniel Borkmann
2018-05-02 18:54   ` David Miller
2018-05-02 19:39 ` [PATCH bpf 0/2] Two x86 BPF JIT fixes Alexei Starovoitov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.