netdev - Re: [PATCH net] net: filter: initialize A and X registers

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:	Wed, 23 Apr 2014 12:50:43 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	ast@...mgrid.com
Cc:	dborkman@...hat.com, netdev@...r.kernel.org
Subject: Re: [PATCH net] net: filter: initialize A and X registers

From: Alexei Starovoitov <ast@...mgrid.com>
Date: Tue, 22 Apr 2014 21:59:32 -0700

> On Tue, Apr 22, 2014 at 8:57 PM, David Miller <davem@...emloft.net> wrote:
>> From: Alexei Starovoitov <ast@...mgrid.com>
>> Date: Tue, 22 Apr 2014 20:18:57 -0700
>>
>>> exisiting BPF verifier allows uninitialized access to registers,
>>> 'ret A' is considered to be a valid filter.
>>> So initialize A and X to zero to prevent leaking kernel memory
>>> In the future BPF verifier will be rejecting such filters
>>>
>>> Signed-off-by: Alexei Starovoitov <ast@...mgrid.com>
>>
>> Has the code always been like this?
> 
> the previous interpreter had:
> unsigned int sk_run_filter(const struct sk_buff *skb,
>                            const struct sock_filter *fentry)
> {
>         void *ptr;
>         u32 A = 0;                      /* Accumulator */
>         u32 X = 0;                      /* Index Register */
> ...
> so it wasn't affected.
> 
>> Did the eBPF changes introduce this problem either directly or
>> indirectly?
> 
> this bug is an oversight on my side.
> I believe in the net-next it should be fixed by making verifier smarter
> instead of wasting run time cycles to initialize regs.
> For now the same fix is needed for both net and net-next.
> We can remove extra assignments when verifier becomes smarter.
> ebpf verifier that I posted earlier had checks for uninitialized regs and stack.
> I missed lack of uninit regs part in classic verifier when ebpf verifier and jit
> were dropped from the patch set.

I think you will need to add the initializations during validation
rather than fail because existing BPF filters would be accepted and
run.  You can't just make them fail unexpectedly after all of these
years.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html