Re: [PATCH v6 bpf-next 3/4] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling

[email protected]
Newsgroups gmane.linux.kernel.lsm,gmane.linux.kernel.bpf,gmane.linux.file-systems,gmane.linux.kernel
Message-ID <c832e29380545375b9898d4bf6066d0f989dc1065b4c155513d1321ab38db56f@mail.kernel.org>
> diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> index f1863a891db6..7b55073934e0 100644
> --- a/fs/bpf_fs_kfuncs.c
> +++ b/fs/bpf_fs_kfuncs.c
> @@ -11,7 +11,9 @@
>  #include <linux/file.h>
>  #include <linux/kernfs.h>
>  #include <linux/mm.h>
> +#include <linux/security.h>
>  #include <linux/xattr.h>
> +#include <uapi/linux/lsm.h>
>
>  __bpf_kfunc_start_defs();
>
> @@ -379,6 +381,44 @@ __bpf_kfunc struct inode *bpf_real_data_inode(struct file *file)
>  	return d_real_inode(file_dentry(file));
>  }
>
> +/**
> + * bpf_init_inode_xattr - set an xattr on a new inode from inode_init_security
> + * @xattrs: inode_init_security xattr state from the hook context
> + * @name__str: xattr name (e.g., "bpf.file_label")
> + * @value_p: dynptr containing the xattr value
> + *
> + * Only callable from lsm/inode_init_security programs.
> + *
> + * Return: 0 on success, -EOPNOTSUPP if the filesystem does not accept
> + *         xattrs at inode creation, negative error on other failures.
> + */
> +__bpf_kfunc int bpf_init_inode_xattr(struct lsm_xattrs *xattrs,
> +				     const char *name__str,
> +				     const struct bpf_dynptr *value_p)

Paul Moore raised a concern about the location of this kfunc. As the LSM
maintainer, he stated that this kfunc must be placed in
security/bpf_lsm_kfuncs.c to be acceptable.

In the v6 discussion, he explained: "The new LSM kfunc must go in
security/bpf_lsm_kfuncs.c; if that isn't acceptable to the BPF developers
then David will need to find a new way that doesn't involve kfuncs."

Kumar Kartikeya Dwivedi disagreed with this placement requirement, arguing
that it should remain in fs/ with other kfuncs. This represents an
unresolved maintainer disagreement.

Should this kfunc be moved to security/bpf_lsm_kfuncs.c as the LSM
maintainer requested, or does this need further discussion between the BPF
and LSM maintainers before merging?

Reference: https://lore.kernel.org/all/CAHC9VhTS7rSnBqg00ZxNkcZyh_=EeJmn_4z3CTCCxreEEDtTtg@mail.gmail.com/

> +{
> +	struct bpf_dynptr_kern *value_ptr = (struct bpf_dynptr_kern *)value_p;
> +	const void *value;
> +	u32 value_len;
> +
> +	if (!name__str)
> +		return -EINVAL;
> +	if (strncmp(name__str, XATTR_BPF_LSM_SUFFIX,
> +		    sizeof(XATTR_BPF_LSM_SUFFIX) - 1))
> +		return -EPERM;
> +
> +	if (!xattrs->xattrs)
> +		return -EOPNOTSUPP;
> +
> +	value_len = __bpf_dynptr_size(value_ptr);
> +	value = __bpf_dynptr_data(value_ptr, value_len);
> +	if (!value)
> +		return -EINVAL;
> +
> +	return security_lsmxattr_add(xattrs, LSM_ID_BPF,
> +				     name__str + sizeof(XATTR_BPF_LSM_SUFFIX) - 1,
> +				     value, value_len);
> +}
> +
>  __bpf_kfunc_end_defs();
>
>  BTF_KFUNCS_START(bpf_fs_kfunc_set_ids)
> @@ -390,6 +430,7 @@ BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE)
>  BTF_ID_FLAGS(func, bpf_set_dentry_xattr, KF_SLEEPABLE)
>  BTF_ID_FLAGS(func, bpf_remove_dentry_xattr, KF_SLEEPABLE)
>  BTF_ID_FLAGS(func, bpf_real_data_inode, KF_SLEEPABLE | KF_RET_NULL)
> +BTF_ID_FLAGS(func, bpf_init_inode_xattr)
>  BTF_KFUNCS_END(bpf_fs_kfunc_set_ids)


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/30592505811
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.