netdev - Re: [PATCH bpf-next 12/13] bpf: arm64: add JIT support for multi-function programs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 18 Dec 2017 16:51:37 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Arnd Bergmann <arnd@...db.de>, Alexei Starovoitov <ast@...nel.org>
Cc:     "David S . Miller" <davem@...emloft.net>,
        John Fastabend <john.fastabend@...il.com>,
        Edward Cree <ecree@...arflare.com>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        Networking <netdev@...r.kernel.org>, kernel-team@...com
Subject: Re: [PATCH bpf-next 12/13] bpf: arm64: add JIT support for
 multi-function programs

On 12/18/2017 04:29 PM, Arnd Bergmann wrote:
> On Fri, Dec 15, 2017 at 2:55 AM, Alexei Starovoitov <ast@...nel.org> wrote:
> 
> 
>> +       if (jit_data->ctx.offset) {
>> +               ctx = jit_data->ctx;
>> +               image_ptr = jit_data->image;
>> +               header = jit_data->header;
>> +               extra_pass = true;
>> +               goto skip_init_ctx;
>> +       }
>>         memset(&ctx, 0, sizeof(ctx));
>>         ctx.prog = prog;
> 
> The 'goto' jumps over the 'image_size' initialization
> 
>>         prog->bpf_func = (void *)ctx.image;
>>         prog->jited = 1;
>>         prog->jited_len = image_size;
> 
> so we now get a warning here, starting with linux-next-20171218:
> 
> 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]
> 
> I could not figure out what the code should be doing instead, or if it is
> indeed safe and the warning is a false-positive.

Good catch, it's buggy indeed. Fix like below is needed; I can submit
it properly a bit later today:

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 396490c..a6fd585 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -855,6 +855,7 @@ static inline void bpf_flush_icache(void *start, void *end)
 struct arm64_jit_data {
 	struct bpf_binary_header *header;
 	u8 *image;
+	int image_size;
 	struct jit_ctx ctx;
 };

@@ -895,6 +896,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 	if (jit_data->ctx.offset) {
 		ctx = jit_data->ctx;
 		image_ptr = jit_data->image;
+		image_size = jit_data->image_size;
 		header = jit_data->header;
 		extra_pass = true;
 		goto skip_init_ctx;
@@ -975,6 +977,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 	} else {
 		jit_data->ctx = ctx;
 		jit_data->image = image_ptr;
+		jit_data->image_size = image_size;
 		jit_data->header = header;
 	}
 	prog->bpf_func = (void *)ctx.image;