| Message ID | 20180718160329.12149-1-bart.vanassche@wdc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Jul 18, 2018 at 09:03:29AM -0700, Bart Van Assche wrote: > This patch avoids that the following compiler warning is reported when > building with gcc 8 and W=1: > > drivers/infiniband/hw/usnic/usnic_fwd.c:95:2: warning: 'strncpy' output may be truncated copying 16 bytes from a string of length 20 [-Wstringop-truncation] > strncpy(ufdev->name, netdev_name(ufdev->netdev), > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > sizeof(ufdev->name) - 1); > ~~~~~~~~~~~~~~~~~~~~~~~~ > > Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com> > Cc: Christian Benvenuti <benve@cisco.com> > Cc: Dave Goodell <dgoodell@cisco.com> > --- > drivers/infiniband/hw/usnic/usnic_fwd.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) I think this is a bug in netdev_name, it shouldn't return a constant string longer than IFNAMSIZ. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2018-07-23 at 15:29 -0600, Jason Gunthorpe wrote: > On Wed, Jul 18, 2018 at 09:03:29AM -0700, Bart Van Assche wrote: > > This patch avoids that the following compiler warning is reported when > > building with gcc 8 and W=1: > > > > drivers/infiniband/hw/usnic/usnic_fwd.c:95:2: warning: 'strncpy' output may be truncated copying 16 bytes from a string of length 20 [-Wstringop-truncation] > > strncpy(ufdev->name, netdev_name(ufdev->netdev), > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > sizeof(ufdev->name) - 1); > > ~~~~~~~~~~~~~~~~~~~~~~~~ > > > > Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com> > > Cc: Christian Benvenuti <benve@cisco.com> > > Cc: Dave Goodell <dgoodell@cisco.com> > > --- > > drivers/infiniband/hw/usnic/usnic_fwd.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > I think this is a bug in netdev_name, it shouldn't return a constant > string longer than IFNAMSIZ. I think that netdev_name() was intended to be used for logging instead of to copying the result into an IFNAMSIZ array. See also commit 571ba4230381 ("netdevice.h: Add netdev_printk helpers like dev_printk"). How about something like the patch below? diff --git a/drivers/infiniband/hw/usnic/usnic_fwd.c b/drivers/infiniband/hw/usnic/usnic_fwd.c index 995a26b65156..e1a832775d2b 100644 --- a/drivers/infiniband/hw/usnic/usnic_fwd.c +++ b/drivers/infiniband/hw/usnic/usnic_fwd.c @@ -92,8 +92,7 @@ struct usnic_fwd_dev *usnic_fwd_dev_alloc(struct pci_dev *pdev) ufdev->pdev = pdev; ufdev->netdev = pci_get_drvdata(pdev); spin_lock_init(&ufdev->lock); - strncpy(ufdev->name, netdev_name(ufdev->netdev), - sizeof(ufdev->name) - 1); + strlcpy(ufdev->name, ufdev->netdev->name, sizeof(ufdev->name)); return ufdev; } Thanks, Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 23, 2018 at 09:55:36PM +0000, Bart Van Assche wrote: > On Mon, 2018-07-23 at 15:29 -0600, Jason Gunthorpe wrote: > > On Wed, Jul 18, 2018 at 09:03:29AM -0700, Bart Van Assche wrote: > > > This patch avoids that the following compiler warning is reported when > > > building with gcc 8 and W=1: > > > > > > drivers/infiniband/hw/usnic/usnic_fwd.c:95:2: warning: 'strncpy' output may be truncated copying 16 bytes from a string of length 20 [-Wstringop-truncation] > > > strncpy(ufdev->name, netdev_name(ufdev->netdev), > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > sizeof(ufdev->name) - 1); > > > ~~~~~~~~~~~~~~~~~~~~~~~~ > > > > > > Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com> > > > Cc: Christian Benvenuti <benve@cisco.com> > > > Cc: Dave Goodell <dgoodell@cisco.com> > > > drivers/infiniband/hw/usnic/usnic_fwd.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > I think this is a bug in netdev_name, it shouldn't return a constant > > string longer than IFNAMSIZ. > > I think that netdev_name() was intended to be used for logging instead of to copying > the result into an IFNAMSIZ array. See also commit 571ba4230381 ("netdevice.h: Add > netdev_printk helpers like dev_printk"). How about something like the patch below? > > diff --git a/drivers/infiniband/hw/usnic/usnic_fwd.c b/drivers/infiniband/hw/usnic/usnic_fwd.c > index 995a26b65156..e1a832775d2b 100644 > +++ b/drivers/infiniband/hw/usnic/usnic_fwd.c > @@ -92,8 +92,7 @@ struct usnic_fwd_dev *usnic_fwd_dev_alloc(struct pci_dev *pdev) > ufdev->pdev = pdev; > ufdev->netdev = pci_get_drvdata(pdev); > spin_lock_init(&ufdev->lock); > - strncpy(ufdev->name, netdev_name(ufdev->netdev), > - sizeof(ufdev->name) - 1); > + strlcpy(ufdev->name, ufdev->netdev->name, sizeof(ufdev->name)); Okay, but now we don't need the strlcpy as it suppresses a useful warning about truncation if the array lengths become mismatched I'm leering about blanket replacing strncpy with strlcpy, almost every case of truncation is actually a problem that should be solved by preventing truncation or failing something.. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/infiniband/hw/usnic/usnic_fwd.c b/drivers/infiniband/hw/usnic/usnic_fwd.c index 995a26b65156..1c7c5e088b9b 100644 --- a/drivers/infiniband/hw/usnic/usnic_fwd.c +++ b/drivers/infiniband/hw/usnic/usnic_fwd.c @@ -92,8 +92,7 @@ struct usnic_fwd_dev *usnic_fwd_dev_alloc(struct pci_dev *pdev) ufdev->pdev = pdev; ufdev->netdev = pci_get_drvdata(pdev); spin_lock_init(&ufdev->lock); - strncpy(ufdev->name, netdev_name(ufdev->netdev), - sizeof(ufdev->name) - 1); + strlcpy(ufdev->name, netdev_name(ufdev->netdev), sizeof(ufdev->name)); return ufdev; }
This patch avoids that the following compiler warning is reported when building with gcc 8 and W=1: drivers/infiniband/hw/usnic/usnic_fwd.c:95:2: warning: 'strncpy' output may be truncated copying 16 bytes from a string of length 20 [-Wstringop-truncation] strncpy(ufdev->name, netdev_name(ufdev->netdev), ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ sizeof(ufdev->name) - 1); ~~~~~~~~~~~~~~~~~~~~~~~~ Signed-off-by: Bart Van Assche <bart.vanassche@wdc.com> Cc: Christian Benvenuti <benve@cisco.com> Cc: Dave Goodell <dgoodell@cisco.com> --- drivers/infiniband/hw/usnic/usnic_fwd.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)