Re: [PATCH v6 bpf-next 1/4] security: introduce struct lsm_xattrs
[email protected] Thu, 30 Jul 2026 23:53:20 +0000
| Newsgroups | org.kernel.vger.selinux,org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(= s) to consider: - [Medium] Latent buffer overflow: `struct lsm_xattrs` lacks a capacity fie= ld, and `lsm_get_xattr_slot()` increments `xattr_count` without bounds chec= king. -- --- Patch [1]: [PATCH v6 bpf-next 1/4] security: introduce struct lsm_xattr= s --- commit 4d965e040bf3aa7aac8ca18905c2c3f09a037f96 Author: David Windsor <[email protected]> security: introduce struct lsm_xattrs This patch introduces struct lsm_xattrs to consolidate the LSM xattr array and its count into a single context object. This preparation allows better compatibility with the BPF verifier for a planned kfunc that lets BPF LSM programs atomically label new inodes. > diff --git a/include/linux/security.h b/include/linux/security.h > --- a/include/linux/security.h > +++ b/include/linux/security.h [ ... ] > @@ -68,6 +68,11 @@ struct watch; > struct watch_notification; > struct lsm_ctx; > =20 > +struct lsm_xattrs { > + struct xattr *xattrs; > + unsigned int xattr_count; > +}; > + [Severity: Medium] Should this struct also track the maximum capacity of the array? Since this object is being introduced to allow dynamically loaded BPF LSM programs to label new inodes, a BPF program calling the planned kfunc multiple times could potentially exceed the allocated capacity. Tracking the max capacity would allow for bounds checking. > /* Default (no) options for the capable function */ > #define CAP_OPT_NONE 0x0 [ ... ] > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h [ ... ] > -static inline struct xattr *lsm_get_xattr_slot(struct xattr *xattrs, > - int *xattr_count) > +static inline struct xattr *lsm_get_xattr_slot(struct lsm_xattrs *ctx) > { > - if (unlikely(!xattrs)) > + if (unlikely(!ctx || !ctx->xattrs)) > return NULL; > - return &xattrs[(*xattr_count)++]; > + return &ctx->xattrs[ctx->xattr_count++]; > } [Severity: Medium] Does this increment need a bounds check against the array capacity? If a dynamic BPF LSM program requests more slots than were allocated in security_inode_init_security() (via blob_sizes.lbs_xattr_count + 1), this would result in an out-of-bounds write to the array. This could subsequently lead to memory corruption during the cleanup loop that calls kfree() on the array elements. --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260730234533.1912= [email protected]?part=3D1