Re: [PATCH] i40e: fix integer overflow in i40e_dbg_command_write()

[Tony Nguyen <anthony.l.nguyen@intel.com>]

On 9/8/2026 8:39 AM, rivaldihormat-debug wrote:
> The i40e_dbg_command_write() function uses 'count' from user space
> in kzalloc(count + 1) without validation. If count = 0xFFFFFFFF,
> integer overflow occurs.
> 
> An attacker or local user could trigger a buffer overflow or integer
> overflow by writing large amounts of data to the debugfs file.

I re-ran this through Sashiko and it reported a few things:

Sashiko:
Is this description accurate? count is a size_t. Before the debugfs 
handler runs, vfs_write()->rw_verify_area() clamps it to MAX_RW_COUNT
(INT_MAX & PAGE_MASK), so count + 1 cannot wrap on 32-bit or 64-bit.

The allocation size (count + 1), the copy_from_user() length (count) and 
the cmd_buf[count] = '\0' terminator already match. A very large count 
only makes kzalloc() fail.

This looks like hardening that caps the allocation size, not a fix for a
vulnerability. Could the commit message say that?

> Fix by adding validation:
> if (count == 0 || count > PAGE_SIZE) return -EINVAL.
> 
> PAGE_SIZE is chosen as a common limit for debugfs writes to prevent
> excessive stack/heap allocation.
> 
> Signed-off-by: Rifaldi Hormat <rivaldihormat@gmail.com>
> ---
>   drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> index 0b52509cb14c..74e75504fdda 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> @@ -722,6 +722,8 @@ static ssize_t i40e_dbg_command_write(struct file *filp,
>   	int cnt;
>   
>   	/* don't allow partial writes */
> +	if (count == 0 || count > PAGE_SIZE)
> +		return -EINVAL;

Sashiko:
This changes what userspace sees. A zero-length write used to return 0 
and now returns -EINVAL.

The check also runs before the *ppos != 0 test. A write at a nonzero 
offset that is empty or larger than PAGE_SIZE now gets -EINVAL instead of 0.

Was this intended? If so, should the commit message mention it? 
Returning 0 for count == 0 would match normal write semantics.

>   	if (*ppos != 0)
>   		return 0;
>   
> @@ -1605,6 +1607,8 @@ static ssize_t i40e_dbg_netdev_ops_write(struct file *filp,
>   	int i, cnt;
>   
>   	/* don't allow partial writes */
> +	if (count == 0 || count > PAGE_SIZE)
> +		return -EINVAL;
>   	if (*ppos != 0)
>   		return 0;

Sashiko:
i40e_dbg_netdev_ops_write() has the same change: empty writes and writes 
at a nonzero offset now return -EINVAL instead of 0.

Tony:
For these last two, we should try to maintain existing behavior when 
possible and it seems to align with general expectations.

Thanks,
Tony