| Message ID | 20170411162252.84985-1-willemdebruijn.kernel@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tue, 2017-04-11 at 12:22 -0400, Willem de Bruijn wrote: > From: Willem de Bruijn <willemb@google.com> > > BPF helper functions access socket fields through skb->sk. This is not > set in ingress cgroup and socket filters. The association is only made > in skb_set_owner_r once the filter has accepted the packet. Sk is > available as socket lookup has taken place. > > Temporarily set skb->sk to sk in these cases. > > Signed-off-by: Willem de Bruijn <willemb@google.com> > --- > kernel/bpf/cgroup.c | 4 +++- > net/core/filter.c | 6 +++++- > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c > index da0f53690295..a2c387e433a4 100644 > --- a/kernel/bpf/cgroup.c > +++ b/kernel/bpf/cgroup.c > @@ -154,7 +154,7 @@ int __cgroup_bpf_update(struct cgroup *cgrp, struct cgroup *parent, > > /** > * __cgroup_bpf_run_filter_skb() - Run a program for packet filtering > - * @sk: The socken sending or receiving traffic > + * @sk: The socket sending or receiving traffic > * @skb: The skb that is being sent or received > * @type: The type of program to be exectuted > * > @@ -190,9 +190,11 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk, > if (prog) { > unsigned int offset = skb->data - skb_network_header(skb); > It would be nice to not touch sk. swap() is a bit overkill here I think. struct sock *save_sk = skb->sk; skb->sk = sk; > + swap(skb->sk, sk); > __skb_push(skb, offset); > ret = bpf_prog_run_save_cb(prog, skb) == 1 ? 0 : -EPERM; > __skb_pull(skb, offset); > + swap(skb->sk, sk); skb->sk = save_sk; > } >
On Tue, Apr 11, 2017 at 12:51 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Tue, 2017-04-11 at 12:22 -0400, Willem de Bruijn wrote: >> From: Willem de Bruijn <willemb@google.com> >> >> BPF helper functions access socket fields through skb->sk. This is not >> set in ingress cgroup and socket filters. The association is only made >> in skb_set_owner_r once the filter has accepted the packet. Sk is >> available as socket lookup has taken place. >> >> Temporarily set skb->sk to sk in these cases. >> >> Signed-off-by: Willem de Bruijn <willemb@google.com> >> --- >> kernel/bpf/cgroup.c | 4 +++- >> net/core/filter.c | 6 +++++- >> 2 files changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c >> index da0f53690295..a2c387e433a4 100644 >> --- a/kernel/bpf/cgroup.c >> +++ b/kernel/bpf/cgroup.c >> @@ -154,7 +154,7 @@ int __cgroup_bpf_update(struct cgroup *cgrp, struct cgroup *parent, >> >> /** >> * __cgroup_bpf_run_filter_skb() - Run a program for packet filtering >> - * @sk: The socken sending or receiving traffic >> + * @sk: The socket sending or receiving traffic >> * @skb: The skb that is being sent or received >> * @type: The type of program to be exectuted >> * >> @@ -190,9 +190,11 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk, >> if (prog) { >> unsigned int offset = skb->data - skb_network_header(skb); >> > > It would be nice to not touch sk. swap() is a bit overkill here I think. > > struct sock *save_sk = skb->sk; > > skb->sk = sk; > > >> + swap(skb->sk, sk); >> __skb_push(skb, offset); >> ret = bpf_prog_run_save_cb(prog, skb) == 1 ? 0 : -EPERM; >> __skb_pull(skb, offset); >> + swap(skb->sk, sk); > > skb->sk = save_sk; > >> } Thanks. I'll send a v2. >> > > >
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c index da0f53690295..a2c387e433a4 100644 --- a/kernel/bpf/cgroup.c +++ b/kernel/bpf/cgroup.c @@ -154,7 +154,7 @@ int __cgroup_bpf_update(struct cgroup *cgrp, struct cgroup *parent, /** * __cgroup_bpf_run_filter_skb() - Run a program for packet filtering - * @sk: The socken sending or receiving traffic + * @sk: The socket sending or receiving traffic * @skb: The skb that is being sent or received * @type: The type of program to be exectuted * @@ -190,9 +190,11 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk, if (prog) { unsigned int offset = skb->data - skb_network_header(skb); + swap(skb->sk, sk); __skb_push(skb, offset); ret = bpf_prog_run_save_cb(prog, skb) == 1 ? 0 : -EPERM; __skb_pull(skb, offset); + swap(skb->sk, sk); } rcu_read_unlock(); diff --git a/net/core/filter.c b/net/core/filter.c index 15e9a81ffebe..979cb508302a 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -92,8 +92,12 @@ int sk_filter_trim_cap(struct sock *sk, struct sk_buff *skb, unsigned int cap) rcu_read_lock(); filter = rcu_dereference(sk->sk_filter); if (filter) { - unsigned int pkt_len = bpf_prog_run_save_cb(filter->prog, skb); + unsigned int pkt_len; + + swap(skb->sk, sk); + pkt_len = bpf_prog_run_save_cb(filter->prog, skb); err = pkt_len ? pskb_trim(skb, max(cap, pkt_len)) : -EPERM; + swap(skb->sk, sk); } rcu_read_unlock();