On Thu, Oct 05, 2023 at 10:25:08PM +0000, Kiyanovski, Arthur wrote: > > -----Original Message----- > > From: Justin Stitt <[email protected]> > > Sent: Thursday, October 5, 2023 3:56 AM > > To: Agroskin, Shay <[email protected]>; Kiyanovski, Arthur > > <[email protected]>; Arinzon, David <[email protected]>; Dagan, > > Noam <[email protected]>; Bshara, Saeed <[email protected]>; David > > S. Miller <[email protected]>; Eric Dumazet <[email protected]>; > > Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]> > > Cc: [email protected]; [email protected]; linux- > > [email protected]; Justin Stitt <[email protected]> > > Subject: [EXTERNAL] [PATCH] net: ena: replace deprecated strncpy with > > strscpy > > > > CAUTION: This email originated from outside of the organization. Do not > > click > > links or open attachments unless you can confirm the sender and know the > > content is safe. > > > > > > > > `strncpy` is deprecated for use on NUL-terminated destination strings [1] > > and as > > such we should prefer more robust and less ambiguous string interfaces. > > > > NUL-padding is not necessary as host_info is initialized to `ena_dev- > > >host_attr.host_info` which is ultimately zero-initialized via > > alloc_etherdev_mq(). > > > > A suitable replacement is `strscpy` [2] due to the fact that it guarantees > > NUL- > > termination on the destination buffer without unnecessarily NUL-padding. > > > > 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 > > Cc: [email protected] > > Signed-off-by: Justin Stitt <[email protected]> > > --- > > Note: build-tested only. > > --- > > drivers/net/ethernet/amazon/ena/ena_netdev.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c > > b/drivers/net/ethernet/amazon/ena/ena_netdev.c > > index f955bde10cf9..3118a617c9b6 100644 > > --- a/drivers/net/ethernet/amazon/ena/ena_netdev.c > > +++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c > > @@ -3276,8 +3276,8 @@ static void ena_config_host_info(struct > > ena_com_dev *ena_dev, struct pci_dev *pd > > strscpy(host_info->kernel_ver_str, utsname()->version, > > sizeof(host_info->kernel_ver_str) - 1); > > host_info->os_dist = 0; > > - strncpy(host_info->os_dist_str, utsname()->release, > > - sizeof(host_info->os_dist_str) - 1); > > + strscpy(host_info->os_dist_str, utsname()->release, > > + sizeof(host_info->os_dist_str)); > > host_info->driver_version = > > (DRV_MODULE_GEN_MAJOR) | > > (DRV_MODULE_GEN_MINOR << > > ENA_ADMIN_HOST_INFO_MINOR_SHIFT) | > > > > --- > > base-commit: cbf3a2cb156a2c911d8f38d8247814b4c07f49a2 > > change-id: 20231005-strncpy-drivers-net-ethernet-amazon-ena-ena_netdev-c- > > 6c4804466aa7 > > > > Best regards, > > -- > > Justin Stitt <[email protected]> > > > > Thanks for submitting this change. > > The change looks good but the sentence "NUL-padding is not necessary as > host_info is initialized to `ena_dev->host_attr.host_info` which is ultimately > zero-initialized via alloc_etherdev_mq()." is inaccurate. > > host_info allocation is done in ena_com_allocate_host_info() via > dma_alloc_coherent() and is not zero initialized by alloc_etherdev_mq(). > > I looked at both the documentation of dma_alloc_coherent() in > https://www.kernel.org/doc/Documentation/DMA-API.txt > as well as the code itself, and (maybe I'm wrong but) I didn't see 100% > guarantees the that the memory is zero-initialized. > > However zero initialization of the destination doesn't matter in this case, > because strscpy() guarantees a NULL termination.
If this is in DMA memory, should the string buffer be %NUL-padded? (Or is it consumed strictly as a %NUL-terminated string?) -Kees -- Kees Cook
