bpf: arm64: fix uninitialized variable
Subsystems (5)
Alexei Starovoitov
maintainerPATCH 1/1 bpf: arm64: fix uninitialized variable
From: Alexei Starovoitov <[email protected]>
fix the following issue:arch/arm64/net/bpf_jit_comp.c: In function 'bpf_int_jit_compile':arch/arm64/net/bpf_jit_comp.c:982:18: error: 'image_size' may be used uninitialized in this function [-Werror=maybe-uninitialized]
--- arch/arm64/net/bpf_jit_comp.c | 1 + 1 file changed, 1 insertion(+)
arch/arm64/net/bpf_jit_comp.c +1 -0
@@ -897,6 +897,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog) image_ptr = jit_data->image; header = jit_data->header; extra_pass = true; + image_size =sizeof (u32) * ctx.idx; goto skip_init_ctx; }memset (&ctx,0 ,sizeof (ctx));
Signature · 1 line
2.9.5
Daniel Borkmann
maintainerQuoted 20 lines · Alexei Starovoitov
From: Alexei Starovoitov <[email protected]>
fix the following issue:
arch/arm64/net/bpf_jit_comp.c: In function 'bpf_int_jit_compile':arch/arm64/net/bpf_jit_comp.c:982:18: error: 'image_size' may be used uninitialized in this function [-Werror=maybe-uninitialized]Fixes db496944fdaa ("bpf: arm64: add JIT support for multi-function programs")Reported-by Arnd Bergmann <[email protected]>Signed-off-by Alexei Starovoitov <[email protected]>--- arch/arm64/net/bpf_jit_comp.c | 1 + 1 file changed, 1 insertion(+)arch/arm64/net/bpf_jit_comp.c +0
-0
arch/arm64/net/bpf_jit_comp.c struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog) · line 903
Quoted 7 lines
arch/arm64/net/bpf_jit_comp.c +1
-0@@ -897,6 +897,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog) image_ptr = jit_data->image; header = jit_data->header; extra_pass = true; + image_size =sizeof (u32) * ctx.idx; goto skip_init_ctx; }memset (&ctx,0 ,sizeof (ctx));
I don't really mind, but it feels more complex than it needs to be imho, since in the initial pass you fetch 'image_size' in fake pass from ctx.idx, then we set ctx.idx to 0 again, do another pass and use the cached ctx.idx from that second pass instead of the first one where we set 'image_size' originally, so we definitely need to take that into consideration in future reviews at least.
Thanks,
Daniel
Quoted 27 lines · Daniel Borkmann
Quoted 26 lines · Alexei Starovoitov
From: Alexei Starovoitov <[email protected]>
fix the following issue:
arch/arm64/net/bpf_jit_comp.c: In function 'bpf_int_jit_compile':arch/arm64/net/bpf_jit_comp.c:982:18: error: 'image_size' may be used uninitialized in this function [-Werror=maybe-uninitialized]Fixes db496944fdaa ("bpf: arm64: add JIT support for multi-function programs")Reported-by Arnd Bergmann <[email protected]>Signed-off-by Alexei Starovoitov <[email protected]>--- arch/arm64/net/bpf_jit_comp.c | 1 + 1 file changed, 1 insertion(+)arch/arm64/net/bpf_jit_comp.c +1
-0@@ -897,6 +897,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog) image_ptr = jit_data->image; header = jit_data->header; extra_pass = true; + image_size =sizeof (u32) * ctx.idx; goto skip_init_ctx; }memset (&ctx,0 ,sizeof (ctx));
Quoted 8 lines
Quoted 1 line
I don't really mind, but it feels more complex than it needs to be imho, since in the initial pass you fetch '
image_size' in fake pass from ctx.idx, then we set ctx.idx to 0 again, do another pass and use the cached ctx.idx from that second pass instead of the first one where we set 'image_size' originally, so we definitely need to take that into consideration in future reviews at least.
not sure what you mean.
This check: ctx.idx != jit_data->ctx.idx matters the most. After first alloc the 'image_size' variable used for dumping only. That's why the JITing itself worked fine. We could have removed it since it's computable from idx, but imo it's fine this way.
Daniel Borkmann
maintainerQuoted 36 lines · Alexei Starovoitov
Quoted 26 lines · Daniel Borkmann
Quoted 25 lines · Alexei Starovoitov
From: Alexei Starovoitov <[email protected]>
fix the following issue:
arch/arm64/net/bpf_jit_comp.c: In function 'bpf_int_jit_compile':arch/arm64/net/bpf_jit_comp.c:982:18: error: 'image_size' may be used uninitialized in this function [-Werror=maybe-uninitialized]Fixes db496944fdaa ("bpf: arm64: add JIT support for multi-function programs")Reported-by Arnd Bergmann <[email protected]>Signed-off-by Alexei Starovoitov <[email protected]>---
arch/arm64/net/bpf_jit_comp.c | 1 +1 file changed, 1 insertion(+)arch/arm64/net/bpf_jit_comp.c +1
-0@@ -897,6 +897,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog) image_ptr = jit_data->image; header = jit_data->header; extra_pass = true; + image_size = sizeof(u32) * ctx.idx; goto skip_init_ctx; }Quoted 8 lines
arch/arm64/net/bpf_jit_comp.c struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog) · line 903
Quoted 1 line
arch/arm64/net/bpf_jit_comp.c +0
-0memset(&ctx, 0, sizeof(ctx));I don't really mind, but it feels more complex than it needs to be imho, since in the initial pass you fetch '
image_size' in fake pass from ctx.idx, then we set ctx.idx to 0 again, do another pass and use the cached ctx.idx from that second pass instead of the first one where we set 'image_size' originally, so we definitely need to take that into consideration in future reviews at least.
Quoted 5 lines
not sure what you mean.
This check: ctx.idx !=jit_data->ctx.idxmatters the most. After first alloc the 'image_size' variable used for dumping only. That's why the JITing itself worked fine. We could have removed it since it's computable from idx, but imo it's fine this way.
Fair enough, given final ctx.idx value must be guaranteed to never change in future between pass#1 and pass#2 from the first bpf_int_jit_compile() run, then lets go with this smaller version; applied to bpf-next, thanks Alexei!