Re: [PATCH 1/4] bpf: Add pin_name into struct bpf_prog_aux

[Daniel Borkmann <daniel@xxxxxxxxxxxxx>]

On 2/11/22 1:11 PM, Yafang Shao wrote:
> A new member pin_name is added into struct bpf_prog_aux, which will be
> set when the prog is set and cleared when the pinned file is removed.
>
> Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
> ---
>   include/linux/bpf.h      |  2 ++
>   include/uapi/linux/bpf.h |  1 +
>   kernel/bpf/inode.c       | 20 +++++++++++++++++++-
>   3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 0ceb25b..9cf8055 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -933,6 +933,8 @@ struct bpf_prog_aux {
>   		struct work_struct work;
>   		struct rcu_head	rcu;
>   	};
> +
> +	char pin_name[BPF_PIN_NAME_LEN];
>   };

I'm afraid this is not possible. You are assuming a 1:1 relationship between prog
and pin location, but it's really a 1:n (prog can be pinned in multiple locations
and also across multiple mount instances). Also, you can create hard links of pins
which are not handled via bpf_obj_do_pin().

>   struct bpf_array_aux {
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index c14fed8..bada5cc 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1217,6 +1217,7 @@ struct bpf_stack_build_id {
>   };
>   
>   #define BPF_OBJ_NAME_LEN 16U
> +#define BPF_PIN_NAME_LEN 64U
>   
>   union bpf_attr {
>   	struct { /* anonymous struct used by BPF_MAP_CREATE command */
> diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
> index 4477ca8..f1a8811 100644
> --- a/kernel/bpf/inode.c
> +++ b/kernel/bpf/inode.c
> @@ -437,6 +437,8 @@ static int bpf_iter_link_pin_kernel(struct dentry *parent,
>   static int bpf_obj_do_pin(const char __user *pathname, void *raw,
>   			  enum bpf_type type)
>   {
> +	struct bpf_prog_aux *aux;
> +	struct bpf_prog *prog;
>   	struct dentry *dentry;
>   	struct inode *dir;
>   	struct path path;
> @@ -461,6 +463,10 @@ static int bpf_obj_do_pin(const char __user *pathname, void *raw,
>   
>   	switch (type) {
>   	case BPF_TYPE_PROG:
> +		prog = raw;
> +		aux = prog->aux;
> +		(void) strncpy_from_user(aux->pin_name, pathname, BPF_PIN_NAME_LEN);
> +		aux->pin_name[BPF_PIN_NAME_LEN - 1] = '\0';
>   		ret = vfs_mkobj(dentry, mode, bpf_mkprog, raw);
>   		break;
>   	case BPF_TYPE_MAP:
> @@ -611,12 +617,24 @@ static int bpf_show_options(struct seq_file *m, struct dentry *root)
>   
>   static void bpf_free_inode(struct inode *inode)
>   {
> +	struct bpf_prog_aux *aux;
> +	struct bpf_prog *prog;
>   	enum bpf_type type;
>   
>   	if (S_ISLNK(inode->i_mode))
>   		kfree(inode->i_link);
> -	if (!bpf_inode_type(inode, &type))
> +	if (!bpf_inode_type(inode, &type)) {
> +		switch (type) {
> +		case BPF_TYPE_PROG:
> +			prog = inode->i_private;
> +			aux = prog->aux;
> +			aux->pin_name[0] = '\0';
> +			break;
> +		default:
> +			break;
> +		}
>   		bpf_any_put(inode->i_private, type);
> +	}
>   	free_inode_nonrcu(inode);
>   }
>