On Thu, 2026-04-23 at 23:30 +0800, 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, 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]>

Reviewed-by: Roberto Sassu <[email protected]>

Thanks

Roberto

> ---
> Changes since v2:
> - adjust the changelog wording to mention why the accumulated length is
>   safe
> - add the blank line after the allocation error path
> - add a comment explaining why snprintf() cannot truncate in the fill loop
> 
> diff --git a/security/integrity/evm/evm_secfs.c 
> b/security/integrity/evm/evm_secfs.c
> index acd840461902..4baf5e23bc97 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)
> @@ -151,16 +151,22 @@ static ssize_t evm_read_xattrs(struct file *filp, char 
> __user *buf,
>               return -ENOMEM;
>       }
>  
> +     temp[size] = '\0';
> +
> +     /*
> +      * No truncation possible: size is computed over the same enabled
> +      * xattrs under xattr_list_mutex, so offset never exceeds size.
> +      */
>       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;
> +             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);
>  


Reply via email to