On Mon 16-12-24 22:38:16, Song Liu wrote:
> Introduct new xattr name prefix security.bpf., and enable reading these
> xattrs from bpf kfuncs bpf_get_[file|dentry]_xattr().
>
> As we are on it, correct the comments for return value of
> bpf_get_[file|dentry]_xattr(), i.e. return length the xattr value on
> success.
>
> Signed-off-by: Song Liu <song@xxxxxxxxxx>
> Acked-by: Christian Brauner <brauner@xxxxxxxxxx>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> fs/bpf_fs_kfuncs.c | 19 ++++++++++++++-----
> include/uapi/linux/xattr.h | 4 ++++
> 2 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> index 3fe9f59ef867..8a65184c8c2c 100644
> --- a/fs/bpf_fs_kfuncs.c
> +++ b/fs/bpf_fs_kfuncs.c
> @@ -93,6 +93,11 @@ __bpf_kfunc int bpf_path_d_path(struct path *path, char *buf, size_t buf__sz)
> return len;
> }
>
> +static bool match_security_bpf_prefix(const char *name__str)
> +{
> + return !strncmp(name__str, XATTR_NAME_BPF_LSM, XATTR_NAME_BPF_LSM_LEN);
> +}
> +
> /**
> * bpf_get_dentry_xattr - get xattr of a dentry
> * @dentry: dentry to get xattr from
> @@ -101,9 +106,10 @@ __bpf_kfunc int bpf_path_d_path(struct path *path, char *buf, size_t buf__sz)
> *
> * Get xattr *name__str* of *dentry* and store the output in *value_ptr*.
> *
> - * For security reasons, only *name__str* with prefix "user." is allowed.
> + * For security reasons, only *name__str* with prefix "user." or
> + * "security.bpf." is allowed.
> *
> - * Return: 0 on success, a negative value on error.
> + * Return: length of the xattr value on success, a negative value on error.
> */
> __bpf_kfunc int bpf_get_dentry_xattr(struct dentry *dentry, const char *name__str,
> struct bpf_dynptr *value_p)
> @@ -117,7 +123,9 @@ __bpf_kfunc int bpf_get_dentry_xattr(struct dentry *dentry, const char *name__st
> if (WARN_ON(!inode))
> return -EINVAL;
>
> - if (strncmp(name__str, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN))
> + /* Allow reading xattr with user. and security.bpf. prefix */
> + if (strncmp(name__str, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN) &&
> + !match_security_bpf_prefix(name__str))
> return -EPERM;
>
> value_len = __bpf_dynptr_size(value_ptr);
> @@ -139,9 +147,10 @@ __bpf_kfunc int bpf_get_dentry_xattr(struct dentry *dentry, const char *name__st
> *
> * Get xattr *name__str* of *file* and store the output in *value_ptr*.
> *
> - * For security reasons, only *name__str* with prefix "user." is allowed.
> + * For security reasons, only *name__str* with prefix "user." or
> + * "security.bpf." is allowed.
> *
> - * Return: 0 on success, a negative value on error.
> + * Return: length of the xattr value on success, a negative value on error.
> */
> __bpf_kfunc int bpf_get_file_xattr(struct file *file, const char *name__str,
> struct bpf_dynptr *value_p)
> diff --git a/include/uapi/linux/xattr.h b/include/uapi/linux/xattr.h
> index 9854f9cff3c6..c7c85bb504ba 100644
> --- a/include/uapi/linux/xattr.h
> +++ b/include/uapi/linux/xattr.h
> @@ -83,6 +83,10 @@ struct xattr_args {
> #define XATTR_CAPS_SUFFIX "capability"
> #define XATTR_NAME_CAPS XATTR_SECURITY_PREFIX XATTR_CAPS_SUFFIX
>
> +#define XATTR_BPF_LSM_SUFFIX "bpf."
> +#define XATTR_NAME_BPF_LSM (XATTR_SECURITY_PREFIX XATTR_BPF_LSM_SUFFIX)
> +#define XATTR_NAME_BPF_LSM_LEN (sizeof(XATTR_NAME_BPF_LSM) - 1)
> +
> #define XATTR_POSIX_ACL_ACCESS "posix_acl_access"
> #define XATTR_NAME_POSIX_ACL_ACCESS XATTR_SYSTEM_PREFIX XATTR_POSIX_ACL_ACCESS
> #define XATTR_POSIX_ACL_DEFAULT "posix_acl_default"
> --
> 2.43.5
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
