Re: [PATCH bpf-next v3 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 2507 bytes --]

> diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> index 768aca2dc0f0..7abc3f3d1a67 100644
> --- a/fs/bpf_fs_kfuncs.c
> +++ b/fs/bpf_fs_kfuncs.c

[ ... ]

> diff --git a/include/linux/security.h b/include/linux/security.h
> index 153e9043058f..1f8e84e7dd7e 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -68,6 +68,11 @@ struct watch;
>  struct watch_notification;
>  struct lsm_ctx;
>
> +struct xattr_ctx {
> +	struct xattr *xattrs;
> +	int *xattr_count;
> +};
> +

Paul Moore suggested in v2 renaming this to 'struct lsm_xattrs' to avoid
overloading the term 'ctx' in the LSM space. The struct was renamed from
'lsm_xattr_ctx' to 'xattr_ctx' in v3, but 'ctx' is still in the name. Was
there a reason to keep 'ctx' in the name, or would renaming to 'lsm_xattrs'
as Paul suggested be preferable?

>  /* Default (no) options for the capable function */
>  #define CAP_OPT_NONE 0x0
>  /* If capable should audit the security request */

[ ... ]

> diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> index 564071a92d7d..86a8e188b900 100644
> --- a/kernel/bpf/bpf_lsm.c
> +++ b/kernel/bpf/bpf_lsm.c
> @@ -113,6 +113,9 @@ void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog,
>  }
>  #endif
>
> +BTF_ID_LIST_SINGLE(bpf_lsm_inode_init_security_btf_ids, func,
> +		   bpf_lsm_inode_init_security)
> +
>  int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog,
>  			const struct bpf_prog *prog)
>  {
> @@ -137,6 +140,12 @@ int bpf_lsm_verify_prog(struct bpf_verifier_log *vlog,
>  		return -EINVAL;
>  	}
>
> +	/* bpf reserves a fixed number of xattr slots for itself.
> +	 * Set the attach limit so the trampoline rejects excess attaches.
> +	 */

This isn't a bug, but a subsystem pattern flags this as potentially
concerning: the BPF subsystem comment-style rule for files under kernel/bpf/
requires multi-line comments to have the opening /* on its own line, with
comment text starting on the next line. The preferred form would be:

	/*
	 * bpf reserves a fixed number of xattr slots for itself.
	 * Set the attach limit so the trampoline rejects excess attaches.
	 */

> +	if (btf_id == bpf_lsm_inode_init_security_btf_ids[0])
> +		prog->aux->attach_limit = BPF_LSM_INODE_INIT_XATTRS;
> +
>  	return 0;
>  }
>

[ ... ]


---
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/27788616397