netdev - Re: running an eBPF program

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]

Message-Id: <20170527.201107.1558140531080967167.davem@davemloft.net>
Date:   Sat, 27 May 2017 20:11:07 -0400 (EDT)
From:   David Miller <davem@...emloft.net>
To:     ys114321@...il.com
Cc:     adelfuchs@...il.com, netdev@...r.kernel.org
Subject: Re: running an eBPF program

From: Y Song <ys114321@...il.com>
Date: Sat, 27 May 2017 13:52:27 -0700

> On Sat, May 27, 2017 at 1:23 PM, Y Song <ys114321@...il.com> wrote:
>>
>> From verifier error message:
>> ======
>> 0: (bf) r6 = r1
>>
>> 1: (18) r9 = 0xffe0000e
>>
>> 3: (69) r0 = *(u16 *)(r6 +16)
>>
>> invalid bpf_context access off=16 size=2
>> ======
>>
>> The offset 16 of struct __sk_buff is hash.
>> What instruction #3 tries to do is to access 2 bytes of the hash value
>> instead of full 4 bytes.
>> This is explicitly not allowed in verifier due to endianness issue.
> 
> 
> I can reproduce the issue now. My previous statement saying to access
> "hash" field is not correct. It is accessing the protocol field.
> 
> static __inline__ bool flow_dissector(struct __sk_buff *skb,
>                                       struct flow_keys *flow)
> {
>         int poff, nh_off = BPF_LL_OFF + ETH_HLEN;
>         __be16 proto = skb->protocol;
>         __u8 ip_proto;
> 
> The plan so far is to see whether we can fix the issue in LLVM side.

If the compiler properly asks for "__sk_buff + 16" on little-endian
and "__sk_buff + 20" on big-endian, the verifier should instead be
fixed to allow the access to pass.

I can't see any reason why LLVM won't set the offset properly like
that, and it's a completely legitimate optimization that we shouldn't
try to stop LLVM from performing.

It also makes it so that we don't have to fix having absurdly defined
__sk_buff's protocol field as a u32.

Thanks.