> -----Original Message----- > From: Intel-wired-lan <[email protected]> On Behalf > Of Kunwu Chan > Sent: Friday, July 25, 2025 1:33 PM > To: Keller, Jacob E <[email protected]>; Intel Wired LAN > <[email protected]>; [email protected] > Cc: Simon Horman <[email protected]>; Nguyen, Anthony L > <[email protected]>; Wang Haoran <[email protected]>; > Amir Mohammad Jahangirzad <[email protected]> > Subject: Re: [Intel-wired-lan] [PATCH] i40e: remove read access to > debugfs files > > On 2025/7/23 08:14, Jacob Keller wrote: > > The 'command' and 'netdev_ops' debugfs files are a legacy debugging > > interface supported by the i40e driver since its early days by > commit > > 02e9c290814c ("i40e: debugfs interface"). > > > > Both of these debugfs files provide a read handler which is mostly > > useless, and which is implemented with questionable logic. They both > > use a static > > 256 byte buffer which is initialized to the empty string. In the > case > > of the 'command' file this buffer is literally never used and simply > > wastes space. In the case of the 'netdev_ops' file, the last command > > written is saved here. > > > > On read, the files contents are presented as the name of the device > > followed by a colon and then the contents of their respective static > > buffer. For 'command' this will always be "<device>: ". For > > 'netdev_ops', this will be "<device>: <last command written>". But > > note the buffer is shared between all devices operated by this > module. > > At best, it is mostly meaningless information, and at worse it could > > be accessed simultaneously as there doesn't appear to be any locking > mechanism. > > > > We have also recently received multiple reports for both read > > functions about their use of snprintf and potential overflow that > > could result in reading arbitrary kernel memory. For the 'command' > > file, this is definitely impossible, since the static buffer is > always zero and never written to. > > For the 'netdev_ops' file, it does appear to be possible, if the > user > > carefully crafts the command input, it will be copied into the > buffer, > > which could be large enough to cause snprintf to truncate, which > then > > causes the copy_to_user to read beyond the length of the buffer > > allocated by kzalloc. > > > > A minimal fix would be to replace snprintf() with scnprintf() which > > would cap the return to the number of bytes written, preventing an > > overflow. A more involved fix would be to drop the mostly useless > > static buffers, saving 512 bytes and modifying the read functions to > > stop needing those as input. > > > > Instead, lets just completely drop the read access to these files. > > These are debug interfaces exposed as part of debugfs, and I don't > > believe that dropping read access will break any script, as the > > provided output is pretty useless. You can find the netdev name > > through other more standard interfaces, and the 'netdev_ops' > interface > > can easily result in garbage if you issue simultaneous writes to > multiple devices at once. > > > > In order to properly remove the i40e_dbg_netdev_ops_buf, we need to > > refactor its write function to avoid using the static buffer. > Instead, > > use the same logic as the i40e_dbg_command_write, with an allocated > buffer. > > Update the code to use this instead of the static buffer, and ensure > > we free the buffer on exit. This fixes simultaneous writes to > > 'netdev_ops' on multiple devices, and allows us to remove the now > > unused static buffer along with removing the read access. > > > > Reported-by: Kunwu Chan <[email protected]> > > Closes: > > https://lore.kernel.org/intel-wired-lan/20231208031950.47410-1- > chentao > > @kylinos.cn/ > > Reported-by: Wang Haoran <[email protected]> > > Closes: > > https://lore.kernel.org/all/CANZ3JQRRiOdtfQJoP9QM=6LS1Jto8PGBGw6y7- > TL= > > [email protected]/ > > Reported-by: Amir Mohammad Jahangirzad <[email protected]> > > Closes: > > https://lore.kernel.org/all/20250722115017.206969-1- > a.jahangirzad@gmai > > l.com/ > > Signed-off-by: Jacob Keller <[email protected]> > > --- > > I found several reports of the issues with these read functions > going > > at least as far back as 2023, with suggestions to remove the read > > access even back then. None of the fixes got accepted or applied, > but > > neither did Intel follow up with removing the interfaces. Its time > to > > just drop the read access altogether. > > Reviewed-by: Kunwu Chan <[email protected]> > Reviewed-by: Aleksandr Loktionov <[email protected]>
... > > --- > > > > --- > > base-commit: cf074eca0065bc5142e6004ae236bb35a2687fdf > > change-id: 20250722-jk-drop-debugfs-read-access-994721875248 > > > > Best regards, > > -- > > Jacob Keller <[email protected]> > > > -- > Thanks, > TAO. > --- > “Life finds a way.”
