Re: [PATCH net-next 0/7] net: intel: replace deprecated strncpy uses

[Kees Cook]

On Tue, Oct 10, 2023 at 04:22:44PM -0700, Justin Stitt wrote:
> On Tue, Oct 10, 2023 at 4:19 PM Jesse Brandeburg
> <jesse.brandeburg@xxxxxxxxx> wrote:
> >
> > On 10/10/2023 3:26 PM, Justin Stitt wrote:
> > > Hi,
> > >
> > > This series aims to eliminate uses of strncpy() as it is a deprecated
> > > interface [1] with many viable replacements available.
> > >
> > > Predominantly, strscpy() is the go-to replacement as it guarantees
> > > NUL-termination on the destination buffer (which strncpy does not). With
> > > that being said, I did not identify any buffer overread problems as the
> > > size arguments were carefully measured to leave room for trailing
> > > NUL-bytes. Nonetheless, we should favor more robust and less ambiguous
> > > interfaces.
> > >
> > > Previously, each of these patches was sent individually at:
> > > 1) https://lore.kernel.org/all/20231009-strncpy-drivers-net-ethernet-intel-e100-c-v1-1-ca0ff96868a3@xxxxxxxxxx/
> > > 2) https://lore.kernel.org/all/20231010-strncpy-drivers-net-ethernet-intel-e1000-e1000_main-c-v1-1-b1d64581f983@xxxxxxxxxx/
> > > 3) https://lore.kernel.org/all/20231010-strncpy-drivers-net-ethernet-intel-fm10k-fm10k_ethtool-c-v1-1-dbdc4570c5a6@xxxxxxxxxx/
> > > 4) https://lore.kernel.org/all/20231010-strncpy-drivers-net-ethernet-intel-i40e-i40e_ddp-c-v1-1-f01a23394eab@xxxxxxxxxx/
> > > 5) https://lore.kernel.org/all/20231010-strncpy-drivers-net-ethernet-intel-igb-igb_main-c-v1-1-d796234a8abf@xxxxxxxxxx/
> > > 6) https://lore.kernel.org/all/20231010-strncpy-drivers-net-ethernet-intel-igbvf-netdev-c-v1-1-69ccfb2c2aa5@xxxxxxxxxx/
> > > 7) https://lore.kernel.org/all/20231010-strncpy-drivers-net-ethernet-intel-igc-igc_main-c-v1-1-f1f507ecc476@xxxxxxxxxx/
> > >
> > > Consider these dead as this series is their new home :)
> > >
> > > I found all these instances with: $ rg "strncpy\("
> > >
> > > This series may collide in a not-so-nice way with [3]. This series can
> > > go in after that one with a rebase. I'll send a v2 if necessary.
> > >
> > > [3]: https://lore.kernel.org/netdev/20231003183603.3887546-1-jesse.brandeburg@xxxxxxxxx/
> > >
> > > Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
> > > Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
> > > Link: https://github.com/KSPP/linux/issues/90
> > > Signed-off-by: Justin Stitt <justinstitt@xxxxxxxxxx>
> >
> > Thanks Justin for fixing all these!
> >
> > For the series:
> > Reviewed-by: Jesse Brandeburg <jesse.brandeburg@xxxxxxxxx>
> >
> > PS: have you considered adding a script to scripts/coccinelle/api which
> > might catch and try to fix future (ab)users of strncpy?
> 
> There is a checkpatch routine for it. Also, the docs are littered with
> aversions to strncpy. With that being said, I would not be opposed
> to adding more checks, though.
> 
> Once I'm more caught up on all the outstanding strncpy uses,
> I'll look into adding some coccinelle support.

Coccinelle for strncpy is difficult since each set of callers tends to
need careful examination. But the good news here is that at the current
rate, the kernel may be strncpy-free pretty soon. :)

-- 
Kees Cook