Skip to content

Commit

Permalink
Merge branch 'enable-writing-xattr-from-bpf-programs'
Browse files Browse the repository at this point in the history
Song Liu says:

====================
Enable writing xattr from BPF programs

Add support to set and remove xattr from BPF program. Also add
security.bpf. xattr name prefix.

kfuncs are added to set and remove xattrs with security.bpf. name
prefix. Update kfuncs bpf_get_[file|dentry]_xattr to read xattrs
with security.bpf. name prefix. Note that BPF programs can read
user. xattrs, but not write and remove them.

To pick the right version of kfunc to use, a remap logic is added to
btf_kfunc_id_set. This helps move some kfunc specific logic off the
verifier core code. Also use this remap logic to select
bpf_dynptr_from_skb or bpf_dynptr_from_skb_rdonly.

Cover letter of v1 and v2:

Follow up discussion in LPC 2024 [1], that we need security.bpf xattr
prefix. This set adds "security.bpf." xattr name prefix, and allows
bpf kfuncs bpf_get_[file|dentry]_xattr() to read these xattrs.

[1] https://lpc.events/event/18/contributions/1940/
---

Changes v11 => v12:
1. Drop btf_kfunc_id_set.remap and changes for bpf_dynptr_from_skb.
   (Alexei)
2. Minor refactoring in patch 1. (Matt Bobrowski)

v11: https://lore.kernel.org/bpf/20250129205957.2457655-1-song@kernel.org/

Changes v10 => v11:

1. Add Acked-by from Christian Brauner.
2. Fix selftests build error like this one:
   https://github.com/kernel-patches/bpf/actions/runs/13022268618/job/36325472992
3. Rename some variables in the selftests.

v10: https://lore.kernel.org/bpf/20250124202911.3264715-1-song@kernel.org/

Changes v9 => v10:
1. Refactor bpf_[set|remove]_dentry_xattr[_locked]. (Christian Brauner).

v9: https://lore.kernel.org/bpf/20250110011342.2965136-1-song@kernel.org/

Changes v8 => v9
1. Fix build for CONFIG_DEBUG_INFO_BTF=n case. (kernel test robot)

v8: https://lore.kernel.org/bpf/20250108225140.3467654-1-song@kernel.org/

Changes v7 => v8
1. Rebase and resolve conflicts.

v7: https://lore.kernel.org/bpf/20241219221439.2455664-1-song@kernel.org/

Changes v6 => v7
1. Move btf_kfunc_id_remap() to the right place. (Bug reported by CI)

v6: https://lore.kernel.org/bpf/20241219202536.1625216-1-song@kernel.org/

Changes v5 => v6
1. Hide _locked version of the kfuncs from vmlinux.h (Alexei)
2. Add remap logic to btf_kfunc_id_set and use that to pick the correct
   version of kfuncs to use.
3. Also use the remap logic for bpf_dynptr_from_skb[|_rdonly].

v5: https://lore.kernel.org/bpf/20241218044711.1723221-1-song@kernel.org/

Changes v4 => v5
1. Let verifier pick proper kfunc (_locked or not _locked)  based on the
   calling context. (Alexei)
2. Remove the __failure test (6/6 of v4).

v4: https://lore.kernel.org/bpf/20241217063821.482857-1-song@kernel.org/

Changes v3 => v4
1. Do write permission check with inode locked. (Jan Kara)
2. Fix some source_inline warnings.

v3: https://lore.kernel.org/bpf/20241210220627.2800362-1-song@kernel.org/

Changes v2 => v3
1. Add kfuncs to set and remove xattr from BPF programs.

v2: https://lore.kernel.org/bpf/20241016070955.375923-1-song@kernel.org/

Changes v1 => v2
1. Update comment of bpf_get_[file|dentry]_xattr. (Jiri Olsa)
2. Fix comment for return value of bpf_get_[file|dentry]_xattr.

v1: https://lore.kernel.org/bpf/20241002214637.3625277-1-song@kernel.org/
====================

Link: https://patch.msgid.link/20250130213549.3353349-1-song@kernel.org
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
Alexei Starovoitov committed Feb 14, 2025
2 parents b99f27e + 60c2e1f commit 68a4154
Show file tree
Hide file tree
Showing 9 changed files with 573 additions and 25 deletions.
225 changes: 214 additions & 11 deletions fs/bpf_fs_kfuncs.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
/* Copyright (c) 2024 Google LLC. */

#include <linux/bpf.h>
#include <linux/bpf_lsm.h>
#include <linux/btf.h>
#include <linux/btf_ids.h>
#include <linux/dcache.h>
#include <linux/fs.h>
#include <linux/fsnotify.h>
#include <linux/file.h>
#include <linux/mm.h>
#include <linux/xattr.h>
Expand Down Expand Up @@ -93,6 +95,24 @@ __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);
}

static int bpf_xattr_read_permission(const char *name, struct inode *inode)
{
if (WARN_ON(!inode))
return -EINVAL;

/* Allow reading xattr with user. and security.bpf. prefix */
if (strncmp(name, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN) &&
!match_security_bpf_prefix(name))
return -EPERM;

return inode_permission(&nop_mnt_idmap, inode, MAY_READ);
}

/**
* bpf_get_dentry_xattr - get xattr of a dentry
* @dentry: dentry to get xattr from
Expand All @@ -101,9 +121,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 prefixes "user." or
* "security.bpf." are 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)
Expand All @@ -114,18 +135,12 @@ __bpf_kfunc int bpf_get_dentry_xattr(struct dentry *dentry, const char *name__st
void *value;
int ret;

if (WARN_ON(!inode))
return -EINVAL;

if (strncmp(name__str, XATTR_USER_PREFIX, XATTR_USER_PREFIX_LEN))
return -EPERM;

value_len = __bpf_dynptr_size(value_ptr);
value = __bpf_dynptr_data_rw(value_ptr, value_len);
if (!value)
return -EINVAL;

ret = inode_permission(&nop_mnt_idmap, inode, MAY_READ);
ret = bpf_xattr_read_permission(name__str, inode);
if (ret)
return ret;
return __vfs_getxattr(dentry, inode, name__str, value, value_len);
Expand All @@ -139,9 +154,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 prefixes "user." or
* "security.bpf." are 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)
Expand All @@ -154,13 +170,169 @@ __bpf_kfunc int bpf_get_file_xattr(struct file *file, const char *name__str,

__bpf_kfunc_end_defs();

static int bpf_xattr_write_permission(const char *name, struct inode *inode)
{
if (WARN_ON(!inode))
return -EINVAL;

/* Only allow setting and removing security.bpf. xattrs */
if (!match_security_bpf_prefix(name))
return -EPERM;

return inode_permission(&nop_mnt_idmap, inode, MAY_WRITE);
}

/**
* bpf_set_dentry_xattr_locked - set a xattr of a dentry
* @dentry: dentry to get xattr from
* @name__str: name of the xattr
* @value_p: xattr value
* @flags: flags to pass into filesystem operations
*
* Set xattr *name__str* of *dentry* to the value in *value_ptr*.
*
* For security reasons, only *name__str* with prefix "security.bpf."
* is allowed.
*
* The caller already locked dentry->d_inode.
*
* Return: 0 on success, a negative value on error.
*/
int bpf_set_dentry_xattr_locked(struct dentry *dentry, const char *name__str,
const struct bpf_dynptr *value_p, int flags)
{

struct bpf_dynptr_kern *value_ptr = (struct bpf_dynptr_kern *)value_p;
struct inode *inode = d_inode(dentry);
const void *value;
u32 value_len;
int ret;

value_len = __bpf_dynptr_size(value_ptr);
value = __bpf_dynptr_data(value_ptr, value_len);
if (!value)
return -EINVAL;

ret = bpf_xattr_write_permission(name__str, inode);
if (ret)
return ret;

ret = __vfs_setxattr(&nop_mnt_idmap, dentry, inode, name__str,
value, value_len, flags);
if (!ret) {
fsnotify_xattr(dentry);

/* This xattr is set by BPF LSM, so we do not call
* security_inode_post_setxattr. Otherwise, we would
* risk deadlocks by calling back to the same kfunc.
*
* This is the same as security_inode_setsecurity().
*/
}
return ret;
}

/**
* bpf_remove_dentry_xattr_locked - remove a xattr of a dentry
* @dentry: dentry to get xattr from
* @name__str: name of the xattr
*
* Rmove xattr *name__str* of *dentry*.
*
* For security reasons, only *name__str* with prefix "security.bpf."
* is allowed.
*
* The caller already locked dentry->d_inode.
*
* Return: 0 on success, a negative value on error.
*/
int bpf_remove_dentry_xattr_locked(struct dentry *dentry, const char *name__str)
{
struct inode *inode = d_inode(dentry);
int ret;

ret = bpf_xattr_write_permission(name__str, inode);
if (ret)
return ret;

ret = __vfs_removexattr(&nop_mnt_idmap, dentry, name__str);
if (!ret) {
fsnotify_xattr(dentry);

/* This xattr is removed by BPF LSM, so we do not call
* security_inode_post_removexattr. Otherwise, we would
* risk deadlocks by calling back to the same kfunc.
*/
}
return ret;
}

__bpf_kfunc_start_defs();

/**
* bpf_set_dentry_xattr - set a xattr of a dentry
* @dentry: dentry to get xattr from
* @name__str: name of the xattr
* @value_p: xattr value
* @flags: flags to pass into filesystem operations
*
* Set xattr *name__str* of *dentry* to the value in *value_ptr*.
*
* For security reasons, only *name__str* with prefix "security.bpf."
* is allowed.
*
* The caller has not locked dentry->d_inode.
*
* Return: 0 on success, a negative value on error.
*/
__bpf_kfunc int bpf_set_dentry_xattr(struct dentry *dentry, const char *name__str,
const struct bpf_dynptr *value_p, int flags)
{
struct inode *inode = d_inode(dentry);
int ret;

inode_lock(inode);
ret = bpf_set_dentry_xattr_locked(dentry, name__str, value_p, flags);
inode_unlock(inode);
return ret;
}

/**
* bpf_remove_dentry_xattr - remove a xattr of a dentry
* @dentry: dentry to get xattr from
* @name__str: name of the xattr
*
* Rmove xattr *name__str* of *dentry*.
*
* For security reasons, only *name__str* with prefix "security.bpf."
* is allowed.
*
* The caller has not locked dentry->d_inode.
*
* Return: 0 on success, a negative value on error.
*/
__bpf_kfunc int bpf_remove_dentry_xattr(struct dentry *dentry, const char *name__str)
{
struct inode *inode = d_inode(dentry);
int ret;

inode_lock(inode);
ret = bpf_remove_dentry_xattr_locked(dentry, name__str);
inode_unlock(inode);
return ret;
}

__bpf_kfunc_end_defs();

BTF_KFUNCS_START(bpf_fs_kfunc_set_ids)
BTF_ID_FLAGS(func, bpf_get_task_exe_file,
KF_ACQUIRE | KF_TRUSTED_ARGS | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_put_file, KF_RELEASE)
BTF_ID_FLAGS(func, bpf_path_d_path, KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_get_dentry_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_set_dentry_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
BTF_ID_FLAGS(func, bpf_remove_dentry_xattr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
BTF_KFUNCS_END(bpf_fs_kfunc_set_ids)

static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id)
Expand All @@ -171,6 +343,37 @@ static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id)
return -EACCES;
}

/* bpf_[set|remove]_dentry_xattr.* hooks have KF_TRUSTED_ARGS and
* KF_SLEEPABLE, so they are only available to sleepable hooks with
* dentry arguments.
*
* Setting and removing xattr requires exclusive lock on dentry->d_inode.
* Some hooks already locked d_inode, while some hooks have not locked
* d_inode. Therefore, we need different kfuncs for different hooks.
* Specifically, hooks in the following list (d_inode_locked_hooks)
* should call bpf_[set|remove]_dentry_xattr_locked; while other hooks
* should call bpf_[set|remove]_dentry_xattr.
*/
BTF_SET_START(d_inode_locked_hooks)
BTF_ID(func, bpf_lsm_inode_post_removexattr)
BTF_ID(func, bpf_lsm_inode_post_setattr)
BTF_ID(func, bpf_lsm_inode_post_setxattr)
BTF_ID(func, bpf_lsm_inode_removexattr)
BTF_ID(func, bpf_lsm_inode_rmdir)
BTF_ID(func, bpf_lsm_inode_setattr)
BTF_ID(func, bpf_lsm_inode_setxattr)
BTF_ID(func, bpf_lsm_inode_unlink)
#ifdef CONFIG_SECURITY_PATH
BTF_ID(func, bpf_lsm_path_unlink)
BTF_ID(func, bpf_lsm_path_rmdir)
#endif /* CONFIG_SECURITY_PATH */
BTF_SET_END(d_inode_locked_hooks)

bool bpf_lsm_has_d_inode_locked(const struct bpf_prog *prog)
{
return btf_id_set_contains(&d_inode_locked_hooks, prog->aux->attach_btf_id);
}

static const struct btf_kfunc_id_set bpf_fs_kfunc_set = {
.owner = THIS_MODULE,
.set = &bpf_fs_kfunc_set_ids,
Expand Down
18 changes: 18 additions & 0 deletions include/linux/bpf_lsm.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog, bpf_func_t *bpf_func)

int bpf_lsm_get_retval_range(const struct bpf_prog *prog,
struct bpf_retval_range *range);
int bpf_set_dentry_xattr_locked(struct dentry *dentry, const char *name__str,
const struct bpf_dynptr *value_p, int flags);
int bpf_remove_dentry_xattr_locked(struct dentry *dentry, const char *name__str);
bool bpf_lsm_has_d_inode_locked(const struct bpf_prog *prog);

#else /* !CONFIG_BPF_LSM */

static inline bool bpf_lsm_is_sleepable_hook(u32 btf_id)
Expand Down Expand Up @@ -86,6 +91,19 @@ static inline int bpf_lsm_get_retval_range(const struct bpf_prog *prog,
{
return -EOPNOTSUPP;
}
static inline int bpf_set_dentry_xattr_locked(struct dentry *dentry, const char *name__str,
const struct bpf_dynptr *value_p, int flags)
{
return -EOPNOTSUPP;
}
static inline int bpf_remove_dentry_xattr_locked(struct dentry *dentry, const char *name__str)
{
return -EOPNOTSUPP;
}
static inline bool bpf_lsm_has_d_inode_locked(const struct bpf_prog *prog)
{
return false;
}
#endif /* CONFIG_BPF_LSM */

#endif /* _LINUX_BPF_LSM_H */
4 changes: 4 additions & 0 deletions include/uapi/linux/xattr.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 2 additions & 0 deletions kernel/bpf/bpf_lsm.c
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,9 @@ BTF_ID(func, bpf_lsm_inode_getxattr)
BTF_ID(func, bpf_lsm_inode_mknod)
BTF_ID(func, bpf_lsm_inode_need_killpriv)
BTF_ID(func, bpf_lsm_inode_post_setxattr)
BTF_ID(func, bpf_lsm_inode_post_removexattr)
BTF_ID(func, bpf_lsm_inode_readlink)
BTF_ID(func, bpf_lsm_inode_removexattr)
BTF_ID(func, bpf_lsm_inode_rename)
BTF_ID(func, bpf_lsm_inode_rmdir)
BTF_ID(func, bpf_lsm_inode_setattr)
Expand Down
21 changes: 21 additions & 0 deletions kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -11766,6 +11766,8 @@ enum special_kfunc_type {
KF_bpf_iter_num_new,
KF_bpf_iter_num_next,
KF_bpf_iter_num_destroy,
KF_bpf_set_dentry_xattr,
KF_bpf_remove_dentry_xattr,
};

BTF_SET_START(special_kfunc_set)
Expand Down Expand Up @@ -11795,6 +11797,10 @@ BTF_ID(func, bpf_wq_set_callback_impl)
#ifdef CONFIG_CGROUPS
BTF_ID(func, bpf_iter_css_task_new)
#endif
#ifdef CONFIG_BPF_LSM
BTF_ID(func, bpf_set_dentry_xattr)
BTF_ID(func, bpf_remove_dentry_xattr)
#endif
BTF_SET_END(special_kfunc_set)

BTF_ID_LIST(special_kfunc_list)
Expand Down Expand Up @@ -11844,6 +11850,13 @@ BTF_ID(func, bpf_local_irq_restore)
BTF_ID(func, bpf_iter_num_new)
BTF_ID(func, bpf_iter_num_next)
BTF_ID(func, bpf_iter_num_destroy)
#ifdef CONFIG_BPF_LSM
BTF_ID(func, bpf_set_dentry_xattr)
BTF_ID(func, bpf_remove_dentry_xattr)
#else
BTF_ID_UNUSED
BTF_ID_UNUSED
#endif

static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
{
Expand Down Expand Up @@ -20924,6 +20937,14 @@ static void specialize_kfunc(struct bpf_verifier_env *env,
*/
env->seen_direct_write = seen_direct_write;
}

if (func_id == special_kfunc_list[KF_bpf_set_dentry_xattr] &&
bpf_lsm_has_d_inode_locked(prog))
*addr = (unsigned long)bpf_set_dentry_xattr_locked;

if (func_id == special_kfunc_list[KF_bpf_remove_dentry_xattr] &&
bpf_lsm_has_d_inode_locked(prog))
*addr = (unsigned long)bpf_remove_dentry_xattr_locked;
}

static void __fixup_collection_insert_kfunc(struct bpf_insn_aux_data *insn_aux,
Expand Down
Loading

0 comments on commit 68a4154

Please sign in to comment.