netdev - Re: __sk_buff.data

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: <58F8C160.6010905@iogearbox.net>
Date:   Thu, 20 Apr 2017 16:10:40 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Johannes Berg <johannes@...solutions.net>,
        Alexei Starovoitov <ast@...nel.org>
CC:     netdev <netdev@...r.kernel.org>
Subject: Re: __sk_buff.data_end

On 04/20/2017 08:01 AM, Johannes Berg wrote:
> On Thu, 2017-04-20 at 02:01 +0200, Daniel Borkmann wrote:
>>
>> Yeah, should work as well for the 32 bit archs, on 64 bit we
>> have this effectively already:
>
> Right.
>
> [...]
>
>> Can you elaborate on why this works for mac80211? It uses cb
>> only up to that point from where you invoke the prog?
>
> No, it works because then I can move a u64 field to the same offset,
> and save/restore it across the BPF call :)

Right.

> But I don't have a *pointer* field to move there, and no space for the
> alignment anyway (already using all 48 bytes).
>
> Come to think of it - somebody had proposed extensions to this by
> passing an on-stack pointer in addition to the data in the cb.
>
> Perhaps we can extend BPF to have an optional second argument, and
> track a second context around the verifier, if applicable? Then we can
> solve all of this really easily, because it means we don't always have
> to go from the SKB context but could go from the other one (which could
> be that on-stack buffer).

I think this would be a rather more complex operation on the BPF side,
it would need changes from LLVM (which assumes initial ctx sits in r1),
verifier for tracking this ctx2, all the way down to JITs plus some way
to handle 1 and 2 argument program calls generically. Much easier to
pass additional meta data for the program via cb[], for example.

> Alternatively I can clear another pointer (u64) in the CB, store a
> pointer there, and always emit code following that pointer - should be
> possible right?

What kind of pointer? If it's something like data_end as read-only, then
this needs to be tracked in the verifier in addition, of course. Other
option you could do (depending on what you want to achieve) is to have
a bpf_probe_read() version as a helper for your prog type that would
further walk that pointer/struct (similar to tracing) where this comes
w/o any backward compat guarantees, though.