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: <58F8C9BA.3090707@iogearbox.net>
Date:   Thu, 20 Apr 2017 16:46:18 +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 04:32 PM, Johannes Berg wrote:
> On Thu, 2017-04-20 at 16:28 +0200, Daniel Borkmann wrote:
>>
>> I see what you mean now. Yes, that's fine. We already do something
>> similar essentially with skb->ifindex access already (skb->dev +
>> dev->ifindex), f.e.:
>>
>> [...]
>> 	case offsetof(struct __sk_buff, ifindex):
>> 		BUILD_BUG_ON(FIELD_SIZEOF(struct net_device, ifindex)
>> != 4);
>>
>> 		*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(struct sk_buff,
>> dev),
>> 				      si->dst_reg, si->src_reg,
>> 				      offsetof(struct sk_buff, dev));
>> 		*insn++ = BPF_JMP_IMM(BPF_JEQ, si->dst_reg, 0, 1);
>> 		*insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg,
>> 				      offsetof(struct net_device,
>> ifindex));
>> 		break;
>> [...]
>
> Oh, right, good point.
>
>> Which is not too different from the above. You'd probably need to
>> populate the struct wifi_data each time if you place it onto the
>> stack, but perhaps could be optimized by storing that somewhere
>> else (e.g. somewhere via netdev, etc) and walking the pointer from
>> there, which would also spare you the cb[] save/restore.
>
> Hmm. I don't see what "somewhere else" I could possibly have though,
> given that I want the (kernel-side) context to be "struct sk_buff *"
> to allow the skb helpers?

I have not enough context on the wireless side, perhaps could be
somewhere under skb->dev->ieee80211_ptr or so, iff suitable. But
it also really doesn't matter much since this is all transparently
handled in the kernel, meaning these kind of rewrites can still be
changed at a later point in time, f.e. if it's only 'u64 boottime_ns'
right now, that could live directly in the cb[] w/o extra pointer,
and should that grow to more members, then it could be moved behind
a pointer later on and it still works as-is from the program point
of view.