* [PATCH bpf-next v1] bpf: Fix bpf/sk_skb_pull_data for flags == 0
@ 2022-07-13 1:26 Joanne Koong
2022-07-13 7:23 ` Daniel Borkmann
0 siblings, 1 reply; 3+ messages in thread
From: Joanne Koong @ 2022-07-13 1:26 UTC (permalink / raw)
To: bpf; +Cc: andrii, daniel, ast, Joanne Koong
In the case where flags is 0, bpf_skb_pull_data and sk_skb_pull_data
should pull the entire skb payload including the bytes in the non-linear
page buffers.
This is documented in the uapi:
"If a zero value is passed for *len*, then the whole length of the *skb*
is pulled"
Fixes: 36bbef52c7eb6 ("bpf: direct packet write and access for helpers
for clsact progs")
Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
---
net/core/filter.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/filter.c b/net/core/filter.c
index 4ef77ec5255e..97eb15891bfc 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1838,7 +1838,7 @@ BPF_CALL_2(bpf_skb_pull_data, struct sk_buff *, skb, u32, len)
* access case. By this we overcome limitations of only current
* headroom being accessible.
*/
- return bpf_try_make_writable(skb, len ? : skb_headlen(skb));
+ return bpf_try_make_writable(skb, len ? : skb->len);
}
static const struct bpf_func_proto bpf_skb_pull_data_proto = {
@@ -1878,7 +1878,7 @@ BPF_CALL_2(sk_skb_pull_data, struct sk_buff *, skb, u32, len)
* access case. By this we overcome limitations of only current
* headroom being accessible.
*/
- return sk_skb_try_make_writable(skb, len ? : skb_headlen(skb));
+ return sk_skb_try_make_writable(skb, len ? : skb->len);
}
static const struct bpf_func_proto sk_skb_pull_data_proto = {
--
2.30.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next v1] bpf: Fix bpf/sk_skb_pull_data for flags == 0
2022-07-13 1:26 [PATCH bpf-next v1] bpf: Fix bpf/sk_skb_pull_data for flags == 0 Joanne Koong
@ 2022-07-13 7:23 ` Daniel Borkmann
2022-07-13 20:50 ` Joanne Koong
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Borkmann @ 2022-07-13 7:23 UTC (permalink / raw)
To: Joanne Koong, bpf; +Cc: andrii, ast
On 7/13/22 3:26 AM, Joanne Koong wrote:
> In the case where flags is 0, bpf_skb_pull_data and sk_skb_pull_data
> should pull the entire skb payload including the bytes in the non-linear
> page buffers.
>
> This is documented in the uapi:
> "If a zero value is passed for *len*, then the whole length of the *skb*
> is pulled"
>
> Fixes: 36bbef52c7eb6 ("bpf: direct packet write and access for helpers
> for clsact progs")
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
This is not correct. We should fix the helper doc fa15601ab31e ("bpf: add
documentation for eBPF helpers (33-41)"). It will make the head private
for writing (e.g. for direct packet access), but not linearize the entire
skb, so skb_headlen is correct here.
> ---
> net/core/filter.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 4ef77ec5255e..97eb15891bfc 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -1838,7 +1838,7 @@ BPF_CALL_2(bpf_skb_pull_data, struct sk_buff *, skb, u32, len)
> * access case. By this we overcome limitations of only current
> * headroom being accessible.
> */
> - return bpf_try_make_writable(skb, len ? : skb_headlen(skb));
> + return bpf_try_make_writable(skb, len ? : skb->len);
> }
>
> static const struct bpf_func_proto bpf_skb_pull_data_proto = {
> @@ -1878,7 +1878,7 @@ BPF_CALL_2(sk_skb_pull_data, struct sk_buff *, skb, u32, len)
> * access case. By this we overcome limitations of only current
> * headroom being accessible.
> */
> - return sk_skb_try_make_writable(skb, len ? : skb_headlen(skb));
> + return sk_skb_try_make_writable(skb, len ? : skb->len);
> }
>
> static const struct bpf_func_proto sk_skb_pull_data_proto = {
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next v1] bpf: Fix bpf/sk_skb_pull_data for flags == 0
2022-07-13 7:23 ` Daniel Borkmann
@ 2022-07-13 20:50 ` Joanne Koong
0 siblings, 0 replies; 3+ messages in thread
From: Joanne Koong @ 2022-07-13 20:50 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: bpf, Andrii Nakryiko, Alexei Starovoitov
On Wed, Jul 13, 2022 at 12:23 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 7/13/22 3:26 AM, Joanne Koong wrote:
> > In the case where flags is 0, bpf_skb_pull_data and sk_skb_pull_data
> > should pull the entire skb payload including the bytes in the non-linear
> > page buffers.
> >
> > This is documented in the uapi:
> > "If a zero value is passed for *len*, then the whole length of the *skb*
> > is pulled"
> >
> > Fixes: 36bbef52c7eb6 ("bpf: direct packet write and access for helpers
> > for clsact progs")
> > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
>
> This is not correct. We should fix the helper doc fa15601ab31e ("bpf: add
> documentation for eBPF helpers (33-41)"). It will make the head private
> for writing (e.g. for direct packet access), but not linearize the entire
> skb, so skb_headlen is correct here.
>
Great, I'll fix up the uapi doc. I'll change it to something like: "If
a zero value is passed for *len*, then all bytes in the head of the
skb will be made readable and writable".
> > ---
> > net/core/filter.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index 4ef77ec5255e..97eb15891bfc 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -1838,7 +1838,7 @@ BPF_CALL_2(bpf_skb_pull_data, struct sk_buff *, skb, u32, len)
> > * access case. By this we overcome limitations of only current
> > * headroom being accessible.
> > */
> > - return bpf_try_make_writable(skb, len ? : skb_headlen(skb));
> > + return bpf_try_make_writable(skb, len ? : skb->len);
> > }
> >
> > static const struct bpf_func_proto bpf_skb_pull_data_proto = {
> > @@ -1878,7 +1878,7 @@ BPF_CALL_2(sk_skb_pull_data, struct sk_buff *, skb, u32, len)
> > * access case. By this we overcome limitations of only current
> > * headroom being accessible.
> > */
> > - return sk_skb_try_make_writable(skb, len ? : skb_headlen(skb));
> > + return sk_skb_try_make_writable(skb, len ? : skb->len);
> > }
> >
> > static const struct bpf_func_proto sk_skb_pull_data_proto = {
> >
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-07-13 20:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-13 1:26 [PATCH bpf-next v1] bpf: Fix bpf/sk_skb_pull_data for flags == 0 Joanne Koong
2022-07-13 7:23 ` Daniel Borkmann
2022-07-13 20:50 ` Joanne Koong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.