On Fri, 2026-04-17 at 10:30 +0200, Roberto Sassu wrote:
> On 4/17/2026 2:44 PM, Pengpeng Hou wrote:
> > evm_read_xattrs() allocates size + 1 bytes, fills them from the list of
> > enabled xattrs, and then passes strlen(temp) to
> > simple_read_from_buffer(). When no configured xattrs are enabled, the
> > fill loop stores nothing and temp[0] remains uninitialized, so strlen()
> > reads beyond initialized memory.
> >
> > Explicitly terminate the buffer after allocation, use snprintf() for
> > each formatted line, and pass the accumulated length to
>
> pass the accumulate length (without risk of truncation) to ...
>
> > simple_read_from_buffer().
> >
> > Fixes: fa516b66a1bf ("EVM: Allow runtime modification of the set of
> > verified xattrs")
> > Signed-off-by: Pengpeng Hou <[email protected]>
> > ---
> > Changes since v1:
> > - add the Fixes tag
> > - replace sprintf() with snprintf()
> > - explicitly terminate the buffer instead of switching to kzalloc()
> >
> > security/integrity/evm/evm_secfs.c | 11 ++++++-----
> > 1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/security/integrity/evm/evm_secfs.c
> > b/security/integrity/evm/evm_secfs.c
> > index acd840461902..b7882a4ce9d0 100644
> > --- a/security/integrity/evm/evm_secfs.c
> > +++ b/security/integrity/evm/evm_secfs.c
> > @@ -127,8 +127,8 @@ static ssize_t evm_read_xattrs(struct file *filp, char
> > __user *buf,
> > size_t count, loff_t *ppos)
> > {
> > char *temp;
> > - int offset = 0;
> > - ssize_t rc, size = 0;
> > + size_t offset = 0, size = 0;
> > + ssize_t rc;
> > struct xattr_list *xattr;
> >
> > if (*ppos != 0)
> > @@ -150,17 +150,18 @@ static ssize_t evm_read_xattrs(struct file *filp,
> > char __user *buf,
> > mutex_unlock(&xattr_list_mutex);
> > return -ENOMEM;
> > }
>
> Please add a newline here.
>
> > + temp[size] = '\0';
> >
> > list_for_each_entry(xattr, &evm_config_xattrnames, list) {
> > if (!xattr->enabled)
> > continue;
> >
> > - sprintf(temp + offset, "%s\n", xattr->name);
> > - offset += strlen(xattr->name) + 1;
>
> Also a comment like:
>
> /*
> * No truncation possible: size is computed over the same
> * enabled xattrs under xattr_list_mutex, so offset never exceeds size.
> */
>
> to motivate why it is fine to increment offset without checking.
Any progress? The changes should be straightforward.
Thanks
Roberto
> Thanks
>
> Roberto
>
> > + offset += snprintf(temp + offset, size + 1 - offset, "%s\n",
> > + xattr->name);
> > }
> >
> > mutex_unlock(&xattr_list_mutex);
> > - rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
> > + rc = simple_read_from_buffer(buf, count, ppos, temp, offset);
> >
> > kfree(temp);
> >
