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: <1492697865.3109.7.camel@sipsolutions.net>
Date:   Thu, 20 Apr 2017 16:17:45 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>
Cc:     netdev <netdev@...r.kernel.org>
Subject: Re: __sk_buff.data_end

On Thu, 2017-04-20 at 16:10 +0200, Daniel Borkmann wrote:
> 
> 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.

Yeah, it did seem very complex :)

> > 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.

I meant something like this

struct wifi_cb {
	struct wifi_data *wifi_data;
	...
	void *data_end; // with BUILD_BUG_ON to the right offset
};

Then struct wifi_data can contain extra data that doesn't fit into
wifi_cb, like the stuff I evicted for *data_end and *wifi_data. Let's
say one of those fields is "u64 boottime_ns;" (as I did in my patch
now), so we have

struct wifi_data {
	u64 boottime_ns;
};

then I can still have

struct __wifi_sk_buff {
	u32 len;
	u32 data;
	u32 data_end;
	u32 boottime_ns; // this is strange but
			 // seems to be done this way?
};

And then when boottime_ns is accessed, I can have:

        case offsetof(struct __wifi_sk_buff, boottime_ns):
                off  = si->off;
                off -= offsetof(struct __wifi_sk_buff, boottime_ns);
                off += offsetof(struct sk_buff, cb);
                off += offsetof(struct wifi_cb, wifi_data);
                *insn++ = BPF_LDX_MEM(BPF_SIZEOF(void *), si->dst_reg,
                                      si->src_reg, off);
		off = offsetof(struct wifi_data, boottime_ns);
		*isns++ = BPF_LDX_MEM(BPF_SIZEOF(u64), si->dst_reg,
				      si->src_reg, off);
                break;

no?

It seems to me this should work, and essentially emit code to follow
the pointer to inside struct wifi_data. Assuming 

johannes