[PATCH bpf-next 1/2] bpf: enforce VFS constraints for xattr related BPF kfuncs

[Matt Bobrowski <mattbobrowski@google.com>]

Enforce VFS constraints and semantics regarding name and value lengths
within the xattr related BPF kfuncs. Specifically, reject names that
are empty or longer than XATTR_NAME_MAX, and values larger than
XATTR_SIZE_MAX. Also validate the supplied flags to ensure that only
XATTR_CREATE and XATTR_REPLACE can be used alongside the default flag
value 0.

Fixes: 56467292794b ("bpf: fs/xattr: Add BPF kfuncs to set and remove xattrs")
Closes: https://lore.kernel.org/bpf/20260429221005.6D1C6C19425@smtp.kernel.org/
Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
---
 fs/bpf_fs_kfuncs.c | 80 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 65 insertions(+), 15 deletions(-)

diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
index 9d27be058494..a48cf35dedd2 100644
--- a/fs/bpf_fs_kfuncs.c
+++ b/fs/bpf_fs_kfuncs.c
@@ -93,6 +93,21 @@ __bpf_kfunc int bpf_path_d_path(const struct path *path, char *buf, size_t buf__
 	return len;
 }
 
+static int bpf_xattr_validate_name(const char *name)
+{
+	u32 name_len;
+
+	/*
+	 * Impose the same restrictions on the supplied name as done so within
+	 * the VFS by helpers like import_xattr_name().
+	 */
+	name_len = strlen(name);
+	if (!name_len || name_len > XATTR_NAME_MAX)
+		return -ERANGE;
+
+	return 0;
+}
+
 static bool match_security_bpf_prefix(const char *name__str)
 {
 	return !strncmp(name__str, XATTR_NAME_BPF_LSM, XATTR_NAME_BPF_LSM_LEN);
@@ -117,10 +132,10 @@ static int bpf_xattr_read_permission(const char *name, struct inode *inode)
  * @name__str: name of the xattr
  * @value_p: output buffer of the xattr value
  *
- * Get xattr *name__str* of *dentry* and store the output in *value_ptr*.
+ * Get xattr *name__str* of *dentry* and store the output in *value_p*.
  *
- * For security reasons, only *name__str* with prefixes "user." or
- * "security.bpf." are allowed.
+ * For security reasons, only *name__str* values prefixed with "user." or
+ * "security.bpf." are permitted.
  *
  * Return: length of the xattr value on success, a negative value on error.
  */
@@ -133,6 +148,10 @@ __bpf_kfunc int bpf_get_dentry_xattr(struct dentry *dentry, const char *name__st
 	void *value;
 	int ret;
 
+	ret = bpf_xattr_validate_name(name__str);
+	if (ret)
+		return ret;
+
 	value_len = __bpf_dynptr_size(value_ptr);
 	value = __bpf_dynptr_data_rw(value_ptr, value_len);
 	if (!value)
@@ -150,10 +169,10 @@ __bpf_kfunc int bpf_get_dentry_xattr(struct dentry *dentry, const char *name__st
  * @name__str: name of the xattr
  * @value_p: output buffer of the xattr value
  *
- * Get xattr *name__str* of *file* and store the output in *value_ptr*.
+ * Get xattr *name__str* of *file* and store the output in *value_p*.
  *
- * For security reasons, only *name__str* with prefixes "user." or
- * "security.bpf." are allowed.
+ * For security reasons, only *name__str* values prefixed with "user." or
+ * "security.bpf." are permitted.
  *
  * Return: length of the xattr value on success, a negative value on error.
  */
@@ -187,10 +206,18 @@ static int bpf_xattr_write_permission(const char *name, struct inode *inode)
  * @value_p: xattr value
  * @flags: flags to pass into filesystem operations
  *
- * Set xattr *name__str* of *dentry* to the value in *value_ptr*.
+ * Set xattr *name__str* of *dentry* to the value in *value_p*.
  *
- * For security reasons, only *name__str* with prefix "security.bpf."
- * is allowed.
+ * For security reasons, only *name__str* values prefixed with "security.bpf."
+ * are permitted.
+ *
+ * The length constraints imposed on both the xattr name and value abide those
+ * that are also enforced by the VFS. Additionally, the flags argument respects
+ * what's enforced by the VFS in the same way. By default, the flag value of 0
+ * is permitted and an xattr will be created if it does not exist, or the value
+ * will be replaced if the xattr already exists. More course grained control
+ * over these exact semantics is permitted by explicitly specifying either
+ * XATTR_CREATE or XATTR_REPLACE.
  *
  * The caller already locked dentry->d_inode.
  *
@@ -206,7 +233,17 @@ int bpf_set_dentry_xattr_locked(struct dentry *dentry, const char *name__str,
 	u32 value_len;
 	int ret;
 
+	if (flags & ~(XATTR_CREATE | XATTR_REPLACE))
+		return -EINVAL;
+
+	ret = bpf_xattr_validate_name(name__str);
+	if (ret)
+		return ret;
+
 	value_len = __bpf_dynptr_size(value_ptr);
+	if (value_len > XATTR_SIZE_MAX)
+		return -E2BIG;
+
 	value = __bpf_dynptr_data(value_ptr, value_len);
 	if (!value)
 		return -EINVAL;
@@ -237,8 +274,8 @@ int bpf_set_dentry_xattr_locked(struct dentry *dentry, const char *name__str,
  *
  * Rmove xattr *name__str* of *dentry*.
  *
- * For security reasons, only *name__str* with prefix "security.bpf."
- * is allowed.
+ * For security reasons, only *name__str* values prefixed with "security.bpf."
+ * are permitted.
  *
  * The caller already locked dentry->d_inode.
  *
@@ -249,6 +286,10 @@ int bpf_remove_dentry_xattr_locked(struct dentry *dentry, const char *name__str)
 	struct inode *inode = d_inode(dentry);
 	int ret;
 
+	ret = bpf_xattr_validate_name(name__str);
+	if (ret)
+		return ret;
+
 	ret = bpf_xattr_write_permission(name__str, inode);
 	if (ret)
 		return ret;
@@ -274,11 +315,19 @@ __bpf_kfunc_start_defs();
  * @value_p: xattr value
  * @flags: flags to pass into filesystem operations
  *
- * Set xattr *name__str* of *dentry* to the value in *value_ptr*.
+ * Set xattr *name__str* of *dentry* to the value in *value_p*.
  *
  * For security reasons, only *name__str* with prefix "security.bpf."
  * is allowed.
  *
+ * The length constraints imposed on both the xattr name and value abide those
+ * that are also enforced by the VFS. Additionally, the flags argument respects
+ * what's enforced by the VFS in the same way. By default, the flag value of 0
+ * is permitted and an xattr will be created if it does not exist, or the value
+ * will be replaced if the xattr already exists. More course grained control
+ * over these exact semantics is permitted by explicitly specifying either
+ * XATTR_CREATE or XATTR_REPLACE.
+ *
  * The caller has not locked dentry->d_inode.
  *
  * Return: 0 on success, a negative value on error.
@@ -327,14 +376,15 @@ __bpf_kfunc int bpf_remove_dentry_xattr(struct dentry *dentry, const char *name_
  * @name__str: name of the xattr
  * @value_p: output buffer of the xattr value
  *
- * Get xattr *name__str* of *cgroup* and store the output in *value_ptr*.
+ * Get xattr *name__str* of *cgroup* and store the output in *value_p*.
  *
- * For security reasons, only *name__str* with prefix "user." is allowed.
+ * For security reasons, only *name__str* values prefixed with "user." are
+ * permitted.
  *
  * Return: length of the xattr value on success, a negative value on error.
  */
 __bpf_kfunc int bpf_cgroup_read_xattr(struct cgroup *cgroup, const char *name__str,
-					struct bpf_dynptr *value_p)
+				      struct bpf_dynptr *value_p)
 {
 	struct bpf_dynptr_kern *value_ptr = (struct bpf_dynptr_kern *)value_p;
 	u32 value_len;
-- 
2.54.0.545.g6539524ca2-goog