linux-kernel - Re: We found a bug in i40e_debugfs.c for the latest linux

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANZ3JQQtC1ytmaqGR3xx6eDVyV-ZJp=hCZDcAJV-ktA2RHvTYA@mail.gmail.com>
Date: Tue, 15 Jul 2025 21:38:11 +0800
From: Wang Haoran <haoranwangsec@...il.com>
To: Simon Horman <horms@...nel.org>
Cc: anthony.l.nguyen@...el.com, przemyslaw.kitszel@...el.com, 
	andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com, 
	kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: We found a bug in i40e_debugfs.c for the latest linux

Hi Simon,

Thanks for the clarification.

We’ve observed that i40e_dbg_command_buf is
initialized with a fixed size of 256 bytes, but we
didn’t find any assignment statements updating
its contents elsewhere in the kernel source code.

We’re unsure whether this buffer could potentially
be used or modified in other contexts that we
might have missed.

If the buffer is indeed isolated and only used
as currently observed, then the current use of
snprintf() should be safe.

We’d appreciate your confirmation on whether
this buffer could potentially be used beyond its
current scope.

Regards,
Wang Haoran

Simon Horman <horms@...nel.org> 于2025年7月15日周二 02:10写道：
>
> On Thu, Jul 10, 2025 at 10:14:18AM +0800, Wang Haoran wrote:
> > Hi, my name is Wang Haoran. We found a bug in the
> > i40e_dbg_command_read function located in
> > drivers/net/ethernet/intel/i40e/i40e_debugfs.c in the latest Linux
> > kernel (version 6.15.5).
> > The buffer "i40e_dbg_command_buf" has a size of 256. When formatted
> > together with the network device name (name), a newline character, and
> > a null terminator, the total formatted string length may exceed the
> > buffer size of 256 bytes.
> > Since "snprintf" returns the total number of bytes that would have
> > been written (the length of  "%s: %s\n" ), this value may exceed the
> > buffer length passed to copy_to_user(), this will ultimatly cause
> > function "copy_to_user" report a buffer overflow error.
> > Replacing snprintf with scnprintf ensures the return value never
> > exceeds the specified buffer size, preventing such issues.
>
> Thanks Wang Haoran.
>
> I agree that using scnprintf() is a better choice here than snprintf().
>
> But it is not clear to me that this is a bug.
>
> I see that i40e_dbg_command_buf is initialised to be the
> empty string. And I don't see it's contents being updated.
>
> While ->name should be no longer than IFNAMSIZ - 1 (=15) bytes long,
> excluding the trailing '\0'.
>
> If so, the string formatted by the line below should always
> comfortably fit within buf_size (256 bytes).
>
> >
> > --- i40e_debugfs.c 2025-07-06 17:04:26.000000000 +0800
> > +++ i40e_debugfs.c 2025-07-09 15:51:47.259130500 +0800
> > @@ -70,7 +70,7 @@
> >   return -ENOSPC;
> >
> >   main_vsi = i40e_pf_get_main_vsi(pf);
> > - len = snprintf(buf, buf_size, "%s: %s\n", main_vsi->netdev->name,
> > + len = scnprintf(buf, buf_size, "%s: %s\n", main_vsi->netdev->name,
> >         i40e_dbg_command_buf);
> >
> >   bytes_not_copied = copy_to_user(buffer, buf, len);
> >
> > Best regards,
> > Wang Haoran
> >