Re: [PATCH bpf-next v8 6/7] bpf: Allow local storage to be used from LSM programs

[Martin KaFai Lau]

On Mon, Aug 03, 2020 at 06:46:54PM +0200, KP Singh wrote:
> From: KP Singh <kpsingh@xxxxxxxxxx>
> 
> Adds support for both bpf_{sk, inode}_storage_{get, delete} to be used
> in LSM programs. These helpers are not used for tracing programs
> (currently) as their usage is tied to the life-cycle of the object and
> should only be used where the owning object won't be freed (when the
> owning object is passed as an argument to the LSM hook). Thus, they
> are safer to use in LSM hooks than tracing. Usage of local storage in
> tracing programs will probably follow a per function based whitelist
> approach.
> 
> Since the UAPI helper signature for bpf_sk_storage expect a bpf_sock,
> it, leads to a compilation warning for LSM programs, it's also updated
> to accept a void * pointer instead.
> 
> Signed-off-by: KP Singh <kpsingh@xxxxxxxxxx>
> ---
>  include/net/bpf_sk_storage.h   |  2 ++
>  include/uapi/linux/bpf.h       |  8 ++++++--
>  kernel/bpf/bpf_lsm.c           | 21 ++++++++++++++++++++-
>  net/core/bpf_sk_storage.c      | 25 +++++++++++++++++++++++++
>  tools/include/uapi/linux/bpf.h |  8 ++++++--
>  5 files changed, 59 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/bpf_sk_storage.h b/include/net/bpf_sk_storage.h
> index 847926cf2899..c5702d7baeaa 100644
> --- a/include/net/bpf_sk_storage.h
> +++ b/include/net/bpf_sk_storage.h
> @@ -20,6 +20,8 @@ void bpf_sk_storage_free(struct sock *sk);
>  
>  extern const struct bpf_func_proto bpf_sk_storage_get_proto;
>  extern const struct bpf_func_proto bpf_sk_storage_delete_proto;
> +extern const struct bpf_func_proto sk_storage_get_btf_proto;
> +extern const struct bpf_func_proto sk_storage_delete_btf_proto;
>  
>  struct bpf_sk_storage_diag;
>  struct sk_buff;
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index e17c00eea5d8..6ffc61dafc5c 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2807,7 +2807,7 @@ union bpf_attr {
>   *
>   *		**-ERANGE** if resulting value was out of range.
>   *
> - * void *bpf_sk_storage_get(struct bpf_map *map, struct bpf_sock *sk, void *value, u64 flags)
> + * void *bpf_sk_storage_get(struct bpf_map *map, void *sk, void *value, u64 flags)
>   *	Description
>   *		Get a bpf-local-storage from a *sk*.
>   *
> @@ -2823,6 +2823,10 @@ union bpf_attr {
>   *		"type". The bpf-local-storage "type" (i.e. the *map*) is
>   *		searched against all bpf-local-storages residing at *sk*.
>   *
> + *		For socket programs, *sk* should be a **struct bpf_sock** pointer
> + *		and an **ARG_PTR_TO_BTF_ID** of type **struct sock** for LSM
> + *		programs.
I found it a little vague on what "socket programs" is.  May be:

*sk* is a kernel **struct sock** pointer for LSM program.
*sk* is a **struct bpf_sock** pointer for other program types.

Others LGTM

Acked-by: Martin KaFai Lau <kafai@xxxxxx>