Re: [PATCH bpf-next v6 2/5] bpf: Expose bpf_get_socket_cookie to tracing programs
On Mon, Feb 1, 2021 at 11:37 PM Alexei Starovoitov
wrote:
>
> On Mon, Feb 1, 2021 at 2:32 PM Daniel Borkmann wrote:
> >
> > On 1/30/21 12:45 PM, Florent Revest wrote:
> > > On Fri, Jan 29, 2021 at 1:49 PM Daniel Borkmann
> > > wrote:
> > >> On 1/29/21 11:57 AM, Daniel Borkmann wrote:
> > >>> On 1/27/21 10:01 PM, Andrii Nakryiko wrote:
> > On Tue, Jan 26, 2021 at 10:36 AM Florent Revest
> > wrote:
> > >
> > > This needs a new helper that:
> > > - can work in a sleepable context (using sock_gen_cookie)
> > > - takes a struct sock pointer and checks that it's not NULL
> > >
> > > Signed-off-by: Florent Revest
> > > Acked-by: KP Singh
> > > ---
> > >include/linux/bpf.h| 1 +
> > >include/uapi/linux/bpf.h | 8
> > >kernel/trace/bpf_trace.c | 2 ++
> > >net/core/filter.c | 12
> > >tools/include/uapi/linux/bpf.h | 8
> > >5 files changed, 31 insertions(+)
> > >
> > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > > index 1aac2af12fed..26219465e1f7 100644
> > > --- a/include/linux/bpf.h
> > > +++ b/include/linux/bpf.h
> > > @@ -1874,6 +1874,7 @@ extern const struct bpf_func_proto
> > > bpf_per_cpu_ptr_proto;
> > >extern const struct bpf_func_proto bpf_this_cpu_ptr_proto;
> > >extern const struct bpf_func_proto bpf_ktime_get_coarse_ns_proto;
> > >extern const struct bpf_func_proto bpf_sock_from_file_proto;
> > > +extern const struct bpf_func_proto bpf_get_socket_ptr_cookie_proto;
> > >
> > >const struct bpf_func_proto *bpf_tracing_func_proto(
> > > enum bpf_func_id func_id, const struct bpf_prog *prog);
> > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > index 0b735c2729b2..5855c398d685 100644
> > > --- a/include/uapi/linux/bpf.h
> > > +++ b/include/uapi/linux/bpf.h
> > > @@ -1673,6 +1673,14 @@ union bpf_attr {
> > > * Return
> > > * A 8-byte long unique number.
> > > *
> > > + * u64 bpf_get_socket_cookie(void *sk)
> >
> > should the type be `struct sock *` then?
> > >>>
> > >>> Checking libbpf's generated bpf_helper_defs.h it generates:
> > >>>
> > >>> /*
> > >>>* bpf_get_socket_cookie
> > >>>*
> > >>>* If the **struct sk_buff** pointed by *skb* has a known socket,
> > >>>* retrieve the cookie (generated by the kernel) of this socket.
> > >>>* If no cookie has been set yet, generate a new cookie. Once
> > >>>* generated, the socket cookie remains stable for the life of
> > >>> the
> > >>>* socket. This helper can be useful for monitoring per socket
> > >>>* networking traffic statistics as it provides a global socket
> > >>>* identifier that can be assumed unique.
> > >>>*
> > >>>* Returns
> > >>>* A 8-byte long non-decreasing number on success, or 0 if the
> > >>>* socket field is missing inside *skb*.
> > >>>*/
> > >>> static __u64 (*bpf_get_socket_cookie)(void *ctx) = (void *) 46;
> > >>>
> > >>> So in terms of helper comment it's picking up the description from the
> > >>> `u64 bpf_get_socket_cookie(struct sk_buff *skb)` signature. With that
> > >>> in mind it would likely make sense to add the actual `struct sock *`
> > >>> type
> > >>> to the comment to make it more clear in here.
> > >>
> > >> One thought that still came to mind when looking over the series again,
> > >> do
> > >> we need to blacklist certain functions from bpf_get_socket_cookie() under
> > >> tracing e.g. when attaching to, say fexit? For example, if sk_prot_free()
> > >> would be temporary uninlined/exported for testing and
> > >> bpf_get_socket_cookie()
> > >> was invoked from a prog upon fexit where sock was already passed back to
> > >> allocator, I presume there's risk of mem corruption, no?
> > >
> > > Mh, this is interesting. I can try to add a deny list in v7 but I'm
> > > not sure whether I'll be able to catch them all. I'm assuming that
> > > __sk_destruct, sk_destruct, __sk_free, sk_free would be other
> > > problematic functions but potentially there would be more.
> >
> > I was just looking at bpf_skb_output() from a7658e1a4164 ("bpf: Check types
> > of
> > arguments passed into helpers") which afaiu has similar issue, back at the
> > time
> > this was introduced there was no fentry/fexit but rather raw tp progs, so
> > you
> > could still safely dump skb this way including for kfree_skb() tp.
> > Presumably if
> > you bpf_skb_output() at 'fexit/kfree_skb' you might be able to similarly
> > crash
>
> the verifier cannot check absolutely everything.
> Whitelisting and blacklisting all combinations is not practical.
Ok, I'm sending a v7 that only changes the signature to take a struct
sock * argument then and I won't be adding an allow or deny list in
this series.
Re: [PATCH bpf-next v6 2/5] bpf: Expose bpf_get_socket_cookie to tracing programs
On Mon, Feb 1, 2021 at 2:32 PM Daniel Borkmann wrote:
>
> On 1/30/21 12:45 PM, Florent Revest wrote:
> > On Fri, Jan 29, 2021 at 1:49 PM Daniel Borkmann
> > wrote:
> >> On 1/29/21 11:57 AM, Daniel Borkmann wrote:
> >>> On 1/27/21 10:01 PM, Andrii Nakryiko wrote:
> On Tue, Jan 26, 2021 at 10:36 AM Florent Revest
> wrote:
> >
> > This needs a new helper that:
> > - can work in a sleepable context (using sock_gen_cookie)
> > - takes a struct sock pointer and checks that it's not NULL
> >
> > Signed-off-by: Florent Revest
> > Acked-by: KP Singh
> > ---
> >include/linux/bpf.h| 1 +
> >include/uapi/linux/bpf.h | 8
> >kernel/trace/bpf_trace.c | 2 ++
> >net/core/filter.c | 12
> >tools/include/uapi/linux/bpf.h | 8
> >5 files changed, 31 insertions(+)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 1aac2af12fed..26219465e1f7 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -1874,6 +1874,7 @@ extern const struct bpf_func_proto
> > bpf_per_cpu_ptr_proto;
> >extern const struct bpf_func_proto bpf_this_cpu_ptr_proto;
> >extern const struct bpf_func_proto bpf_ktime_get_coarse_ns_proto;
> >extern const struct bpf_func_proto bpf_sock_from_file_proto;
> > +extern const struct bpf_func_proto bpf_get_socket_ptr_cookie_proto;
> >
> >const struct bpf_func_proto *bpf_tracing_func_proto(
> > enum bpf_func_id func_id, const struct bpf_prog *prog);
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 0b735c2729b2..5855c398d685 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -1673,6 +1673,14 @@ union bpf_attr {
> > * Return
> > * A 8-byte long unique number.
> > *
> > + * u64 bpf_get_socket_cookie(void *sk)
>
> should the type be `struct sock *` then?
> >>>
> >>> Checking libbpf's generated bpf_helper_defs.h it generates:
> >>>
> >>> /*
> >>>* bpf_get_socket_cookie
> >>>*
> >>>* If the **struct sk_buff** pointed by *skb* has a known socket,
> >>>* retrieve the cookie (generated by the kernel) of this socket.
> >>>* If no cookie has been set yet, generate a new cookie. Once
> >>>* generated, the socket cookie remains stable for the life of the
> >>>* socket. This helper can be useful for monitoring per socket
> >>>* networking traffic statistics as it provides a global socket
> >>>* identifier that can be assumed unique.
> >>>*
> >>>* Returns
> >>>* A 8-byte long non-decreasing number on success, or 0 if the
> >>>* socket field is missing inside *skb*.
> >>>*/
> >>> static __u64 (*bpf_get_socket_cookie)(void *ctx) = (void *) 46;
> >>>
> >>> So in terms of helper comment it's picking up the description from the
> >>> `u64 bpf_get_socket_cookie(struct sk_buff *skb)` signature. With that
> >>> in mind it would likely make sense to add the actual `struct sock *` type
> >>> to the comment to make it more clear in here.
> >>
> >> One thought that still came to mind when looking over the series again, do
> >> we need to blacklist certain functions from bpf_get_socket_cookie() under
> >> tracing e.g. when attaching to, say fexit? For example, if sk_prot_free()
> >> would be temporary uninlined/exported for testing and
> >> bpf_get_socket_cookie()
> >> was invoked from a prog upon fexit where sock was already passed back to
> >> allocator, I presume there's risk of mem corruption, no?
> >
> > Mh, this is interesting. I can try to add a deny list in v7 but I'm
> > not sure whether I'll be able to catch them all. I'm assuming that
> > __sk_destruct, sk_destruct, __sk_free, sk_free would be other
> > problematic functions but potentially there would be more.
>
> I was just looking at bpf_skb_output() from a7658e1a4164 ("bpf: Check types of
> arguments passed into helpers") which afaiu has similar issue, back at the
> time
> this was introduced there was no fentry/fexit but rather raw tp progs, so you
> could still safely dump skb this way including for kfree_skb() tp. Presumably
> if
> you bpf_skb_output() at 'fexit/kfree_skb' you might be able to similarly crash
the verifier cannot check absolutely everything.
Whitelisting and blacklisting all combinations is not practical.
> your kernel which I don't think is intentional (also given we go above and
> beyond
> in other areas to avoid crashing or destabilizing e.g. [0] to mention one).
> Maybe
> these should really only be for BPF_TRACE_RAW_TP (or BPF_PROG_TYPE_LSM) where
> it
> can be audited that it's safe to use like a7658e1a4164's original intention
> ...
> or have some sort of function annotation like __acquires/__releases but for
> tracing
> e.g.
Re: [PATCH bpf-next v6 2/5] bpf: Expose bpf_get_socket_cookie to tracing programs
On 1/30/21 12:45 PM, Florent Revest wrote:
On Fri, Jan 29, 2021 at 1:49 PM Daniel Borkmann wrote:
On 1/29/21 11:57 AM, Daniel Borkmann wrote:
On 1/27/21 10:01 PM, Andrii Nakryiko wrote:
On Tue, Jan 26, 2021 at 10:36 AM Florent Revest wrote:
This needs a new helper that:
- can work in a sleepable context (using sock_gen_cookie)
- takes a struct sock pointer and checks that it's not NULL
Signed-off-by: Florent Revest
Acked-by: KP Singh
---
include/linux/bpf.h| 1 +
include/uapi/linux/bpf.h | 8
kernel/trace/bpf_trace.c | 2 ++
net/core/filter.c | 12
tools/include/uapi/linux/bpf.h | 8
5 files changed, 31 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 1aac2af12fed..26219465e1f7 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1874,6 +1874,7 @@ extern const struct bpf_func_proto bpf_per_cpu_ptr_proto;
extern const struct bpf_func_proto bpf_this_cpu_ptr_proto;
extern const struct bpf_func_proto bpf_ktime_get_coarse_ns_proto;
extern const struct bpf_func_proto bpf_sock_from_file_proto;
+extern const struct bpf_func_proto bpf_get_socket_ptr_cookie_proto;
const struct bpf_func_proto *bpf_tracing_func_proto(
enum bpf_func_id func_id, const struct bpf_prog *prog);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 0b735c2729b2..5855c398d685 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1673,6 +1673,14 @@ union bpf_attr {
* Return
* A 8-byte long unique number.
*
+ * u64 bpf_get_socket_cookie(void *sk)
should the type be `struct sock *` then?
Checking libbpf's generated bpf_helper_defs.h it generates:
/*
* bpf_get_socket_cookie
*
* If the **struct sk_buff** pointed by *skb* has a known socket,
* retrieve the cookie (generated by the kernel) of this socket.
* If no cookie has been set yet, generate a new cookie. Once
* generated, the socket cookie remains stable for the life of the
* socket. This helper can be useful for monitoring per socket
* networking traffic statistics as it provides a global socket
* identifier that can be assumed unique.
*
* Returns
* A 8-byte long non-decreasing number on success, or 0 if the
* socket field is missing inside *skb*.
*/
static __u64 (*bpf_get_socket_cookie)(void *ctx) = (void *) 46;
So in terms of helper comment it's picking up the description from the
`u64 bpf_get_socket_cookie(struct sk_buff *skb)` signature. With that
in mind it would likely make sense to add the actual `struct sock *` type
to the comment to make it more clear in here.
One thought that still came to mind when looking over the series again, do
we need to blacklist certain functions from bpf_get_socket_cookie() under
tracing e.g. when attaching to, say fexit? For example, if sk_prot_free()
would be temporary uninlined/exported for testing and bpf_get_socket_cookie()
was invoked from a prog upon fexit where sock was already passed back to
allocator, I presume there's risk of mem corruption, no?
Mh, this is interesting. I can try to add a deny list in v7 but I'm
not sure whether I'll be able to catch them all. I'm assuming that
__sk_destruct, sk_destruct, __sk_free, sk_free would be other
problematic functions but potentially there would be more.
I was just looking at bpf_skb_output() from a7658e1a4164 ("bpf: Check types of
arguments passed into helpers") which afaiu has similar issue, back at the time
this was introduced there was no fentry/fexit but rather raw tp progs, so you
could still safely dump skb this way including for kfree_skb() tp. Presumably if
you bpf_skb_output() at 'fexit/kfree_skb' you might be able to similarly crash
your kernel which I don't think is intentional (also given we go above and
beyond
in other areas to avoid crashing or destabilizing e.g. [0] to mention one).
Maybe
these should really only be for BPF_TRACE_RAW_TP (or BPF_PROG_TYPE_LSM) where it
can be audited that it's safe to use like a7658e1a4164's original intention ...
or have some sort of function annotation like __acquires/__releases but for
tracing
e.g. __frees(skb) where use would then be blocked (not sure iff feasible).
[0] https://lore.kernel.org/bpf/20210126001219.845816-1-...@fb.com/
Re: [PATCH bpf-next v6 2/5] bpf: Expose bpf_get_socket_cookie to tracing programs
On Fri, Jan 29, 2021 at 1:49 PM Daniel Borkmann wrote:
>
> On 1/29/21 11:57 AM, Daniel Borkmann wrote:
> > On 1/27/21 10:01 PM, Andrii Nakryiko wrote:
> >> On Tue, Jan 26, 2021 at 10:36 AM Florent Revest
> >> wrote:
> >>>
> >>> This needs a new helper that:
> >>> - can work in a sleepable context (using sock_gen_cookie)
> >>> - takes a struct sock pointer and checks that it's not NULL
> >>>
> >>> Signed-off-by: Florent Revest
> >>> Acked-by: KP Singh
> >>> ---
> >>> include/linux/bpf.h| 1 +
> >>> include/uapi/linux/bpf.h | 8
> >>> kernel/trace/bpf_trace.c | 2 ++
> >>> net/core/filter.c | 12
> >>> tools/include/uapi/linux/bpf.h | 8
> >>> 5 files changed, 31 insertions(+)
> >>>
> >>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> >>> index 1aac2af12fed..26219465e1f7 100644
> >>> --- a/include/linux/bpf.h
> >>> +++ b/include/linux/bpf.h
> >>> @@ -1874,6 +1874,7 @@ extern const struct bpf_func_proto
> >>> bpf_per_cpu_ptr_proto;
> >>> extern const struct bpf_func_proto bpf_this_cpu_ptr_proto;
> >>> extern const struct bpf_func_proto bpf_ktime_get_coarse_ns_proto;
> >>> extern const struct bpf_func_proto bpf_sock_from_file_proto;
> >>> +extern const struct bpf_func_proto bpf_get_socket_ptr_cookie_proto;
> >>>
> >>> const struct bpf_func_proto *bpf_tracing_func_proto(
> >>> enum bpf_func_id func_id, const struct bpf_prog *prog);
> >>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> >>> index 0b735c2729b2..5855c398d685 100644
> >>> --- a/include/uapi/linux/bpf.h
> >>> +++ b/include/uapi/linux/bpf.h
> >>> @@ -1673,6 +1673,14 @@ union bpf_attr {
> >>>* Return
> >>>* A 8-byte long unique number.
> >>>*
> >>> + * u64 bpf_get_socket_cookie(void *sk)
> >>
> >> should the type be `struct sock *` then?
> >
> > Checking libbpf's generated bpf_helper_defs.h it generates:
> >
> > /*
> > * bpf_get_socket_cookie
> > *
> > * If the **struct sk_buff** pointed by *skb* has a known socket,
> > * retrieve the cookie (generated by the kernel) of this socket.
> > * If no cookie has been set yet, generate a new cookie. Once
> > * generated, the socket cookie remains stable for the life of the
> > * socket. This helper can be useful for monitoring per socket
> > * networking traffic statistics as it provides a global socket
> > * identifier that can be assumed unique.
> > *
> > * Returns
> > * A 8-byte long non-decreasing number on success, or 0 if the
> > * socket field is missing inside *skb*.
> > */
> > static __u64 (*bpf_get_socket_cookie)(void *ctx) = (void *) 46;
> >
> > So in terms of helper comment it's picking up the description from the
> > `u64 bpf_get_socket_cookie(struct sk_buff *skb)` signature. With that
> > in mind it would likely make sense to add the actual `struct sock *` type
> > to the comment to make it more clear in here.
>
> One thought that still came to mind when looking over the series again, do
> we need to blacklist certain functions from bpf_get_socket_cookie() under
> tracing e.g. when attaching to, say fexit? For example, if sk_prot_free()
> would be temporary uninlined/exported for testing and bpf_get_socket_cookie()
> was invoked from a prog upon fexit where sock was already passed back to
> allocator, I presume there's risk of mem corruption, no?
Mh, this is interesting. I can try to add a deny list in v7 but I'm
not sure whether I'll be able to catch them all. I'm assuming that
__sk_destruct, sk_destruct, __sk_free, sk_free would be other
problematic functions but potentially there would be more.
Re: [PATCH bpf-next v6 2/5] bpf: Expose bpf_get_socket_cookie to tracing programs
On Wed, Jan 27, 2021 at 10:01 PM Andrii Nakryiko
wrote:
>
> On Tue, Jan 26, 2021 at 10:36 AM Florent Revest wrote:
> >
> > This needs a new helper that:
> > - can work in a sleepable context (using sock_gen_cookie)
> > - takes a struct sock pointer and checks that it's not NULL
> >
> > Signed-off-by: Florent Revest
> > Acked-by: KP Singh
> > ---
> > include/linux/bpf.h| 1 +
> > include/uapi/linux/bpf.h | 8
> > kernel/trace/bpf_trace.c | 2 ++
> > net/core/filter.c | 12
> > tools/include/uapi/linux/bpf.h | 8
> > 5 files changed, 31 insertions(+)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 1aac2af12fed..26219465e1f7 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -1874,6 +1874,7 @@ extern const struct bpf_func_proto
> > bpf_per_cpu_ptr_proto;
> > extern const struct bpf_func_proto bpf_this_cpu_ptr_proto;
> > extern const struct bpf_func_proto bpf_ktime_get_coarse_ns_proto;
> > extern const struct bpf_func_proto bpf_sock_from_file_proto;
> > +extern const struct bpf_func_proto bpf_get_socket_ptr_cookie_proto;
> >
> > const struct bpf_func_proto *bpf_tracing_func_proto(
> > enum bpf_func_id func_id, const struct bpf_prog *prog);
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 0b735c2729b2..5855c398d685 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -1673,6 +1673,14 @@ union bpf_attr {
> > * Return
> > * A 8-byte long unique number.
> > *
> > + * u64 bpf_get_socket_cookie(void *sk)
>
> should the type be `struct sock *` then?
Oh, absolutely. :) Thank you Andrii
Re: [PATCH bpf-next v6 2/5] bpf: Expose bpf_get_socket_cookie to tracing programs
On Tue, Jan 26, 2021 at 10:36 AM Florent Revest wrote:
>
> This needs a new helper that:
> - can work in a sleepable context (using sock_gen_cookie)
> - takes a struct sock pointer and checks that it's not NULL
>
> Signed-off-by: Florent Revest
> Acked-by: KP Singh
> ---
> include/linux/bpf.h| 1 +
> include/uapi/linux/bpf.h | 8
> kernel/trace/bpf_trace.c | 2 ++
> net/core/filter.c | 12
> tools/include/uapi/linux/bpf.h | 8
> 5 files changed, 31 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 1aac2af12fed..26219465e1f7 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1874,6 +1874,7 @@ extern const struct bpf_func_proto
> bpf_per_cpu_ptr_proto;
> extern const struct bpf_func_proto bpf_this_cpu_ptr_proto;
> extern const struct bpf_func_proto bpf_ktime_get_coarse_ns_proto;
> extern const struct bpf_func_proto bpf_sock_from_file_proto;
> +extern const struct bpf_func_proto bpf_get_socket_ptr_cookie_proto;
>
> const struct bpf_func_proto *bpf_tracing_func_proto(
> enum bpf_func_id func_id, const struct bpf_prog *prog);
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 0b735c2729b2..5855c398d685 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1673,6 +1673,14 @@ union bpf_attr {
> * Return
> * A 8-byte long unique number.
> *
> + * u64 bpf_get_socket_cookie(void *sk)
should the type be `struct sock *` then?
> + * Description
> + * Equivalent to **bpf_get_socket_cookie**\ () helper that
> accepts
> + * *sk*, but gets socket from a BTF **struct sock**. This helper
> + * also works for sleepable programs.
> + * Return
> + * A 8-byte long unique number or 0 if *sk* is NULL.
> + *
> * u32 bpf_get_socket_uid(struct sk_buff *skb)
> * Return
> * The owner UID of the socket associated to *skb*. If the socket
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 6c0018abe68a..845b2168e006 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -1760,6 +1760,8 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const
> struct bpf_prog *prog)
> return _sk_storage_delete_tracing_proto;
> case BPF_FUNC_sock_from_file:
> return _sock_from_file_proto;
> + case BPF_FUNC_get_socket_cookie:
> + return _get_socket_ptr_cookie_proto;
> #endif
> case BPF_FUNC_seq_printf:
> return prog->expected_attach_type == BPF_TRACE_ITER ?
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 9ab94e90d660..606e2b6115ed 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4631,6 +4631,18 @@ static const struct bpf_func_proto
> bpf_get_socket_cookie_sock_proto = {
> .arg1_type = ARG_PTR_TO_CTX,
> };
>
> +BPF_CALL_1(bpf_get_socket_ptr_cookie, struct sock *, sk)
> +{
> + return sk ? sock_gen_cookie(sk) : 0;
> +}
> +
> +const struct bpf_func_proto bpf_get_socket_ptr_cookie_proto = {
> + .func = bpf_get_socket_ptr_cookie,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_BTF_ID_SOCK_COMMON,
> +};
> +
> BPF_CALL_1(bpf_get_socket_cookie_sock_ops, struct bpf_sock_ops_kern *, ctx)
> {
> return __sock_gen_cookie(ctx->sk);
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 0b735c2729b2..5855c398d685 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -1673,6 +1673,14 @@ union bpf_attr {
> * Return
> * A 8-byte long unique number.
> *
> + * u64 bpf_get_socket_cookie(void *sk)
> + * Description
> + * Equivalent to **bpf_get_socket_cookie**\ () helper that
> accepts
> + * *sk*, but gets socket from a BTF **struct sock**. This helper
> + * also works for sleepable programs.
> + * Return
> + * A 8-byte long unique number or 0 if *sk* is NULL.
> + *
> * u32 bpf_get_socket_uid(struct sk_buff *skb)
> * Return
> * The owner UID of the socket associated to *skb*. If the socket
> --
> 2.30.0.280.ga3ce27912f-goog
>
[PATCH bpf-next v6 2/5] bpf: Expose bpf_get_socket_cookie to tracing programs
This needs a new helper that:
- can work in a sleepable context (using sock_gen_cookie)
- takes a struct sock pointer and checks that it's not NULL
Signed-off-by: Florent Revest
Acked-by: KP Singh
---
include/linux/bpf.h| 1 +
include/uapi/linux/bpf.h | 8
kernel/trace/bpf_trace.c | 2 ++
net/core/filter.c | 12
tools/include/uapi/linux/bpf.h | 8
5 files changed, 31 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 1aac2af12fed..26219465e1f7 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1874,6 +1874,7 @@ extern const struct bpf_func_proto bpf_per_cpu_ptr_proto;
extern const struct bpf_func_proto bpf_this_cpu_ptr_proto;
extern const struct bpf_func_proto bpf_ktime_get_coarse_ns_proto;
extern const struct bpf_func_proto bpf_sock_from_file_proto;
+extern const struct bpf_func_proto bpf_get_socket_ptr_cookie_proto;
const struct bpf_func_proto *bpf_tracing_func_proto(
enum bpf_func_id func_id, const struct bpf_prog *prog);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 0b735c2729b2..5855c398d685 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1673,6 +1673,14 @@ union bpf_attr {
* Return
* A 8-byte long unique number.
*
+ * u64 bpf_get_socket_cookie(void *sk)
+ * Description
+ * Equivalent to **bpf_get_socket_cookie**\ () helper that accepts
+ * *sk*, but gets socket from a BTF **struct sock**. This helper
+ * also works for sleepable programs.
+ * Return
+ * A 8-byte long unique number or 0 if *sk* is NULL.
+ *
* u32 bpf_get_socket_uid(struct sk_buff *skb)
* Return
* The owner UID of the socket associated to *skb*. If the socket
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 6c0018abe68a..845b2168e006 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1760,6 +1760,8 @@ tracing_prog_func_proto(enum bpf_func_id func_id, const
struct bpf_prog *prog)
return _sk_storage_delete_tracing_proto;
case BPF_FUNC_sock_from_file:
return _sock_from_file_proto;
+ case BPF_FUNC_get_socket_cookie:
+ return _get_socket_ptr_cookie_proto;
#endif
case BPF_FUNC_seq_printf:
return prog->expected_attach_type == BPF_TRACE_ITER ?
diff --git a/net/core/filter.c b/net/core/filter.c
index 9ab94e90d660..606e2b6115ed 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4631,6 +4631,18 @@ static const struct bpf_func_proto
bpf_get_socket_cookie_sock_proto = {
.arg1_type = ARG_PTR_TO_CTX,
};
+BPF_CALL_1(bpf_get_socket_ptr_cookie, struct sock *, sk)
+{
+ return sk ? sock_gen_cookie(sk) : 0;
+}
+
+const struct bpf_func_proto bpf_get_socket_ptr_cookie_proto = {
+ .func = bpf_get_socket_ptr_cookie,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_BTF_ID_SOCK_COMMON,
+};
+
BPF_CALL_1(bpf_get_socket_cookie_sock_ops, struct bpf_sock_ops_kern *, ctx)
{
return __sock_gen_cookie(ctx->sk);
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 0b735c2729b2..5855c398d685 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1673,6 +1673,14 @@ union bpf_attr {
* Return
* A 8-byte long unique number.
*
+ * u64 bpf_get_socket_cookie(void *sk)
+ * Description
+ * Equivalent to **bpf_get_socket_cookie**\ () helper that accepts
+ * *sk*, but gets socket from a BTF **struct sock**. This helper
+ * also works for sleepable programs.
+ * Return
+ * A 8-byte long unique number or 0 if *sk* is NULL.
+ *
* u32 bpf_get_socket_uid(struct sk_buff *skb)
* Return
* The owner UID of the socket associated to *skb*. If the socket
--
2.30.0.280.ga3ce27912f-goog
