[dpdk-dev] [PATCH v5 00/16] Fix default build on gcc8.0.1

[dpdk-dev] [PATCH v5 00/16] Fix default build on gcc8.0.1

[Andy Green]

This series allows dpdk master to build on Fedora 28,
with the x86_64 default config.

Acked-by: Neil Horman <nhorman@tuxdriver.com>

---

Andy Green (16):
      devtools/check-git: provide more generic grep pattern
      net/nfp: solve buffer overflow
      bus/pci: replace strncpy dangerous code
      bus/dpaa: solve inconsistent struct alignment
      net/axgbe: solve broken eeprom string comp
      net/nfp/nfpcore: solve strncpy misuse
      net/nfp/nfpcore: off-by-one and no NUL on strncpy use
      net/nfp: don't memcpy out of source range
      net/qede: strncpy length constant and NUL
      net/qede: solve broken strncpy
      net/sfc: make sure that copied stats name is NUL-terminated
      net/vdev_netvsc: readlink inputs cannot be aliased
      net/vdev_netvsc: 3 x strncpy misuse
      app/proc-info: sprintf overrun bug
      app/test-bbdev: strcpy ok for allocated string
      app/test-bbdev: strcpy ok for allocated string


 app/proc-info/main.c                       |    8 ++++++--
 app/test-bbdev/test_bbdev_vector.c         |    6 +++---
 devtools/check-git-log.sh                  |    4 ++--
 drivers/bus/dpaa/base/qbman/qman.c         |   14 +++++++-------
 drivers/bus/dpaa/include/fsl_qman.h        |   24 +++++++++++++-----------
 drivers/bus/pci/linux/pci.c                |    2 +-
 drivers/net/axgbe/axgbe_phy_impl.c         |    4 ++--
 drivers/net/nfp/nfp_net.c                  |    6 +++---
 drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c |    4 +++-
 drivers/net/nfp/nfpcore/nfp_resource.c     |   10 ++++++----
 drivers/net/qede/base/ecore_int.c          |    8 +++++---
 drivers/net/qede/qede_main.c               |    7 ++++---
 drivers/net/sfc/sfc_ethdev.c               |    6 +++---
 drivers/net/vdev_netvsc/vdev_netvsc.c      |   15 +++++++++------
 14 files changed, 67 insertions(+), 51 deletions(-)

--
Signature

[dpdk-dev] [PATCH v5 01/16] devtools/check-git: provide more generic grep pattern

[Andy Green]

On Fedora 28, every patch is faulted for
"Wrong headline uppercase", because [A-Z] is not
always case sensitive.

Change to use [[:upper:]]

Signed-off-by: Andy Green <andy@warmcat.com>
---
 devtools/check-git-log.sh |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
index c601f6ae9..2542d9ee0 100755
--- a/devtools/check-git-log.sh
+++ b/devtools/check-git-log.sh
@@ -106,8 +106,8 @@ bad=$(echo "$headlines" | grep --color=always \
 
 # check headline lowercase for first words
 bad=$(echo "$headlines" | grep --color=always \
-	-e '^.*[A-Z].*:' \
-	-e ': *[A-Z]' \
+	-e '^.*[[:upper:]].*:' \
+	-e ': *[[:upper:]]' \
 	| sed 's,^,\t,')
 [ -z "$bad" ] || printf "Wrong headline uppercase:\n$bad\n"
 

[dpdk-dev] [PATCH v5 02/16] net/nfp: solve buffer overflow

[Andy Green]

/home/agreen/projects/dpdk/drivers/net/nfp/nfp_net.c: In
function ‘nfp_pf_pci_probe’:
/home/agreen/projects/dpdk/drivers/net/nfp/nfp_net.c:3160:
23: error: ‘%s’ directive writing up to 99 bytes into a
region of size 76 [-Werror=format-overflow=]
  sprintf(fw_name, "%s/%s.nffw", DEFAULT_FW_PATH, serial);

Note fw_buf still has to increase somewhat even after
restricting serial[], since otherwise:

/home/agreen/projects/dpdk/drivers/net/nfp/nfp_net.c: In
function ‘nfp_pf_pci_probe’:
/home/agreen/projects/dpdk/drivers/net/nfp/nfp_net.c:3176:23:
error: ‘%s’ directive writing up to 99 bytes into a region
of size 76 [-Werror=format-overflow=]
  sprintf(fw_name, "%s/%s", DEFAULT_FW_PATH, card);
                       ^~
/home/agreen/projects/dpdk/drivers/net/nfp/nfp_net.c:3262:32:
  err = nfp_fw_upload(dev, nsp, card_desc);
                                ~~~~~~~~~
/home/agreen/projects/dpdk/drivers/net/nfp/nfp_net.c:3176:2:
note: ‘sprintf’ output between 25 and 124 bytes into a
destination of size 100
  sprintf(fw_name, "%s/%s", DEFAULT_FW_PATH, card);

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: 896c265ef954 ("net/nfp: use new CPP interface")
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/net/nfp/nfp_net.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 048324ec9..78113b41b 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -3144,8 +3144,8 @@ nfp_fw_upload(struct rte_pci_device *dev, struct nfp_nsp *nsp, char *card)
 	struct nfp_cpp *cpp = nsp->cpp;
 	int fw_f;
 	char *fw_buf;
-	char fw_name[100];
-	char serial[100];
+	char fw_name[125];
+	char serial[40];
 	struct stat file_stat;
 	off_t fsize, bytes;
 

[dpdk-dev] [PATCH v5 03/16] bus/pci: replace strncpy dangerous code

[Andy Green]

In function ‘pci_get_kernel_driver_by_path’,
    inlined from ‘pci_scan_one.isra.1’ at /home/agreen/projects/dpdk/
	drivers/bus/pci/linux/pci.c:317:8:
/home/agreen/projects/dpdk/drivers/bus/pci/linux/pci.c:57:3: error:
‘strncpy’ specified bound depends on the length of the source argument
[-Werror=stringop-overflow=]
   strncpy(dri_name, name + 1, strlen(name + 1) + 1);

Signed-off-by: Andy Green <andy@warmcat.com>
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Fixes: d9a8cd9595f2 ("pci: add kernel driver type")
Cc: stable@dpdk.org
---
 drivers/bus/pci/linux/pci.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 4630a8057..a73ee49c2 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -54,7 +54,7 @@ pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
 
 	name = strrchr(path, '/');
 	if (name) {
-		strncpy(dri_name, name + 1, strlen(name + 1) + 1);
+		strlcpy(dri_name, name + 1, sizeof(dri_name));
 		return 0;
 	}
 

Re: [dpdk-dev] [PATCH v5 03/16] bus/pci: replace strncpy dangerous code

[Yao, Lei A]

Hi, Andy

This patch will break the vfio-pci driver on my server. 
I can't launch NIC with vfio-pci using testpmd.  Could you have 
a check on this? Thanks a lot!

My server info:
OS: Ubuntu 16.04 LTS
gcc: 5.4.0
kernel: 4.4.0
CPU: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
NIC: Ethernet Controller X710 for 10GbE SFP+ 

My Step:
1. Bind NIC to vfio-pci driver
modprobe vfio-pci
dpdk-devbind.py -b vfio-pci [PCI address of NIC]

2. Launch testpmd;
./x86_64-native-linuxapp-gcc/app/testpmd -c 0x03 -n 4 -- -i

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andy Green
> Sent: Saturday, May 12, 2018 9:48 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v5 03/16] bus/pci: replace strncpy dangerous
> code
> 
> In function ‘pci_get_kernel_driver_by_path’,
>     inlined from ‘pci_scan_one.isra.1’ at /home/agreen/projects/dpdk/
> 	drivers/bus/pci/linux/pci.c:317:8:
> /home/agreen/projects/dpdk/drivers/bus/pci/linux/pci.c:57:3: error:
> ‘strncpy’ specified bound depends on the length of the source argument
> [-Werror=stringop-overflow=]
>    strncpy(dri_name, name + 1, strlen(name + 1) + 1);
> 
> Signed-off-by: Andy Green <andy@warmcat.com>
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> Fixes: d9a8cd9595f2 ("pci: add kernel driver type")
> Cc: stable@dpdk.org
> ---
>  drivers/bus/pci/linux/pci.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> index 4630a8057..a73ee49c2 100644
> --- a/drivers/bus/pci/linux/pci.c
> +++ b/drivers/bus/pci/linux/pci.c
> @@ -54,7 +54,7 @@ pci_get_kernel_driver_by_path(const char *filename,
> char *dri_name)
> 
>  	name = strrchr(path, '/');
>  	if (name) {
> -		strncpy(dri_name, name + 1, strlen(name + 1) + 1);
> +		strlcpy(dri_name, name + 1, sizeof(dri_name));
>  		return 0;
>  	}
> 

[dpdk-dev] [PATCH] bus/pci: correct the earlier strlcpy conversion

[Andy Green]

Fixes: fe5f777b5383 ("bus/pci: replace strncpy by strlcpy")
Signed-off-by: Andy Green <andy@warmcat.com>
---
 drivers/bus/pci/linux/pci.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index a73ee49c2..004600f1c 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -33,7 +33,8 @@
 extern struct rte_pci_bus rte_pci_bus;
 
 static int
-pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
+pci_get_kernel_driver_by_path(const char *filename, char *dri_name,
+			      size_t len)
 {
 	int count;
 	char path[PATH_MAX];
@@ -54,7 +55,7 @@ pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
 
 	name = strrchr(path, '/');
 	if (name) {
-		strlcpy(dri_name, name + 1, sizeof(dri_name));
+		strlcpy(dri_name, name + 1, len);
 		return 0;
 	}
 
@@ -314,7 +315,7 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 
 	/* parse driver */
 	snprintf(filename, sizeof(filename), "%s/driver", dirname);
-	ret = pci_get_kernel_driver_by_path(filename, driver);
+	ret = pci_get_kernel_driver_by_path(filename, driver, sizeof(driver));
 	if (ret < 0) {
 		RTE_LOG(ERR, EAL, "Fail to get kernel driver\n");
 		free(dev);

Re: [dpdk-dev] [PATCH] bus/pci: correct the earlier strlcpy conversion

[Andrew Rybchenko]

On 05/15/2018 10:31 AM, Andy Green wrote:
> Fixes: fe5f777b5383 ("bus/pci: replace strncpy by strlcpy")
> Signed-off-by: Andy Green <andy@warmcat.com>

Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>

Suggest to reword it as:

bus/pci: fix invalid size of driver name buffer

Variable dri_name is a pointer and it is incorrect to use its
size as the buffer size. Caller knows the buffer size and
it is safer to pass it explicitly.

Re: [dpdk-dev] [PATCH] bus/pci: correct the earlier strlcpy conversion

[Thomas Monjalon]

15/05/2018 12:51, Andrew Rybchenko:
> On 05/15/2018 10:31 AM, Andy Green wrote:
> > Fixes: fe5f777b5383 ("bus/pci: replace strncpy by strlcpy")
> > Signed-off-by: Andy Green <andy@warmcat.com>
> 
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 
> Suggest to reword it as:
> 
> bus/pci: fix invalid size of driver name buffer
> 
> Variable dri_name is a pointer and it is incorrect to use its
> size as the buffer size. Caller knows the buffer size and
> it is safer to pass it explicitly.

Applied, thanks

Re: [dpdk-dev] [PATCH v5 03/16] bus/pci: replace strncpy dangerous code

[Andy Green]

On 05/15/2018 02:12 PM, Yao, Lei A wrote:
> Hi, Andy
> 
> This patch will break the vfio-pci driver on my server.
> I can't launch NIC with vfio-pci using testpmd.  Could you have
> a check on this? Thanks a lot!
> 
> My server info:
> OS: Ubuntu 16.04 LTS
> gcc: 5.4.0
> kernel: 4.4.0
> CPU: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
> NIC: Ethernet Controller X710 for 10GbE SFP+
> 
> My Step:
> 1. Bind NIC to vfio-pci driver
> modprobe vfio-pci
> dpdk-devbind.py -b vfio-pci [PCI address of NIC]
> 
> 2. Launch testpmd;
> ./x86_64-native-linuxapp-gcc/app/testpmd -c 0x03 -n 4 -- -i

I don't have any nic to test with.

But it doesn't matter the patch is indeed wrong... I just sent you and 
the list a fix on top of the incomplete patch.  "bus/pci: correct the 
earlier strlcpy conversion"

Sorry...

-Andy

>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andy Green
>> Sent: Saturday, May 12, 2018 9:48 AM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH v5 03/16] bus/pci: replace strncpy dangerous
>> code
>>
>> In function ‘pci_get_kernel_driver_by_path’,
>>      inlined from ‘pci_scan_one.isra.1’ at /home/agreen/projects/dpdk/
>> 	drivers/bus/pci/linux/pci.c:317:8:
>> /home/agreen/projects/dpdk/drivers/bus/pci/linux/pci.c:57:3: error:
>> ‘strncpy’ specified bound depends on the length of the source argument
>> [-Werror=stringop-overflow=]
>>     strncpy(dri_name, name + 1, strlen(name + 1) + 1);
>>
>> Signed-off-by: Andy Green <andy@warmcat.com>
>> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
>> Fixes: d9a8cd9595f2 ("pci: add kernel driver type")
>> Cc: stable@dpdk.org
>> ---
>>   drivers/bus/pci/linux/pci.c |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
>> index 4630a8057..a73ee49c2 100644
>> --- a/drivers/bus/pci/linux/pci.c
>> +++ b/drivers/bus/pci/linux/pci.c
>> @@ -54,7 +54,7 @@ pci_get_kernel_driver_by_path(const char *filename,
>> char *dri_name)
>>
>>   	name = strrchr(path, '/');
>>   	if (name) {
>> -		strncpy(dri_name, name + 1, strlen(name + 1) + 1);
>> +		strlcpy(dri_name, name + 1, sizeof(dri_name));
>>   		return 0;
>>   	}
>>
> 

Re: [dpdk-dev] [PATCH v5 03/16] bus/pci: replace strncpy dangerous code

[Yao, Lei A]

> -----Original Message-----
> From: Andy Green [mailto:andy@warmcat.com]
> Sent: Tuesday, May 15, 2018 3:33 PM
> To: Yao, Lei A <lei.a.yao@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5 03/16] bus/pci: replace strncpy dangerous
> code
> 
> 
> 
> On 05/15/2018 02:12 PM, Yao, Lei A wrote:
> > Hi, Andy
> >
> > This patch will break the vfio-pci driver on my server.
> > I can't launch NIC with vfio-pci using testpmd.  Could you have
> > a check on this? Thanks a lot!
> >
> > My server info:
> > OS: Ubuntu 16.04 LTS
> > gcc: 5.4.0
> > kernel: 4.4.0
> > CPU: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
> > NIC: Ethernet Controller X710 for 10GbE SFP+
> >
> > My Step:
> > 1. Bind NIC to vfio-pci driver
> > modprobe vfio-pci
> > dpdk-devbind.py -b vfio-pci [PCI address of NIC]
> >
> > 2. Launch testpmd;
> > ./x86_64-native-linuxapp-gcc/app/testpmd -c 0x03 -n 4 -- -i
> 
> I don't have any nic to test with.
> 
> But it doesn't matter the patch is indeed wrong... I just sent you and
> the list a fix on top of the incomplete patch.  "bus/pci: correct the
> earlier strlcpy conversion"
> 
> Sorry...
> 
> -Andy
> 
Hi, Andy

Thanks a lot for your quick fix. It can work on my server now. 

BRs
Lei

> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andy Green
> >> Sent: Saturday, May 12, 2018 9:48 AM
> >> To: dev@dpdk.org
> >> Subject: [dpdk-dev] [PATCH v5 03/16] bus/pci: replace strncpy dangerous
> >> code
> >>
> >> In function ‘pci_get_kernel_driver_by_path’,
> >>      inlined from ‘pci_scan_one.isra.1’ at /home/agreen/projects/dpdk/
> >> 	drivers/bus/pci/linux/pci.c:317:8:
> >> /home/agreen/projects/dpdk/drivers/bus/pci/linux/pci.c:57:3: error:
> >> ‘strncpy’ specified bound depends on the length of the source argument
> >> [-Werror=stringop-overflow=]
> >>     strncpy(dri_name, name + 1, strlen(name + 1) + 1);
> >>
> >> Signed-off-by: Andy Green <andy@warmcat.com>
> >> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> >> Fixes: d9a8cd9595f2 ("pci: add kernel driver type")
> >> Cc: stable@dpdk.org
> >> ---
> >>   drivers/bus/pci/linux/pci.c |    2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> >> index 4630a8057..a73ee49c2 100644
> >> --- a/drivers/bus/pci/linux/pci.c
> >> +++ b/drivers/bus/pci/linux/pci.c
> >> @@ -54,7 +54,7 @@ pci_get_kernel_driver_by_path(const char
> *filename,
> >> char *dri_name)
> >>
> >>   	name = strrchr(path, '/');
> >>   	if (name) {
> >> -		strncpy(dri_name, name + 1, strlen(name + 1) + 1);
> >> +		strlcpy(dri_name, name + 1, sizeof(dri_name));
> >>   		return 0;
> >>   	}
> >>
> >

[dpdk-dev] [PATCH v5 04/16] bus/dpaa: solve inconsistent struct alignment

[Andy Green]

The actual descriptor for qm_mr_entry is 64-byte aligned.

But the original code plays a trick, and puts a u8 common
to the three descriptor subtypes in the union afterwards
outside their structure definitions.

Unfortunately since they compose a struct qm_fd with
alignment 8, this trick destroys the ability of the compiler
to understand what has happened, resulting in this kind of
problem:

/home/agreen/projects/dpdk/drivers/bus/dpaa/include/
fsl_qman.h:354:3: error: alignment 1 of ‘struct <anonymous>’
is less than 8 [-Werror=packed-not-aligned]
   } __packed dcern;

on gcc 8 / Fedora 28 out of the box.

This patch moves the u8 verb into the structure definitions
composed into the union, so the alignment of the parent struct
containing the alignment 8 object can also be seen to be
alignment 8 by the compiler.  Uses of .verb are fixed up to use
.ern.verb (the same offset of +0 inside all the structs in
the union).

The final struct layout should be unchanged.

Signed-off-by: Andy Green <andy@warmcat.com>
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Tested-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Fixes: c47ff048b99a ("bus/dpaa: add QMAN driver core routines")
Fixes: f6fadc3e6310 ("bus/dpaa: add QMAN interface driver")
Cc: stable@dpdk.org
---
 drivers/bus/dpaa/base/qbman/qman.c  |   14 +++++++-------
 drivers/bus/dpaa/include/fsl_qman.h |   24 +++++++++++++-----------
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/bus/dpaa/base/qbman/qman.c b/drivers/bus/dpaa/base/qbman/qman.c
index 2810fdd26..27d98cc10 100644
--- a/drivers/bus/dpaa/base/qbman/qman.c
+++ b/drivers/bus/dpaa/base/qbman/qman.c
@@ -314,9 +314,9 @@ static int drain_mr_fqrni(struct qm_portal *p)
 		if (!msg)
 			return 0;
 	}
-	if ((msg->verb & QM_MR_VERB_TYPE_MASK) != QM_MR_VERB_FQRNI) {
+	if ((msg->ern.verb & QM_MR_VERB_TYPE_MASK) != QM_MR_VERB_FQRNI) {
 		/* We aren't draining anything but FQRNIs */
-		pr_err("Found verb 0x%x in MR\n", msg->verb);
+		pr_err("Found verb 0x%x in MR\n", msg->ern.verb);
 		return -1;
 	}
 	qm_mr_next(p);
@@ -483,7 +483,7 @@ static inline void qm_mr_pvb_update(struct qm_portal *portal)
 	/* when accessing 'verb', use __raw_readb() to ensure that compiler
 	 * inlining doesn't try to optimise out "excess reads".
 	 */
-	if ((__raw_readb(&res->verb) & QM_MR_VERB_VBIT) == mr->vbit) {
+	if ((__raw_readb(&res->ern.verb) & QM_MR_VERB_VBIT) == mr->vbit) {
 		mr->pi = (mr->pi + 1) & (QM_MR_SIZE - 1);
 		if (!mr->pi)
 			mr->vbit ^= QM_MR_VERB_VBIT;
@@ -832,7 +832,7 @@ static u32 __poll_portal_slow(struct qman_portal *p, u32 is)
 			goto mr_done;
 		swapped_msg = *msg;
 		hw_fd_to_cpu(&swapped_msg.ern.fd);
-		verb = msg->verb & QM_MR_VERB_TYPE_MASK;
+		verb = msg->ern.verb & QM_MR_VERB_TYPE_MASK;
 		/* The message is a software ERN iff the 0x20 bit is set */
 		if (verb & 0x20) {
 			switch (verb) {
@@ -1666,7 +1666,7 @@ int qman_retire_fq(struct qman_fq *fq, u32 *flags)
 			 */
 			struct qm_mr_entry msg;
 
-			msg.verb = QM_MR_VERB_FQRNI;
+			msg.ern.verb = QM_MR_VERB_FQRNI;
 			msg.fq.fqs = mcr->alterfq.fqs;
 			msg.fq.fqid = fq->fqid;
 #ifdef CONFIG_FSL_QMAN_FQ_LOOKUP
@@ -2643,7 +2643,7 @@ int qman_shutdown_fq(u32 fqid)
 				qm_mr_pvb_update(low_p);
 				msg = qm_mr_current(low_p);
 				while (msg) {
-					if ((msg->verb &
+					if ((msg->ern.verb &
 					     QM_MR_VERB_TYPE_MASK)
 					    == QM_MR_VERB_FQRN)
 						found_fqrn = 1;
@@ -2711,7 +2711,7 @@ int qman_shutdown_fq(u32 fqid)
 			qm_mr_pvb_update(low_p);
 			msg = qm_mr_current(low_p);
 			while (msg) {
-				if ((msg->verb & QM_MR_VERB_TYPE_MASK) ==
+				if ((msg->ern.verb & QM_MR_VERB_TYPE_MASK) ==
 				    QM_MR_VERB_FQRL)
 					orl_empty = 1;
 				qm_mr_next(low_p);
diff --git a/drivers/bus/dpaa/include/fsl_qman.h b/drivers/bus/dpaa/include/fsl_qman.h
index e9793f30d..e4ad7ae48 100644
--- a/drivers/bus/dpaa/include/fsl_qman.h
+++ b/drivers/bus/dpaa/include/fsl_qman.h
@@ -284,20 +284,20 @@ static inline dma_addr_t qm_sg_addr(const struct qm_sg_entry *sg)
 	} while (0)
 
 /* See 1.5.8.1: "Enqueue Command" */
-struct qm_eqcr_entry {
+struct __rte_aligned(8) qm_eqcr_entry {
 	u8 __dont_write_directly__verb;
 	u8 dca;
 	u16 seqnum;
 	u32 orp;	/* 24-bit */
 	u32 fqid;	/* 24-bit */
 	u32 tag;
-	struct qm_fd fd;
+	struct qm_fd fd; /* this has alignment 8 */
 	u8 __reserved3[32];
 } __packed;
 
 
 /* "Frame Dequeue Response" */
-struct qm_dqrr_entry {
+struct __rte_aligned(8) qm_dqrr_entry {
 	u8 verb;
 	u8 stat;
 	u16 seqnum;	/* 15-bit */
@@ -305,7 +305,7 @@ struct qm_dqrr_entry {
 	u8 __reserved2[3];
 	u32 fqid;	/* 24-bit */
 	u32 contextB;
-	struct qm_fd fd;
+	struct qm_fd fd; /* this has alignment 8 */
 	u8 __reserved4[32];
 };
 
@@ -323,18 +323,19 @@ struct qm_dqrr_entry {
 /* "ERN Message Response" */
 /* "FQ State Change Notification" */
 struct qm_mr_entry {
-	u8 verb;
 	union {
 		struct {
+			u8 verb;
 			u8 dca;
 			u16 seqnum;
 			u8 rc;		/* Rejection Code */
 			u32 orp:24;
 			u32 fqid;	/* 24-bit */
 			u32 tag;
-			struct qm_fd fd;
-		} __packed ern;
+			struct qm_fd fd; /* this has alignment 8 */
+		} __packed __rte_aligned(8) ern;
 		struct {
+			u8 verb;
 #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
 			u8 colour:2;	/* See QM_MR_DCERN_COLOUR_* */
 			u8 __reserved1:4;
@@ -349,18 +350,19 @@ struct qm_mr_entry {
 			u32 __reserved3:24;
 			u32 fqid;	/* 24-bit */
 			u32 tag;
-			struct qm_fd fd;
-		} __packed dcern;
+			struct qm_fd fd; /* this has alignment 8 */
+		} __packed __rte_aligned(8) dcern;
 		struct {
+			u8 verb;
 			u8 fqs;		/* Frame Queue Status */
 			u8 __reserved1[6];
 			u32 fqid;	/* 24-bit */
 			u32 contextB;
 			u8 __reserved2[16];
-		} __packed fq;		/* FQRN/FQRNI/FQRL/FQPN */
+		} __packed __rte_aligned(8) fq;	/* FQRN/FQRNI/FQRL/FQPN */
 	};
 	u8 __reserved2[32];
-} __packed;
+} __packed __rte_aligned(8);
 #define QM_MR_VERB_VBIT			0x80
 /*
  * ERNs originating from direct-connect portals ("dcern") use 0x20 as a verb

[dpdk-dev] [PATCH v5 05/16] net/axgbe: solve broken eeprom string comp

[Andy Green]

/home/agreen/projects/dpdk/drivers/net/axgbe/axgbe_phy_impl.c:576:6:
error: ‘__builtin_memcmp_eq’ reading 16 bytes from a region of
size 9 [-Werror=stringop-overflow=]
  if (memcmp(&sfp_eeprom->base[AXGBE_SFP_BASE_VENDOR_NAME],
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      AXGBE_BEL_FUSE_VENDOR, AXGBE_SFP_BASE_VENDOR_NAME_LEN))

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/net/axgbe/axgbe_phy_impl.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index dfa908dd8..973177f69 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -574,11 +574,11 @@ static bool axgbe_phy_belfuse_parse_quirks(struct axgbe_port *pdata)
 	struct axgbe_sfp_eeprom *sfp_eeprom = &phy_data->sfp_eeprom;
 
 	if (memcmp(&sfp_eeprom->base[AXGBE_SFP_BASE_VENDOR_NAME],
-		   AXGBE_BEL_FUSE_VENDOR, AXGBE_SFP_BASE_VENDOR_NAME_LEN))
+		   AXGBE_BEL_FUSE_VENDOR, strlen(AXGBE_BEL_FUSE_VENDOR)))
 		return false;
 
 	if (!memcmp(&sfp_eeprom->base[AXGBE_SFP_BASE_VENDOR_PN],
-		    AXGBE_BEL_FUSE_PARTNO, AXGBE_SFP_BASE_VENDOR_PN_LEN)) {
+		    AXGBE_BEL_FUSE_PARTNO, strlen(AXGBE_BEL_FUSE_PARTNO))) {
 		phy_data->sfp_base = AXGBE_SFP_BASE_1000_SX;
 		phy_data->sfp_cable = AXGBE_SFP_CABLE_ACTIVE;
 		phy_data->sfp_speed = AXGBE_SFP_SPEED_1000;

[dpdk-dev] [PATCH v5 06/16] net/nfp/nfpcore: solve strncpy misuse

[Andy Green]

Signed-off-by: Andy Green <andy@warmcat.com>
Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Tested-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Fixes: c7e9729da6b5 ("net/nfp: support CPP")
---
 drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
index 4e6c66624..52b294888 100644
--- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
+++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
@@ -31,6 +31,8 @@
 #include <sys/file.h>
 #include <sys/stat.h>
 
+#include <rte_string_fns.h>
+
 #include "nfp_cpp.h"
 #include "nfp_target.h"
 #include "nfp6000/nfp6000.h"
@@ -846,7 +848,7 @@ nfp6000_init(struct nfp_cpp *cpp, const char *devname)
 
 
 	memset(desc->busdev, 0, BUSDEV_SZ);
-	strncpy(desc->busdev, devname, strlen(devname));
+	strlcpy(desc->busdev, devname, sizeof(desc->busdev));
 
 	ret = nfp_acquire_process_lock(desc);
 	if (ret)

[dpdk-dev] [PATCH v5 07/16] net/nfp/nfpcore: off-by-one and no NUL on strncpy use

[Andy Green]

/home/agreen/projects/dpdk/drivers/net/nfp/nfpcore/nfp_resource.c:
76:2:error: ‘strncpy’ output may be truncated copying 8 bytes from
a string of length 8 [-Werror=stringop-truncation]
  strncpy(name_pad, res->name, sizeof(name_pad));

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: c7e9729da6b5 ("net/nfp: support CPP")
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/net/nfp/nfpcore/nfp_resource.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/nfp/nfpcore/nfp_resource.c b/drivers/net/nfp/nfpcore/nfp_resource.c
index e1df2b2e1..dd41fa4de 100644
--- a/drivers/net/nfp/nfpcore/nfp_resource.c
+++ b/drivers/net/nfp/nfpcore/nfp_resource.c
@@ -7,6 +7,8 @@
 #include <time.h>
 #include <endian.h>
 
+#include <rte_string_fns.h>
+
 #include "nfp_cpp.h"
 #include "nfp6000/nfp6000.h"
 #include "nfp_resource.h"
@@ -65,22 +67,22 @@ struct nfp_resource {
 static int
 nfp_cpp_resource_find(struct nfp_cpp *cpp, struct nfp_resource *res)
 {
-	char name_pad[NFP_RESOURCE_ENTRY_NAME_SZ] = {};
+	char name_pad[NFP_RESOURCE_ENTRY_NAME_SZ + 2];
 	struct nfp_resource_entry entry;
 	uint32_t cpp_id, key;
 	int ret, i;
 
 	cpp_id = NFP_CPP_ID(NFP_RESOURCE_TBL_TARGET, 3, 0);  /* Atomic read */
 
-	memset(name_pad, 0, NFP_RESOURCE_ENTRY_NAME_SZ);
-	strncpy(name_pad, res->name, sizeof(name_pad));
+	memset(name_pad, 0, sizeof(name_pad));
+	strlcpy(name_pad, res->name, sizeof(name_pad));
 
 	/* Search for a matching entry */
 	if (!memcmp(name_pad, NFP_RESOURCE_TBL_NAME "\0\0\0\0\0\0\0\0", 8)) {
 		printf("Grabbing device lock not supported\n");
 		return -EOPNOTSUPP;
 	}
-	key = nfp_crc32_posix(name_pad, sizeof(name_pad));
+	key = nfp_crc32_posix(name_pad, NFP_RESOURCE_ENTRY_NAME_SZ);
 
 	for (i = 0; i < NFP_RESOURCE_TBL_ENTRIES; i++) {
 		uint64_t addr = NFP_RESOURCE_TBL_BASE +

[dpdk-dev] [PATCH v5 08/16] net/nfp: don't memcpy out of source range

[Andy Green]

/home/agreen/projects/dpdk/drivers/net/nfp/nfp_net.c:669:2:
error: ‘memcpy’ forming offset [5, 6] is out of the bounds
[0, 4] of object ‘tmp’ with type ‘uint32_t’ {aka ‘unsigned
int’} [-Werror=array-bounds]  memcpy(&hw->mac_addr[0],
&tmp, sizeof(struct ether_addr));

Signed-off-by: Andy Green <andy@warmcat.com>
Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Tested-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Fixes: e6decee38209 ("net/nfp: use random MAC address if not configured")
Cc: stable@dpdk.org
---
 drivers/net/nfp/nfp_net.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 78113b41b..f114b1839 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -666,7 +666,7 @@ nfp_net_vf_read_mac(struct nfp_net_hw *hw)
 	uint32_t tmp;
 
 	tmp = rte_be_to_cpu_32(nn_cfg_readl(hw, NFP_NET_CFG_MACADDR));
-	memcpy(&hw->mac_addr[0], &tmp, sizeof(struct ether_addr));
+	memcpy(&hw->mac_addr[0], &tmp, 4);
 
 	tmp = rte_be_to_cpu_32(nn_cfg_readl(hw, NFP_NET_CFG_MACADDR + 4));
 	memcpy(&hw->mac_addr[4], &tmp, 2);

[dpdk-dev] [PATCH v5 09/16] net/qede: strncpy length constant and NUL

[Andy Green]

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: 8427c6647964 ("net/qede/base: add attention formatting string")
Cc: stable@dpdk.org
---
 drivers/net/qede/base/ecore_int.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/qede/base/ecore_int.c b/drivers/net/qede/base/ecore_int.c
index f43781ba4..d9e22b5ed 100644
--- a/drivers/net/qede/base/ecore_int.c
+++ b/drivers/net/qede/base/ecore_int.c
@@ -6,6 +6,8 @@
  * See LICENSE.qede_pmd for copyright and licensing details.
  */
 
+#include <rte_string_fns.h>
+
 #include "bcm_osal.h"
 #include "ecore.h"
 #include "ecore_spq.h"
@@ -1104,9 +1106,9 @@ static enum _ecore_status_t ecore_int_deassertion(struct ecore_hwfn *p_hwfn,
 							      p_aeu->bit_name,
 							      num);
 					else
-						OSAL_STRNCPY(bit_name,
-							     p_aeu->bit_name,
-							     30);
+						strlcpy(bit_name,
+							p_aeu->bit_name,
+							sizeof(bit_name));
 
 					/* We now need to pass bitmask in its
 					 * correct position.

[dpdk-dev] [PATCH v5 10/16] net/qede: solve broken strncpy

[Andy Green]

/home/agreen/projects/dpdk/drivers/net/qede/qede_main.c: In function
‘qed_slowpath_start’:
/home/agreen/projects/dpdk/drivers/net/qede/qede_main.c:307:3:
error: ‘strncpy’ output may be truncated copying 12 bytes from a
string of length 127 [-Werror=stringop-truncation]
   strncpy((char *)drv_version.name, (const char *)params->name,
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    MCP_DRV_VER_STR_SIZE - 4);
    ~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: 86a2265e59d7 ("qede: add SRIOV support")
Cc: stable@dpdk.org
---
 drivers/net/qede/qede_main.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/qede/qede_main.c b/drivers/net/qede/qede_main.c
index 2333ca073..fcfc32d0d 100644
--- a/drivers/net/qede/qede_main.c
+++ b/drivers/net/qede/qede_main.c
@@ -9,6 +9,7 @@
 #include <limits.h>
 #include <time.h>
 #include <rte_alarm.h>
+#include <rte_string_fns.h>
 
 #include "qede_ethdev.h"
 
@@ -303,9 +304,9 @@ static int qed_slowpath_start(struct ecore_dev *edev,
 		drv_version.version = (params->drv_major << 24) |
 		    (params->drv_minor << 16) |
 		    (params->drv_rev << 8) | (params->drv_eng);
-		/* TBD: strlcpy() */
-		strncpy((char *)drv_version.name, (const char *)params->name,
-			MCP_DRV_VER_STR_SIZE - 4);
+		strlcpy((char *)drv_version.name, (const char *)params->name,
+			sizeof(drv_version.name));
+		drv_version.name[sizeof(drv_version.name) - 1] = '\0';
 		rc = ecore_mcp_send_drv_version(hwfn, hwfn->p_main_ptt,
 						&drv_version);
 		if (rc) {

[dpdk-dev] [PATCH v5 11/16] net/sfc: make sure that copied stats name is NUL-terminated

[Andy Green]

Signed-off-by: Andy Green <andy@warmcat.com>
Acked-by: Andrew Rybchenko <arybchenko@oktetlabs.ru>
Fixes: 73280c1e4ff2 ("net/sfc: support xstats retrieval by ID")
Fixes: 7b9891769f4b ("net/sfc: support extended statistics")
Cc: stable@dpdk.org
---
 drivers/net/sfc/sfc_ethdev.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index e42d55350..a8c0f8e19 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -13,6 +13,7 @@
 #include <rte_pci.h>
 #include <rte_bus_pci.h>
 #include <rte_errno.h>
+#include <rte_string_fns.h>
 
 #include "efx.h"
 
@@ -663,7 +664,7 @@ sfc_xstats_get_names(struct rte_eth_dev *dev,
 	for (i = 0; i < EFX_MAC_NSTATS; ++i) {
 		if (EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i)) {
 			if (xstats_names != NULL && nstats < xstats_count)
-				strncpy(xstats_names[nstats].name,
+				strlcpy(xstats_names[nstats].name,
 					efx_mac_stat_name(sa->nic, i),
 					sizeof(xstats_names[0].name));
 			nstats++;
@@ -741,9 +742,8 @@ sfc_xstats_get_names_by_id(struct rte_eth_dev *dev,
 		if ((ids == NULL) || (ids[nb_written] == nb_supported)) {
 			char *name = xstats_names[nb_written++].name;
 
-			strncpy(name, efx_mac_stat_name(sa->nic, i),
+			strlcpy(name, efx_mac_stat_name(sa->nic, i),
 				sizeof(xstats_names[0].name));
-			name[sizeof(xstats_names[0].name) - 1] = '\0';
 		}
 
 		++nb_supported;

[dpdk-dev] [PATCH v5 12/16] net/vdev_netvsc: readlink inputs cannot be aliased

[Andy Green]

/home/agreen/projects/dpdk/drivers/net/vdev_netvsc/
vdev_netvsc.c:335:2:error: passing argument 2 to restrict-
qualified parameter aliases with argument 1 [-Werror=restrict]
  ret = readlink(buf, buf, size);
  ^~~

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: e7dc5d7becc5 ("net/vdev_netvsc: implement core functionality")
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/net/vdev_netvsc/vdev_netvsc.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c b/drivers/net/vdev_netvsc/vdev_netvsc.c
index c321a9f1b..dca25761d 100644
--- a/drivers/net/vdev_netvsc/vdev_netvsc.c
+++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
@@ -327,12 +327,14 @@ static int
 vdev_netvsc_sysfs_readlink(char *buf, size_t size, const char *if_name,
 			   const char *relpath)
 {
+	char in[160];
 	int ret;
 
-	ret = snprintf(buf, size, "/sys/class/net/%s/%s", if_name, relpath);
-	if (ret == -1 || (size_t)ret >= size)
+	ret = snprintf(in, sizeof(in) - 1, "/sys/class/net/%s/%s",
+		       if_name, relpath);
+	if (ret == -1 || (size_t)ret >= sizeof(in) - 1)
 		return -ENOBUFS;
-	ret = readlink(buf, buf, size);
+	ret = readlink(in, buf, size);
 	if (ret == -1)
 		return -errno;
 	if ((size_t)ret >= size - 1)

Re: [dpdk-dev] [PATCH v5 12/16] net/vdev_netvsc: readlink inputs cannot be aliased

[Matan Azrad]

Hi Andy

From: Andy Green
> /home/agreen/projects/dpdk/drivers/net/vdev_netvsc/

Please replace "/home/agreen/projects/dpdk" in $DPDK_DIR,
I think this is relevant for all the series.

> vdev_netvsc.c:335:2:error: passing argument 2 to restrict- qualified parameter
> aliases with argument 1 [-Werror=restrict]
>   ret = readlink(buf, buf, size);
>   ^~~

Where this compilation error does come from?
What is the ARCH\gcc version? Why was it compiled well and now it not?
And the title should be something like, fix compilation issue in [distro X][arch Y] [gcc Z] [any other compilation specifications]
Please specify only the specification which causes the error.
I think this is relevant for all the series too.

> Signed-off-by: Andy Green <andy@warmcat.com>
> Fixes: e7dc5d7becc5 ("net/vdev_netvsc: implement core functionality")
What's about backporting it to stable?

The fixes line (and Cc lines) should be before the Signed-off-by line and an empty line should be between them,
I think this is relevant for all the series too.
 
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
>  drivers/net/vdev_netvsc/vdev_netvsc.c |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c
> b/drivers/net/vdev_netvsc/vdev_netvsc.c
> index c321a9f1b..dca25761d 100644
> --- a/drivers/net/vdev_netvsc/vdev_netvsc.c
> +++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
> @@ -327,12 +327,14 @@ static int
>  vdev_netvsc_sysfs_readlink(char *buf, size_t size, const char *if_name,
>  			   const char *relpath)
>  {
> +	char in[160];

Where the number 160 is come from?
Why not RTE_MAX(sizeof(ctx->yield), 256u) as defined for buf?

>  	int ret;
> 
> -	ret = snprintf(buf, size, "/sys/class/net/%s/%s", if_name, relpath);
> -	if (ret == -1 || (size_t)ret >= size)
> +	ret = snprintf(in, sizeof(in) - 1, "/sys/class/net/%s/%s",
> +		       if_name, relpath);
> +	if (ret == -1 || (size_t)ret >= sizeof(in) - 1)

I don’t think you need the " - 1" here.
 
>  		return -ENOBUFS;
> -	ret = readlink(buf, buf, size);
> +	ret = readlink(in, buf, size);
>  	if (ret == -1)
>  		return -errno;
>  	if ((size_t)ret >= size - 1)

Re: [dpdk-dev] [PATCH v5 12/16] net/vdev_netvsc: readlink inputs cannot be aliased

[Andy Green]

On 05/13/2018 03:20 PM, Matan Azrad wrote:
> Hi Andy
> 
> From: Andy Green
>> /home/agreen/projects/dpdk/drivers/net/vdev_netvsc/
> 
> Please replace "/home/agreen/projects/dpdk" in $DPDK_DIR,
> I think this is relevant for all the series.
> 
>> vdev_netvsc.c:335:2:error: passing argument 2 to restrict- qualified parameter
>> aliases with argument 1 [-Werror=restrict]
>>    ret = readlink(buf, buf, size);
>>    ^~~
> 
> Where this compilation error does come from?
> What is the ARCH\gcc version? Why was it compiled well and now it not?

Because as the series cover page says, these problems are all coming 
from Fedora 28 + gcc8.0.1 (on x86_64) which has some very cool new 
static analysis features.

In each case gcc8 has complained, the code was broken actually, the 
tools until now were not good enough to tell us the stuff needed fixing 
is why it's coming now.

> And the title should be something like, fix compilation issue in [distro X][arch Y] [gcc Z] [any other compilation specifications]
> Please specify only the specification which causes the error.
> I think this is relevant for all the series too.
> 
>> Signed-off-by: Andy Green <andy@warmcat.com>
>> Fixes: e7dc5d7becc5 ("net/vdev_netvsc: implement core functionality")
> What's about backporting it to stable?

That's something for the maintainer(s) to take care about.

> The fixes line (and Cc lines) should be before the Signed-off-by line and an empty line should be between them,
> I think this is relevant for all the series too.

 From my perspective, which is not that of a paid dev for this project 
who has now spent a week on these patches, functionally it doesn't 
really matter which order those things are in.

If the maintainer really cares he can munge the patches to be what he 
prefers.

If this turns into a job for me and I have more patches later, I will 
try to align with the project-specific requirements better.

>> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
>> ---
>>   drivers/net/vdev_netvsc/vdev_netvsc.c |    8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c
>> b/drivers/net/vdev_netvsc/vdev_netvsc.c
>> index c321a9f1b..dca25761d 100644
>> --- a/drivers/net/vdev_netvsc/vdev_netvsc.c
>> +++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
>> @@ -327,12 +327,14 @@ static int
>>   vdev_netvsc_sysfs_readlink(char *buf, size_t size, const char *if_name,
>>   			   const char *relpath)
>>   {
>> +	char in[160];
> 
> Where the number 160 is come from?
> Why not RTE_MAX(sizeof(ctx->yield), 256u) as defined for buf?

You're right, it's just a random pathlength-like number.
I borrowed the sizing code you mentioned from the function below instead 
(whose random pathlength-like number is at least consistent with the 
rest of the code).

>>   	int ret;
>>
>> -	ret = snprintf(buf, size, "/sys/class/net/%s/%s", if_name, relpath);
>> -	if (ret == -1 || (size_t)ret >= size)
>> +	ret = snprintf(in, sizeof(in) - 1, "/sys/class/net/%s/%s",
>> +		       if_name, relpath);
>> +	if (ret == -1 || (size_t)ret >= sizeof(in) - 1)
> 
> I don’t think you need the " - 1" here.

Yes, I changed it thanks.

-Andy

>>   		return -ENOBUFS;
>> -	ret = readlink(buf, buf, size);
>> +	ret = readlink(in, buf, size);
>>   	if (ret == -1)
>>   		return -errno;
>>   	if ((size_t)ret >= size - 1)
> 

[dpdk-dev] [PATCH v5 13/16] net/vdev_netvsc: 3 x strncpy misuse

[Andy Green]

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: e7dc5d7becc5 ("net/vdev_netvsc: implement core functionality")
Cc: stable@dpdk.org
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/net/vdev_netvsc/vdev_netvsc.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c b/drivers/net/vdev_netvsc/vdev_netvsc.c
index dca25761d..f1d036152 100644
--- a/drivers/net/vdev_netvsc/vdev_netvsc.c
+++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
@@ -35,6 +35,7 @@
 #include <rte_hypervisor.h>
 #include <rte_kvargs.h>
 #include <rte_log.h>
+#include <rte_string_fns.h>
 
 #define VDEV_NETVSC_DRIVER net_vdev_netvsc
 #define VDEV_NETVSC_DRIVER_NAME RTE_STR(VDEV_NETVSC_DRIVER)
@@ -182,7 +183,7 @@ vdev_netvsc_foreach_iface(int (*func)(const struct if_nameindex *iface,
 		is_netvsc_ret = vdev_netvsc_iface_is_netvsc(&iface[i]) ? 1 : 0;
 		if (is_netvsc ^ is_netvsc_ret)
 			continue;
-		strncpy(req.ifr_name, iface[i].if_name, sizeof(req.ifr_name));
+		strlcpy(req.ifr_name, iface[i].if_name, sizeof(req.ifr_name));
 		if (ioctl(s, SIOCGIFHWADDR, &req) == -1) {
 			DRV_LOG(WARNING, "cannot retrieve information about"
 					 " interface \"%s\": %s",
@@ -384,7 +385,7 @@ vdev_netvsc_device_probe(const struct if_nameindex *iface,
 		DRV_LOG(DEBUG,
 			"NetVSC interface \"%s\" (index %u) renamed \"%s\"",
 			ctx->if_name, ctx->if_index, iface->if_name);
-		strncpy(ctx->if_name, iface->if_name, sizeof(ctx->if_name));
+		strlcpy(ctx->if_name, iface->if_name, sizeof(ctx->if_name));
 		return 0;
 	}
 	if (!is_same_ether_addr(eth_addr, &ctx->if_addr))
@@ -582,7 +583,7 @@ vdev_netvsc_netvsc_probe(const struct if_nameindex *iface,
 		goto error;
 	}
 	ctx->id = vdev_netvsc_ctx_count;
-	strncpy(ctx->if_name, iface->if_name, sizeof(ctx->if_name));
+	strlcpy(ctx->if_name, iface->if_name, sizeof(ctx->if_name));
 	ctx->if_index = iface->if_index;
 	ctx->if_addr = *eth_addr;
 	ctx->pipe[0] = -1;

Re: [dpdk-dev] [PATCH v5 13/16] net/vdev_netvsc: 3 x strncpy misuse

[Matan Azrad]

Hi Andy

From: Andy Green
> Signed-off-by: Andy Green <andy@warmcat.com>
> Fixes: e7dc5d7becc5 ("net/vdev_netvsc: implement core functionality")
> Cc: stable@dpdk.org

No need the fix line, it is just conversion.

Actually the strlcpy was introduced later and the next commit should have it:
c022cb400e92 ("convert snprintf to strlcpy")

which came after the vdev_netvsc driver had been introduced.

The title can be as above;
net/vdev_netvsc: convert snprintf to strlcpy

The body could be:

Continue snprintf to strlcpy conversions started by commit c022cb400e92 ("convert snprintf to strlcpy").

> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
>  drivers/net/vdev_netvsc/vdev_netvsc.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c
> b/drivers/net/vdev_netvsc/vdev_netvsc.c
> index dca25761d..f1d036152 100644
> --- a/drivers/net/vdev_netvsc/vdev_netvsc.c
> +++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
> @@ -35,6 +35,7 @@
>  #include <rte_hypervisor.h>
>  #include <rte_kvargs.h>
>  #include <rte_log.h>
> +#include <rte_string_fns.h>
> 
>  #define VDEV_NETVSC_DRIVER net_vdev_netvsc  #define
> VDEV_NETVSC_DRIVER_NAME RTE_STR(VDEV_NETVSC_DRIVER) @@ -182,7
> +183,7 @@ vdev_netvsc_foreach_iface(int (*func)(const struct if_nameindex
> *iface,
>  		is_netvsc_ret = vdev_netvsc_iface_is_netvsc(&iface[i]) ? 1 : 0;
>  		if (is_netvsc ^ is_netvsc_ret)
>  			continue;
> -		strncpy(req.ifr_name, iface[i].if_name, sizeof(req.ifr_name));
> +		strlcpy(req.ifr_name, iface[i].if_name, sizeof(req.ifr_name));
>  		if (ioctl(s, SIOCGIFHWADDR, &req) == -1) {
>  			DRV_LOG(WARNING, "cannot retrieve information
> about"
>  					 " interface \"%s\": %s",
> @@ -384,7 +385,7 @@ vdev_netvsc_device_probe(const struct if_nameindex
> *iface,
>  		DRV_LOG(DEBUG,
>  			"NetVSC interface \"%s\" (index %u) renamed \"%s\"",
>  			ctx->if_name, ctx->if_index, iface->if_name);
> -		strncpy(ctx->if_name, iface->if_name, sizeof(ctx->if_name));
> +		strlcpy(ctx->if_name, iface->if_name, sizeof(ctx->if_name));
>  		return 0;
>  	}
>  	if (!is_same_ether_addr(eth_addr, &ctx->if_addr)) @@ -582,7 +583,7
> @@ vdev_netvsc_netvsc_probe(const struct if_nameindex *iface,
>  		goto error;
>  	}
>  	ctx->id = vdev_netvsc_ctx_count;
> -	strncpy(ctx->if_name, iface->if_name, sizeof(ctx->if_name));
> +	strlcpy(ctx->if_name, iface->if_name, sizeof(ctx->if_name));
>  	ctx->if_index = iface->if_index;
>  	ctx->if_addr = *eth_addr;
>  	ctx->pipe[0] = -1;

[dpdk-dev] [PATCH v5 14/16] app/proc-info: sprintf overrun bug

[Andy Green]

/home/agreen/projects/dpdk/app/proc-info/main.c: In function
‘nic_xstats_display’:
/home/agreen/projects/dpdk/app/proc-info/main.c:495:45:
error: ‘%s’ directive writing up to 255 bytes into a region
of size between 165 and 232 [-Werror=format-overflow=]
    sprintf(buf, "PUTVAL %s/dpdkstat-port.%u/%s-%s N:%"
                                             ^~
     PRIu64"\n", host_id, port_id, counter_type,
                                   ~~~~~~~~~~~~
/home/agreen/projects/dpdk/app/proc-info/main.c:495:4: note:
‘sprintf’ output between 31 and 435 bytes into a destination
of size 256
    sprintf(buf, "PUTVAL %s/dpdkstat-port.%u/%s-%s N:%"
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     PRIu64"\n", host_id, port_id, counter_type,
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     xstats_names[i].name, values[i]);

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: 2deb6b5246d7 ("app/procinfo: add collectd format and host id")
Cc: stable@dpdk.org
---
 app/proc-info/main.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 539e13243..c20effa4f 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -488,14 +488,18 @@ nic_xstats_display(uint16_t port_id)
 		if (enable_collectd_format) {
 			char counter_type[MAX_STRING_LEN];
 			char buf[MAX_STRING_LEN];
+			size_t n;
 
 			collectd_resolve_cnt_type(counter_type,
 						  sizeof(counter_type),
 						  xstats_names[i].name);
-			sprintf(buf, "PUTVAL %s/dpdkstat-port.%u/%s-%s N:%"
+			n = snprintf(buf, MAX_STRING_LEN,
+				"PUTVAL %s/dpdkstat-port.%u/%s-%s N:%"
 				PRIu64"\n", host_id, port_id, counter_type,
 				xstats_names[i].name, values[i]);
-			ret = write(stdout_fd, buf, strlen(buf));
+			if (n > sizeof(buf) - 1)
+				n = sizeof(buf) - 1;
+			ret = write(stdout_fd, buf, n);
 			if (ret < 0)
 				goto err;
 		} else {

[dpdk-dev] [PATCH v5 15/16] app/test-bbdev: strcpy ok for allocated string

[Andy Green]

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: f714a18885a6 ("app/testbbdev: add test application for bbdev")
Cc: stable@dpdk.org
---
 app/test-bbdev/test_bbdev_vector.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test-bbdev/test_bbdev_vector.c b/app/test-bbdev/test_bbdev_vector.c
index a37e35f4d..c574f2135 100644
--- a/app/test-bbdev/test_bbdev_vector.c
+++ b/app/test-bbdev/test_bbdev_vector.c
@@ -892,7 +892,7 @@ test_bbdev_vector_read(const char *filename,
 		}
 
 		memset(entry, 0, strlen(line) + 1);
-		strncpy(entry, line, strlen(line));
+		strcpy(entry, line);
 
 		/* check if entry ends with , or = */
 		if (entry[strlen(entry) - 1] == ','

[dpdk-dev] [PATCH v5 16/16] app/test-bbdev: strcpy ok for allocated string

[Andy Green]

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: f714a18885a6 ("app/testbbdev: add test application for bbdev")
Cc: stable@dpdk.org
---
 app/test-bbdev/test_bbdev_vector.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-bbdev/test_bbdev_vector.c b/app/test-bbdev/test_bbdev_vector.c
index c574f2135..81b8ee78f 100644
--- a/app/test-bbdev/test_bbdev_vector.c
+++ b/app/test-bbdev/test_bbdev_vector.c
@@ -891,7 +891,6 @@ test_bbdev_vector_read(const char *filename,
 			goto exit;
 		}
 
-		memset(entry, 0, strlen(line) + 1);
 		strcpy(entry, line);
 
 		/* check if entry ends with , or = */
@@ -914,7 +913,8 @@ test_bbdev_vector_read(const char *filename,
 				}
 
 				entry = entry_extended;
-				strncat(entry, line, strlen(line));
+				/* entry has been allocated accordingly */
+				strcpy(&entry[strlen(entry)], line);
 
 				if (entry[strlen(entry) - 1] != ',')
 					break;

[dpdk-dev] [PATCH v6 00/16] Fix default build on gcc8.0.1

[Andy Green]

This series allows dpdk master to build on Fedora 28,
with the x86_64 default config.

Acked-by: Neil Horman <nhorman@tuxdriver.com>
---

Andy Green (16):
      devtools/check-git: provide more generic grep pattern
      net/nfp: solve buffer overflow
      bus/pci: replace strncpy dangerous code
      bus/dpaa: solve inconsistent struct alignment
      net/axgbe: solve broken eeprom string comp
      net/nfp/nfpcore: solve strncpy misuse
      net/nfp/nfpcore: off-by-one and no NUL on strncpy use
      net/nfp: don't memcpy out of source range
      net/qede: strncpy length constant and NUL
      net/qede: solve broken strncpy
      net/sfc: make sure that copied stats name is NUL-terminated
      net/vdev_netvsc: readlink inputs cannot be aliased
      net/vdev_netvsc: convert snprintf to strlcpy
      app/proc-info: sprintf overrun bug
      app/test-bbdev: strcpy ok for allocated string
      app/test-bbdev: strcpy ok for allocated string


 app/proc-info/main.c                       |    8 ++++++--
 app/test-bbdev/test_bbdev_vector.c         |    6 +++---
 devtools/check-git-log.sh                  |    4 ++--
 drivers/bus/dpaa/base/qbman/qman.c         |   14 +++++++-------
 drivers/bus/dpaa/include/fsl_qman.h        |   24 +++++++++++++-----------
 drivers/bus/pci/linux/pci.c                |    2 +-
 drivers/net/axgbe/axgbe_phy_impl.c         |    4 ++--
 drivers/net/nfp/nfp_net.c                  |    6 +++---
 drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c |    4 +++-
 drivers/net/nfp/nfpcore/nfp_resource.c     |   10 ++++++----
 drivers/net/qede/base/ecore_int.c          |    8 +++++---
 drivers/net/qede/qede_main.c               |    7 ++++---
 drivers/net/sfc/sfc_ethdev.c               |    6 +++---
 drivers/net/vdev_netvsc/vdev_netvsc.c      |   16 ++++++++++------
 14 files changed, 68 insertions(+), 51 deletions(-)

--
Signature

[dpdk-dev] [PATCH v6 01/16] devtools/check-git: provide more generic grep pattern

[Andy Green]

On Fedora 28, every patch is faulted for
"Wrong headline uppercase", because [A-Z] is not
always case sensitive.

Change to use [[:upper:]]

Signed-off-by: Andy Green <andy@warmcat.com>
---
 devtools/check-git-log.sh |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/devtools/check-git-log.sh b/devtools/check-git-log.sh
index c601f6ae9..2542d9ee0 100755
--- a/devtools/check-git-log.sh
+++ b/devtools/check-git-log.sh
@@ -106,8 +106,8 @@ bad=$(echo "$headlines" | grep --color=always \
 
 # check headline lowercase for first words
 bad=$(echo "$headlines" | grep --color=always \
-	-e '^.*[A-Z].*:' \
-	-e ': *[A-Z]' \
+	-e '^.*[[:upper:]].*:' \
+	-e ': *[[:upper:]]' \
 	| sed 's,^,\t,')
 [ -z "$bad" ] || printf "Wrong headline uppercase:\n$bad\n"
 

Re: [dpdk-dev] [PATCH v6 01/16] devtools/check-git: provide more generic grep pattern

[Ferruh Yigit]

On 5/14/2018 5:59 AM, Andy Green wrote:
> On Fedora 28, every patch is faulted for
> "Wrong headline uppercase", because [A-Z] is not
> always case sensitive.
> 
> Change to use [[:upper:]]
> 
> Signed-off-by: Andy Green <andy@warmcat.com>

Tested-by: Ferruh Yigit <ferruh.yigit@intel.com>

[dpdk-dev] [PATCH v6 02/16] net/nfp: solve buffer overflow

[Andy Green]

/home/agreen/projects/dpdk/drivers/net/nfp/nfp_net.c: In
function ‘nfp_pf_pci_probe’:
/home/agreen/projects/dpdk/drivers/net/nfp/nfp_net.c:3160:
23: error: ‘%s’ directive writing up to 99 bytes into a
region of size 76 [-Werror=format-overflow=]
  sprintf(fw_name, "%s/%s.nffw", DEFAULT_FW_PATH, serial);

Note fw_buf still has to increase somewhat even after
restricting serial[], since otherwise:

/home/agreen/projects/dpdk/drivers/net/nfp/nfp_net.c: In
function ‘nfp_pf_pci_probe’:
/home/agreen/projects/dpdk/drivers/net/nfp/nfp_net.c:3176:23:
error: ‘%s’ directive writing up to 99 bytes into a region
of size 76 [-Werror=format-overflow=]
  sprintf(fw_name, "%s/%s", DEFAULT_FW_PATH, card);
                       ^~
/home/agreen/projects/dpdk/drivers/net/nfp/nfp_net.c:3262:32:
  err = nfp_fw_upload(dev, nsp, card_desc);
                                ~~~~~~~~~
/home/agreen/projects/dpdk/drivers/net/nfp/nfp_net.c:3176:2:
note: ‘sprintf’ output between 25 and 124 bytes into a
destination of size 100
  sprintf(fw_name, "%s/%s", DEFAULT_FW_PATH, card);

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: 896c265ef954 ("net/nfp: use new CPP interface")
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/net/nfp/nfp_net.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 048324ec9..78113b41b 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -3144,8 +3144,8 @@ nfp_fw_upload(struct rte_pci_device *dev, struct nfp_nsp *nsp, char *card)
 	struct nfp_cpp *cpp = nsp->cpp;
 	int fw_f;
 	char *fw_buf;
-	char fw_name[100];
-	char serial[100];
+	char fw_name[125];
+	char serial[40];
 	struct stat file_stat;
 	off_t fsize, bytes;
 

[dpdk-dev] [PATCH v6 03/16] bus/pci: replace strncpy dangerous code

[Andy Green]

In function ‘pci_get_kernel_driver_by_path’,
    inlined from ‘pci_scan_one.isra.1’ at /home/agreen/projects/dpdk/
	drivers/bus/pci/linux/pci.c:317:8:
/home/agreen/projects/dpdk/drivers/bus/pci/linux/pci.c:57:3: error:
‘strncpy’ specified bound depends on the length of the source argument
[-Werror=stringop-overflow=]
   strncpy(dri_name, name + 1, strlen(name + 1) + 1);

Signed-off-by: Andy Green <andy@warmcat.com>
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
Fixes: d9a8cd9595f2 ("pci: add kernel driver type")
Cc: stable@dpdk.org
---
 drivers/bus/pci/linux/pci.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 4630a8057..a73ee49c2 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -54,7 +54,7 @@ pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
 
 	name = strrchr(path, '/');
 	if (name) {
-		strncpy(dri_name, name + 1, strlen(name + 1) + 1);
+		strlcpy(dri_name, name + 1, sizeof(dri_name));
 		return 0;
 	}
 

[dpdk-dev] [PATCH v6 04/16] bus/dpaa: solve inconsistent struct alignment

[Andy Green]

The actual descriptor for qm_mr_entry is 64-byte aligned.

But the original code plays a trick, and puts a u8 common
to the three descriptor subtypes in the union afterwards
outside their structure definitions.

Unfortunately since they compose a struct qm_fd with
alignment 8, this trick destroys the ability of the compiler
to understand what has happened, resulting in this kind of
problem:

/home/agreen/projects/dpdk/drivers/bus/dpaa/include/
fsl_qman.h:354:3: error: alignment 1 of ‘struct <anonymous>’
is less than 8 [-Werror=packed-not-aligned]
   } __packed dcern;

on gcc 8 / Fedora 28 out of the box.

This patch moves the u8 verb into the structure definitions
composed into the union, so the alignment of the parent struct
containing the alignment 8 object can also be seen to be
alignment 8 by the compiler.  Uses of .verb are fixed up to use
.ern.verb (the same offset of +0 inside all the structs in
the union).

The final struct layout should be unchanged.

Signed-off-by: Andy Green <andy@warmcat.com>
Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Tested-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Fixes: c47ff048b99a ("bus/dpaa: add QMAN driver core routines")
Fixes: f6fadc3e6310 ("bus/dpaa: add QMAN interface driver")
Cc: stable@dpdk.org
---
 drivers/bus/dpaa/base/qbman/qman.c  |   14 +++++++-------
 drivers/bus/dpaa/include/fsl_qman.h |   24 +++++++++++++-----------
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/drivers/bus/dpaa/base/qbman/qman.c b/drivers/bus/dpaa/base/qbman/qman.c
index 2810fdd26..27d98cc10 100644
--- a/drivers/bus/dpaa/base/qbman/qman.c
+++ b/drivers/bus/dpaa/base/qbman/qman.c
@@ -314,9 +314,9 @@ static int drain_mr_fqrni(struct qm_portal *p)
 		if (!msg)
 			return 0;
 	}
-	if ((msg->verb & QM_MR_VERB_TYPE_MASK) != QM_MR_VERB_FQRNI) {
+	if ((msg->ern.verb & QM_MR_VERB_TYPE_MASK) != QM_MR_VERB_FQRNI) {
 		/* We aren't draining anything but FQRNIs */
-		pr_err("Found verb 0x%x in MR\n", msg->verb);
+		pr_err("Found verb 0x%x in MR\n", msg->ern.verb);
 		return -1;
 	}
 	qm_mr_next(p);
@@ -483,7 +483,7 @@ static inline void qm_mr_pvb_update(struct qm_portal *portal)
 	/* when accessing 'verb', use __raw_readb() to ensure that compiler
 	 * inlining doesn't try to optimise out "excess reads".
 	 */
-	if ((__raw_readb(&res->verb) & QM_MR_VERB_VBIT) == mr->vbit) {
+	if ((__raw_readb(&res->ern.verb) & QM_MR_VERB_VBIT) == mr->vbit) {
 		mr->pi = (mr->pi + 1) & (QM_MR_SIZE - 1);
 		if (!mr->pi)
 			mr->vbit ^= QM_MR_VERB_VBIT;
@@ -832,7 +832,7 @@ static u32 __poll_portal_slow(struct qman_portal *p, u32 is)
 			goto mr_done;
 		swapped_msg = *msg;
 		hw_fd_to_cpu(&swapped_msg.ern.fd);
-		verb = msg->verb & QM_MR_VERB_TYPE_MASK;
+		verb = msg->ern.verb & QM_MR_VERB_TYPE_MASK;
 		/* The message is a software ERN iff the 0x20 bit is set */
 		if (verb & 0x20) {
 			switch (verb) {
@@ -1666,7 +1666,7 @@ int qman_retire_fq(struct qman_fq *fq, u32 *flags)
 			 */
 			struct qm_mr_entry msg;
 
-			msg.verb = QM_MR_VERB_FQRNI;
+			msg.ern.verb = QM_MR_VERB_FQRNI;
 			msg.fq.fqs = mcr->alterfq.fqs;
 			msg.fq.fqid = fq->fqid;
 #ifdef CONFIG_FSL_QMAN_FQ_LOOKUP
@@ -2643,7 +2643,7 @@ int qman_shutdown_fq(u32 fqid)
 				qm_mr_pvb_update(low_p);
 				msg = qm_mr_current(low_p);
 				while (msg) {
-					if ((msg->verb &
+					if ((msg->ern.verb &
 					     QM_MR_VERB_TYPE_MASK)
 					    == QM_MR_VERB_FQRN)
 						found_fqrn = 1;
@@ -2711,7 +2711,7 @@ int qman_shutdown_fq(u32 fqid)
 			qm_mr_pvb_update(low_p);
 			msg = qm_mr_current(low_p);
 			while (msg) {
-				if ((msg->verb & QM_MR_VERB_TYPE_MASK) ==
+				if ((msg->ern.verb & QM_MR_VERB_TYPE_MASK) ==
 				    QM_MR_VERB_FQRL)
 					orl_empty = 1;
 				qm_mr_next(low_p);
diff --git a/drivers/bus/dpaa/include/fsl_qman.h b/drivers/bus/dpaa/include/fsl_qman.h
index e9793f30d..e4ad7ae48 100644
--- a/drivers/bus/dpaa/include/fsl_qman.h
+++ b/drivers/bus/dpaa/include/fsl_qman.h
@@ -284,20 +284,20 @@ static inline dma_addr_t qm_sg_addr(const struct qm_sg_entry *sg)
 	} while (0)
 
 /* See 1.5.8.1: "Enqueue Command" */
-struct qm_eqcr_entry {
+struct __rte_aligned(8) qm_eqcr_entry {
 	u8 __dont_write_directly__verb;
 	u8 dca;
 	u16 seqnum;
 	u32 orp;	/* 24-bit */
 	u32 fqid;	/* 24-bit */
 	u32 tag;
-	struct qm_fd fd;
+	struct qm_fd fd; /* this has alignment 8 */
 	u8 __reserved3[32];
 } __packed;
 
 
 /* "Frame Dequeue Response" */
-struct qm_dqrr_entry {
+struct __rte_aligned(8) qm_dqrr_entry {
 	u8 verb;
 	u8 stat;
 	u16 seqnum;	/* 15-bit */
@@ -305,7 +305,7 @@ struct qm_dqrr_entry {
 	u8 __reserved2[3];
 	u32 fqid;	/* 24-bit */
 	u32 contextB;
-	struct qm_fd fd;
+	struct qm_fd fd; /* this has alignment 8 */
 	u8 __reserved4[32];
 };
 
@@ -323,18 +323,19 @@ struct qm_dqrr_entry {
 /* "ERN Message Response" */
 /* "FQ State Change Notification" */
 struct qm_mr_entry {
-	u8 verb;
 	union {
 		struct {
+			u8 verb;
 			u8 dca;
 			u16 seqnum;
 			u8 rc;		/* Rejection Code */
 			u32 orp:24;
 			u32 fqid;	/* 24-bit */
 			u32 tag;
-			struct qm_fd fd;
-		} __packed ern;
+			struct qm_fd fd; /* this has alignment 8 */
+		} __packed __rte_aligned(8) ern;
 		struct {
+			u8 verb;
 #if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
 			u8 colour:2;	/* See QM_MR_DCERN_COLOUR_* */
 			u8 __reserved1:4;
@@ -349,18 +350,19 @@ struct qm_mr_entry {
 			u32 __reserved3:24;
 			u32 fqid;	/* 24-bit */
 			u32 tag;
-			struct qm_fd fd;
-		} __packed dcern;
+			struct qm_fd fd; /* this has alignment 8 */
+		} __packed __rte_aligned(8) dcern;
 		struct {
+			u8 verb;
 			u8 fqs;		/* Frame Queue Status */
 			u8 __reserved1[6];
 			u32 fqid;	/* 24-bit */
 			u32 contextB;
 			u8 __reserved2[16];
-		} __packed fq;		/* FQRN/FQRNI/FQRL/FQPN */
+		} __packed __rte_aligned(8) fq;	/* FQRN/FQRNI/FQRL/FQPN */
 	};
 	u8 __reserved2[32];
-} __packed;
+} __packed __rte_aligned(8);
 #define QM_MR_VERB_VBIT			0x80
 /*
  * ERNs originating from direct-connect portals ("dcern") use 0x20 as a verb

[dpdk-dev] [PATCH v6 05/16] net/axgbe: solve broken eeprom string comp

[Andy Green]

/home/agreen/projects/dpdk/drivers/net/axgbe/axgbe_phy_impl.c:576:6:
error: ‘__builtin_memcmp_eq’ reading 16 bytes from a region of
size 9 [-Werror=stringop-overflow=]
  if (memcmp(&sfp_eeprom->base[AXGBE_SFP_BASE_VENDOR_NAME],
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      AXGBE_BEL_FUSE_VENDOR, AXGBE_SFP_BASE_VENDOR_NAME_LEN))

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: a5c7273771e8 ("net/axgbe: add phy programming APIs")
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/net/axgbe/axgbe_phy_impl.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_phy_impl.c b/drivers/net/axgbe/axgbe_phy_impl.c
index dfa908dd8..973177f69 100644
--- a/drivers/net/axgbe/axgbe_phy_impl.c
+++ b/drivers/net/axgbe/axgbe_phy_impl.c
@@ -574,11 +574,11 @@ static bool axgbe_phy_belfuse_parse_quirks(struct axgbe_port *pdata)
 	struct axgbe_sfp_eeprom *sfp_eeprom = &phy_data->sfp_eeprom;
 
 	if (memcmp(&sfp_eeprom->base[AXGBE_SFP_BASE_VENDOR_NAME],
-		   AXGBE_BEL_FUSE_VENDOR, AXGBE_SFP_BASE_VENDOR_NAME_LEN))
+		   AXGBE_BEL_FUSE_VENDOR, strlen(AXGBE_BEL_FUSE_VENDOR)))
 		return false;
 
 	if (!memcmp(&sfp_eeprom->base[AXGBE_SFP_BASE_VENDOR_PN],
-		    AXGBE_BEL_FUSE_PARTNO, AXGBE_SFP_BASE_VENDOR_PN_LEN)) {
+		    AXGBE_BEL_FUSE_PARTNO, strlen(AXGBE_BEL_FUSE_PARTNO))) {
 		phy_data->sfp_base = AXGBE_SFP_BASE_1000_SX;
 		phy_data->sfp_cable = AXGBE_SFP_CABLE_ACTIVE;
 		phy_data->sfp_speed = AXGBE_SFP_SPEED_1000;

[dpdk-dev] [PATCH v6 06/16] net/nfp/nfpcore: solve strncpy misuse

[Andy Green]

Signed-off-by: Andy Green <andy@warmcat.com>
Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Tested-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Fixes: c7e9729da6b5 ("net/nfp: support CPP")
---
 drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
index 4e6c66624..52b294888 100644
--- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
+++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
@@ -31,6 +31,8 @@
 #include <sys/file.h>
 #include <sys/stat.h>
 
+#include <rte_string_fns.h>
+
 #include "nfp_cpp.h"
 #include "nfp_target.h"
 #include "nfp6000/nfp6000.h"
@@ -846,7 +848,7 @@ nfp6000_init(struct nfp_cpp *cpp, const char *devname)
 
 
 	memset(desc->busdev, 0, BUSDEV_SZ);
-	strncpy(desc->busdev, devname, strlen(devname));
+	strlcpy(desc->busdev, devname, sizeof(desc->busdev));
 
 	ret = nfp_acquire_process_lock(desc);
 	if (ret)

[dpdk-dev] [PATCH v6 07/16] net/nfp/nfpcore: off-by-one and no NUL on strncpy use

[Andy Green]

/home/agreen/projects/dpdk/drivers/net/nfp/nfpcore/nfp_resource.c:
76:2:error: ‘strncpy’ output may be truncated copying 8 bytes from
a string of length 8 [-Werror=stringop-truncation]
  strncpy(name_pad, res->name, sizeof(name_pad));

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: c7e9729da6b5 ("net/nfp: support CPP")
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/net/nfp/nfpcore/nfp_resource.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/nfp/nfpcore/nfp_resource.c b/drivers/net/nfp/nfpcore/nfp_resource.c
index e1df2b2e1..dd41fa4de 100644
--- a/drivers/net/nfp/nfpcore/nfp_resource.c
+++ b/drivers/net/nfp/nfpcore/nfp_resource.c
@@ -7,6 +7,8 @@
 #include <time.h>
 #include <endian.h>
 
+#include <rte_string_fns.h>
+
 #include "nfp_cpp.h"
 #include "nfp6000/nfp6000.h"
 #include "nfp_resource.h"
@@ -65,22 +67,22 @@ struct nfp_resource {
 static int
 nfp_cpp_resource_find(struct nfp_cpp *cpp, struct nfp_resource *res)
 {
-	char name_pad[NFP_RESOURCE_ENTRY_NAME_SZ] = {};
+	char name_pad[NFP_RESOURCE_ENTRY_NAME_SZ + 2];
 	struct nfp_resource_entry entry;
 	uint32_t cpp_id, key;
 	int ret, i;
 
 	cpp_id = NFP_CPP_ID(NFP_RESOURCE_TBL_TARGET, 3, 0);  /* Atomic read */
 
-	memset(name_pad, 0, NFP_RESOURCE_ENTRY_NAME_SZ);
-	strncpy(name_pad, res->name, sizeof(name_pad));
+	memset(name_pad, 0, sizeof(name_pad));
+	strlcpy(name_pad, res->name, sizeof(name_pad));
 
 	/* Search for a matching entry */
 	if (!memcmp(name_pad, NFP_RESOURCE_TBL_NAME "\0\0\0\0\0\0\0\0", 8)) {
 		printf("Grabbing device lock not supported\n");
 		return -EOPNOTSUPP;
 	}
-	key = nfp_crc32_posix(name_pad, sizeof(name_pad));
+	key = nfp_crc32_posix(name_pad, NFP_RESOURCE_ENTRY_NAME_SZ);
 
 	for (i = 0; i < NFP_RESOURCE_TBL_ENTRIES; i++) {
 		uint64_t addr = NFP_RESOURCE_TBL_BASE +

[dpdk-dev] [PATCH v6 08/16] net/nfp: don't memcpy out of source range

[Andy Green]

/home/agreen/projects/dpdk/drivers/net/nfp/nfp_net.c:669:2:
error: ‘memcpy’ forming offset [5, 6] is out of the bounds
[0, 4] of object ‘tmp’ with type ‘uint32_t’ {aka ‘unsigned
int’} [-Werror=array-bounds]  memcpy(&hw->mac_addr[0],
&tmp, sizeof(struct ether_addr));

Signed-off-by: Andy Green <andy@warmcat.com>
Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Tested-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Fixes: e6decee38209 ("net/nfp: use random MAC address if not configured")
Cc: stable@dpdk.org
---
 drivers/net/nfp/nfp_net.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index 78113b41b..f114b1839 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -666,7 +666,7 @@ nfp_net_vf_read_mac(struct nfp_net_hw *hw)
 	uint32_t tmp;
 
 	tmp = rte_be_to_cpu_32(nn_cfg_readl(hw, NFP_NET_CFG_MACADDR));
-	memcpy(&hw->mac_addr[0], &tmp, sizeof(struct ether_addr));
+	memcpy(&hw->mac_addr[0], &tmp, 4);
 
 	tmp = rte_be_to_cpu_32(nn_cfg_readl(hw, NFP_NET_CFG_MACADDR + 4));
 	memcpy(&hw->mac_addr[4], &tmp, 2);

[dpdk-dev] [PATCH v6 09/16] net/qede: strncpy length constant and NUL

[Andy Green]

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: 8427c6647964 ("net/qede/base: add attention formatting string")
Cc: stable@dpdk.org
---
 drivers/net/qede/base/ecore_int.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/qede/base/ecore_int.c b/drivers/net/qede/base/ecore_int.c
index f43781ba4..d9e22b5ed 100644
--- a/drivers/net/qede/base/ecore_int.c
+++ b/drivers/net/qede/base/ecore_int.c
@@ -6,6 +6,8 @@
  * See LICENSE.qede_pmd for copyright and licensing details.
  */
 
+#include <rte_string_fns.h>
+
 #include "bcm_osal.h"
 #include "ecore.h"
 #include "ecore_spq.h"
@@ -1104,9 +1106,9 @@ static enum _ecore_status_t ecore_int_deassertion(struct ecore_hwfn *p_hwfn,
 							      p_aeu->bit_name,
 							      num);
 					else
-						OSAL_STRNCPY(bit_name,
-							     p_aeu->bit_name,
-							     30);
+						strlcpy(bit_name,
+							p_aeu->bit_name,
+							sizeof(bit_name));
 
 					/* We now need to pass bitmask in its
 					 * correct position.

Re: [dpdk-dev] [PATCH v6 09/16] net/qede: strncpy length constant and NUL

[Ferruh Yigit]

On 5/14/2018 6:00 AM, Andy Green wrote:

> Fixes: 8427c6647964 ("net/qede/base: add attention formatting string")
> Cc: stable@dpdk.org

> Signed-off-by: Andy Green <andy@warmcat.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

[dpdk-dev] [PATCH v6 10/16] net/qede: solve broken strncpy

[Andy Green]

/home/agreen/projects/dpdk/drivers/net/qede/qede_main.c: In function
‘qed_slowpath_start’:
/home/agreen/projects/dpdk/drivers/net/qede/qede_main.c:307:3:
error: ‘strncpy’ output may be truncated copying 12 bytes from a
string of length 127 [-Werror=stringop-truncation]
   strncpy((char *)drv_version.name, (const char *)params->name,
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    MCP_DRV_VER_STR_SIZE - 4);
    ~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: 86a2265e59d7 ("qede: add SRIOV support")
Cc: stable@dpdk.org
---
 drivers/net/qede/qede_main.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/qede/qede_main.c b/drivers/net/qede/qede_main.c
index 2333ca073..fcfc32d0d 100644
--- a/drivers/net/qede/qede_main.c
+++ b/drivers/net/qede/qede_main.c
@@ -9,6 +9,7 @@
 #include <limits.h>
 #include <time.h>
 #include <rte_alarm.h>
+#include <rte_string_fns.h>
 
 #include "qede_ethdev.h"
 
@@ -303,9 +304,9 @@ static int qed_slowpath_start(struct ecore_dev *edev,
 		drv_version.version = (params->drv_major << 24) |
 		    (params->drv_minor << 16) |
 		    (params->drv_rev << 8) | (params->drv_eng);
-		/* TBD: strlcpy() */
-		strncpy((char *)drv_version.name, (const char *)params->name,
-			MCP_DRV_VER_STR_SIZE - 4);
+		strlcpy((char *)drv_version.name, (const char *)params->name,
+			sizeof(drv_version.name));
+		drv_version.name[sizeof(drv_version.name) - 1] = '\0';
 		rc = ecore_mcp_send_drv_version(hwfn, hwfn->p_main_ptt,
 						&drv_version);
 		if (rc) {

Re: [dpdk-dev] [PATCH v6 10/16] net/qede: solve broken strncpy

[Ferruh Yigit]

On 5/14/2018 6:00 AM, Andy Green wrote:
> /home/agreen/projects/dpdk/drivers/net/qede/qede_main.c: In function
> ‘qed_slowpath_start’:
> /home/agreen/projects/dpdk/drivers/net/qede/qede_main.c:307:3:
> error: ‘strncpy’ output may be truncated copying 12 bytes from a
> string of length 127 [-Werror=stringop-truncation]
>    strncpy((char *)drv_version.name, (const char *)params->name,
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     MCP_DRV_VER_STR_SIZE - 4);
>     ~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Signed-off-by: Andy Green <andy@warmcat.com>
> Fixes: 86a2265e59d7 ("qede: add SRIOV support")
> Cc: stable@dpdk.org
> ---
>  drivers/net/qede/qede_main.c |    7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/qede/qede_main.c b/drivers/net/qede/qede_main.c
> index 2333ca073..fcfc32d0d 100644
> --- a/drivers/net/qede/qede_main.c
> +++ b/drivers/net/qede/qede_main.c
> @@ -9,6 +9,7 @@
>  #include <limits.h>
>  #include <time.h>
>  #include <rte_alarm.h>
> +#include <rte_string_fns.h>
>  
>  #include "qede_ethdev.h"
>  
> @@ -303,9 +304,9 @@ static int qed_slowpath_start(struct ecore_dev *edev,
>  		drv_version.version = (params->drv_major << 24) |
>  		    (params->drv_minor << 16) |
>  		    (params->drv_rev << 8) | (params->drv_eng);
> -		/* TBD: strlcpy() */
> -		strncpy((char *)drv_version.name, (const char *)params->name,
> -			MCP_DRV_VER_STR_SIZE - 4);
> +		strlcpy((char *)drv_version.name, (const char *)params->name,
> +			sizeof(drv_version.name));
> +		drv_version.name[sizeof(drv_version.name) - 1] = '\0';

Why set '\0'? doesn't strlcpy assures it?

>  		rc = ecore_mcp_send_drv_version(hwfn, hwfn->p_main_ptt,
>  						&drv_version);
>  		if (rc) {
> 

Re: [dpdk-dev] [PATCH v6 10/16] net/qede: solve broken strncpy

[Ferruh Yigit]

On 5/14/2018 9:20 PM, Ferruh Yigit wrote:
> On 5/14/2018 6:00 AM, Andy Green wrote:
>> /home/agreen/projects/dpdk/drivers/net/qede/qede_main.c: In function
>> ‘qed_slowpath_start’:
>> /home/agreen/projects/dpdk/drivers/net/qede/qede_main.c:307:3:
>> error: ‘strncpy’ output may be truncated copying 12 bytes from a
>> string of length 127 [-Werror=stringop-truncation]
>>    strncpy((char *)drv_version.name, (const char *)params->name,
>>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>     MCP_DRV_VER_STR_SIZE - 4);
>>     ~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Signed-off-by: Andy Green <andy@warmcat.com>
>> Fixes: 86a2265e59d7 ("qede: add SRIOV support")
>> Cc: stable@dpdk.org
>> ---
>>  drivers/net/qede/qede_main.c |    7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/qede/qede_main.c b/drivers/net/qede/qede_main.c
>> index 2333ca073..fcfc32d0d 100644
>> --- a/drivers/net/qede/qede_main.c
>> +++ b/drivers/net/qede/qede_main.c
>> @@ -9,6 +9,7 @@
>>  #include <limits.h>
>>  #include <time.h>
>>  #include <rte_alarm.h>
>> +#include <rte_string_fns.h>
>>  
>>  #include "qede_ethdev.h"
>>  
>> @@ -303,9 +304,9 @@ static int qed_slowpath_start(struct ecore_dev *edev,
>>  		drv_version.version = (params->drv_major << 24) |
>>  		    (params->drv_minor << 16) |
>>  		    (params->drv_rev << 8) | (params->drv_eng);
>> -		/* TBD: strlcpy() */
>> -		strncpy((char *)drv_version.name, (const char *)params->name,
>> -			MCP_DRV_VER_STR_SIZE - 4);
>> +		strlcpy((char *)drv_version.name, (const char *)params->name,
>> +			sizeof(drv_version.name));
>> +		drv_version.name[sizeof(drv_version.name) - 1] = '\0';
> 
> Why set '\0'? doesn't strlcpy assures it?

Will remove this line while applying, with the change:
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

> 
>>  		rc = ecore_mcp_send_drv_version(hwfn, hwfn->p_main_ptt,
>>  						&drv_version);
>>  		if (rc) {
>>
> 

Re: [dpdk-dev] [PATCH v6 10/16] net/qede: solve broken strncpy

[Andy Green]

On 05/15/2018 05:14 AM, Ferruh Yigit wrote:
> On 5/14/2018 9:20 PM, Ferruh Yigit wrote:
>> On 5/14/2018 6:00 AM, Andy Green wrote:
>>> /home/agreen/projects/dpdk/drivers/net/qede/qede_main.c: In function
>>> ‘qed_slowpath_start’:
>>> /home/agreen/projects/dpdk/drivers/net/qede/qede_main.c:307:3:
>>> error: ‘strncpy’ output may be truncated copying 12 bytes from a
>>> string of length 127 [-Werror=stringop-truncation]
>>>     strncpy((char *)drv_version.name, (const char *)params->name,
>>>     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>      MCP_DRV_VER_STR_SIZE - 4);
>>>      ~~~~~~~~~~~~~~~~~~~~~~~~~
>>>
>>> Signed-off-by: Andy Green <andy@warmcat.com>
>>> Fixes: 86a2265e59d7 ("qede: add SRIOV support")
>>> Cc: stable@dpdk.org
>>> ---
>>>   drivers/net/qede/qede_main.c |    7 ++++---
>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/qede/qede_main.c b/drivers/net/qede/qede_main.c
>>> index 2333ca073..fcfc32d0d 100644
>>> --- a/drivers/net/qede/qede_main.c
>>> +++ b/drivers/net/qede/qede_main.c
>>> @@ -9,6 +9,7 @@
>>>   #include <limits.h>
>>>   #include <time.h>
>>>   #include <rte_alarm.h>
>>> +#include <rte_string_fns.h>
>>>   
>>>   #include "qede_ethdev.h"
>>>   
>>> @@ -303,9 +304,9 @@ static int qed_slowpath_start(struct ecore_dev *edev,
>>>   		drv_version.version = (params->drv_major << 24) |
>>>   		    (params->drv_minor << 16) |
>>>   		    (params->drv_rev << 8) | (params->drv_eng);
>>> -		/* TBD: strlcpy() */
>>> -		strncpy((char *)drv_version.name, (const char *)params->name,
>>> -			MCP_DRV_VER_STR_SIZE - 4);
>>> +		strlcpy((char *)drv_version.name, (const char *)params->name,
>>> +			sizeof(drv_version.name));
>>> +		drv_version.name[sizeof(drv_version.name) - 1] = '\0';
>>
>> Why set '\0'? doesn't strlcpy assures it?

Yeah... it's a leftover.

> Will remove this line while applying, with the change:
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Thanks.

-Andy

>>
>>>   		rc = ecore_mcp_send_drv_version(hwfn, hwfn->p_main_ptt,
>>>   						&drv_version);
>>>   		if (rc) {
>>>
>>
> 

[dpdk-dev] [PATCH v6 11/16] net/sfc: make sure that copied stats name is NUL-terminated

[Andy Green]

Signed-off-by: Andy Green <andy@warmcat.com>
Acked-by: Andrew Rybchenko <arybchenko@oktetlabs.ru>
Fixes: 73280c1e4ff2 ("net/sfc: support xstats retrieval by ID")
Fixes: 7b9891769f4b ("net/sfc: support extended statistics")
Cc: stable@dpdk.org
---
 drivers/net/sfc/sfc_ethdev.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index e42d55350..a8c0f8e19 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -13,6 +13,7 @@
 #include <rte_pci.h>
 #include <rte_bus_pci.h>
 #include <rte_errno.h>
+#include <rte_string_fns.h>
 
 #include "efx.h"
 
@@ -663,7 +664,7 @@ sfc_xstats_get_names(struct rte_eth_dev *dev,
 	for (i = 0; i < EFX_MAC_NSTATS; ++i) {
 		if (EFX_MAC_STAT_SUPPORTED(port->mac_stats_mask, i)) {
 			if (xstats_names != NULL && nstats < xstats_count)
-				strncpy(xstats_names[nstats].name,
+				strlcpy(xstats_names[nstats].name,
 					efx_mac_stat_name(sa->nic, i),
 					sizeof(xstats_names[0].name));
 			nstats++;
@@ -741,9 +742,8 @@ sfc_xstats_get_names_by_id(struct rte_eth_dev *dev,
 		if ((ids == NULL) || (ids[nb_written] == nb_supported)) {
 			char *name = xstats_names[nb_written++].name;
 
-			strncpy(name, efx_mac_stat_name(sa->nic, i),
+			strlcpy(name, efx_mac_stat_name(sa->nic, i),
 				sizeof(xstats_names[0].name));
-			name[sizeof(xstats_names[0].name) - 1] = '\0';
 		}
 
 		++nb_supported;

[dpdk-dev] [PATCH v6 12/16] net/vdev_netvsc: readlink inputs cannot be aliased

[Andy Green]

/home/agreen/projects/dpdk/drivers/net/vdev_netvsc/
vdev_netvsc.c:335:2:error: passing argument 2 to restrict-
qualified parameter aliases with argument 1 [-Werror=restrict]
  ret = readlink(buf, buf, size);
  ^~~

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: e7dc5d7becc5 ("net/vdev_netvsc: implement core functionality")
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/net/vdev_netvsc/vdev_netvsc.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c b/drivers/net/vdev_netvsc/vdev_netvsc.c
index c321a9f1b..ac26e0a3e 100644
--- a/drivers/net/vdev_netvsc/vdev_netvsc.c
+++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
@@ -327,12 +327,15 @@ static int
 vdev_netvsc_sysfs_readlink(char *buf, size_t size, const char *if_name,
 			   const char *relpath)
 {
+	struct vdev_netvsc_ctx *ctx;
+	char in[RTE_MAX(sizeof(ctx->yield), 256u)];
 	int ret;
 
-	ret = snprintf(buf, size, "/sys/class/net/%s/%s", if_name, relpath);
-	if (ret == -1 || (size_t)ret >= size)
+	ret = snprintf(in, sizeof(in) - 1, "/sys/class/net/%s/%s",
+		       if_name, relpath);
+	if (ret == -1 || (size_t)ret >= sizeof(in))
 		return -ENOBUFS;
-	ret = readlink(buf, buf, size);
+	ret = readlink(in, buf, size);
 	if (ret == -1)
 		return -errno;
 	if ((size_t)ret >= size - 1)

[dpdk-dev] [PATCH v6 13/16] net/vdev_netvsc: convert snprintf to strlcpy

[Andy Green]

Continue snprintf to strlcpy conversions started by commit
c022cb400e92 ("convert snprintf to strlcpy").

Signed-off-by: Andy Green <andy@warmcat.com>
Cc: stable@dpdk.org
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/net/vdev_netvsc/vdev_netvsc.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c b/drivers/net/vdev_netvsc/vdev_netvsc.c
index ac26e0a3e..de2bd1479 100644
--- a/drivers/net/vdev_netvsc/vdev_netvsc.c
+++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
@@ -35,6 +35,7 @@
 #include <rte_hypervisor.h>
 #include <rte_kvargs.h>
 #include <rte_log.h>
+#include <rte_string_fns.h>
 
 #define VDEV_NETVSC_DRIVER net_vdev_netvsc
 #define VDEV_NETVSC_DRIVER_NAME RTE_STR(VDEV_NETVSC_DRIVER)
@@ -182,7 +183,7 @@ vdev_netvsc_foreach_iface(int (*func)(const struct if_nameindex *iface,
 		is_netvsc_ret = vdev_netvsc_iface_is_netvsc(&iface[i]) ? 1 : 0;
 		if (is_netvsc ^ is_netvsc_ret)
 			continue;
-		strncpy(req.ifr_name, iface[i].if_name, sizeof(req.ifr_name));
+		strlcpy(req.ifr_name, iface[i].if_name, sizeof(req.ifr_name));
 		if (ioctl(s, SIOCGIFHWADDR, &req) == -1) {
 			DRV_LOG(WARNING, "cannot retrieve information about"
 					 " interface \"%s\": %s",
@@ -385,7 +386,7 @@ vdev_netvsc_device_probe(const struct if_nameindex *iface,
 		DRV_LOG(DEBUG,
 			"NetVSC interface \"%s\" (index %u) renamed \"%s\"",
 			ctx->if_name, ctx->if_index, iface->if_name);
-		strncpy(ctx->if_name, iface->if_name, sizeof(ctx->if_name));
+		strlcpy(ctx->if_name, iface->if_name, sizeof(ctx->if_name));
 		return 0;
 	}
 	if (!is_same_ether_addr(eth_addr, &ctx->if_addr))
@@ -583,7 +584,7 @@ vdev_netvsc_netvsc_probe(const struct if_nameindex *iface,
 		goto error;
 	}
 	ctx->id = vdev_netvsc_ctx_count;
-	strncpy(ctx->if_name, iface->if_name, sizeof(ctx->if_name));
+	strlcpy(ctx->if_name, iface->if_name, sizeof(ctx->if_name));
 	ctx->if_index = iface->if_index;
 	ctx->if_addr = *eth_addr;
 	ctx->pipe[0] = -1;

[dpdk-dev] [PATCH v6 14/16] app/proc-info: sprintf overrun bug

[Andy Green]

/home/agreen/projects/dpdk/app/proc-info/main.c: In function
‘nic_xstats_display’:
/home/agreen/projects/dpdk/app/proc-info/main.c:495:45:
error: ‘%s’ directive writing up to 255 bytes into a region
of size between 165 and 232 [-Werror=format-overflow=]
    sprintf(buf, "PUTVAL %s/dpdkstat-port.%u/%s-%s N:%"
                                             ^~
     PRIu64"\n", host_id, port_id, counter_type,
                                   ~~~~~~~~~~~~
/home/agreen/projects/dpdk/app/proc-info/main.c:495:4: note:
‘sprintf’ output between 31 and 435 bytes into a destination
of size 256
    sprintf(buf, "PUTVAL %s/dpdkstat-port.%u/%s-%s N:%"
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     PRIu64"\n", host_id, port_id, counter_type,
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     xstats_names[i].name, values[i]);

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: 2deb6b5246d7 ("app/procinfo: add collectd format and host id")
Cc: stable@dpdk.org
---
 app/proc-info/main.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 539e13243..c20effa4f 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -488,14 +488,18 @@ nic_xstats_display(uint16_t port_id)
 		if (enable_collectd_format) {
 			char counter_type[MAX_STRING_LEN];
 			char buf[MAX_STRING_LEN];
+			size_t n;
 
 			collectd_resolve_cnt_type(counter_type,
 						  sizeof(counter_type),
 						  xstats_names[i].name);
-			sprintf(buf, "PUTVAL %s/dpdkstat-port.%u/%s-%s N:%"
+			n = snprintf(buf, MAX_STRING_LEN,
+				"PUTVAL %s/dpdkstat-port.%u/%s-%s N:%"
 				PRIu64"\n", host_id, port_id, counter_type,
 				xstats_names[i].name, values[i]);
-			ret = write(stdout_fd, buf, strlen(buf));
+			if (n > sizeof(buf) - 1)
+				n = sizeof(buf) - 1;
+			ret = write(stdout_fd, buf, n);
 			if (ret < 0)
 				goto err;
 		} else {

Re: [dpdk-dev] [PATCH v6 14/16] app/proc-info: sprintf overrun bug

[Ferruh Yigit]

On 5/14/2018 6:01 AM, Andy Green wrote:
> /home/agreen/projects/dpdk/app/proc-info/main.c: In function
> ‘nic_xstats_display’:
> /home/agreen/projects/dpdk/app/proc-info/main.c:495:45:
> error: ‘%s’ directive writing up to 255 bytes into a region
> of size between 165 and 232 [-Werror=format-overflow=]
>     sprintf(buf, "PUTVAL %s/dpdkstat-port.%u/%s-%s N:%"
>                                              ^~
>      PRIu64"\n", host_id, port_id, counter_type,
>                                    ~~~~~~~~~~~~
> /home/agreen/projects/dpdk/app/proc-info/main.c:495:4: note:
> ‘sprintf’ output between 31 and 435 bytes into a destination
> of size 256
>     sprintf(buf, "PUTVAL %s/dpdkstat-port.%u/%s-%s N:%"
>     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>      PRIu64"\n", host_id, port_id, counter_type,
>      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>      xstats_names[i].name, values[i]);
> 
> Signed-off-by: Andy Green <andy@warmcat.com>
> Fixes: 2deb6b5246d7 ("app/procinfo: add collectd format and host id")
> Cc: stable@dpdk.org

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

[dpdk-dev] [PATCH v6 15/16] app/test-bbdev: strcpy ok for allocated string

[Andy Green]

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: f714a18885a6 ("app/testbbdev: add test application for bbdev")
Cc: stable@dpdk.org
---
 app/test-bbdev/test_bbdev_vector.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/test-bbdev/test_bbdev_vector.c b/app/test-bbdev/test_bbdev_vector.c
index a37e35f4d..c574f2135 100644
--- a/app/test-bbdev/test_bbdev_vector.c
+++ b/app/test-bbdev/test_bbdev_vector.c
@@ -892,7 +892,7 @@ test_bbdev_vector_read(const char *filename,
 		}
 
 		memset(entry, 0, strlen(line) + 1);
-		strncpy(entry, line, strlen(line));
+		strcpy(entry, line);
 
 		/* check if entry ends with , or = */
 		if (entry[strlen(entry) - 1] == ','

Re: [dpdk-dev] [PATCH v6 15/16] app/test-bbdev: strcpy ok for allocated string

[Ferruh Yigit]

On 5/14/2018 6:01 AM, Andy Green wrote:
> Signed-off-by: Andy Green <andy@warmcat.com>
> Fixes: f714a18885a6 ("app/testbbdev: add test application for bbdev")
> Cc: stable@dpdk.org
> ---
>  app/test-bbdev/test_bbdev_vector.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/app/test-bbdev/test_bbdev_vector.c b/app/test-bbdev/test_bbdev_vector.c
> index a37e35f4d..c574f2135 100644
> --- a/app/test-bbdev/test_bbdev_vector.c
> +++ b/app/test-bbdev/test_bbdev_vector.c
> @@ -892,7 +892,7 @@ test_bbdev_vector_read(const char *filename,
>  		}
>  
>  		memset(entry, 0, strlen(line) + 1);
> -		strncpy(entry, line, strlen(line));
> +		strcpy(entry, line);

agreed that according above code strcpy is OK but still why change it? Is this
fixing a build error?

>  
>  		/* check if entry ends with , or = */
>  		if (entry[strlen(entry) - 1] == ','
> 

Re: [dpdk-dev] [PATCH v6 15/16] app/test-bbdev: strcpy ok for allocated string

[Ferruh Yigit]

On 5/14/2018 9:42 PM, Ferruh Yigit wrote:
> On 5/14/2018 6:01 AM, Andy Green wrote:
>> Signed-off-by: Andy Green <andy@warmcat.com>
>> Fixes: f714a18885a6 ("app/testbbdev: add test application for bbdev")
>> Cc: stable@dpdk.org
>> ---
>>  app/test-bbdev/test_bbdev_vector.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/app/test-bbdev/test_bbdev_vector.c b/app/test-bbdev/test_bbdev_vector.c
>> index a37e35f4d..c574f2135 100644
>> --- a/app/test-bbdev/test_bbdev_vector.c
>> +++ b/app/test-bbdev/test_bbdev_vector.c
>> @@ -892,7 +892,7 @@ test_bbdev_vector_read(const char *filename,
>>  		}
>>  
>>  		memset(entry, 0, strlen(line) + 1);
>> -		strncpy(entry, line, strlen(line));
>> +		strcpy(entry, line);
> 
> agreed that according above code strcpy is OK but still why change it? Is this
> fixing a build error?

Yes:

.../app/test-bbdev/test_bbdev_vector.c:895:3: error: ‘strncpy’ output truncated
before terminating nul copying as many bytes from a string as its length
[-Werror=stringop-truncation]
   strncpy(entry, line, strlen(line));
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> 
>>  
>>  		/* check if entry ends with , or = */
>>  		if (entry[strlen(entry) - 1] == ','
>>
> 

Re: [dpdk-dev] [PATCH v6 15/16] app/test-bbdev: strcpy ok for allocated string

[Ferruh Yigit]

On 5/14/2018 9:56 PM, Ferruh Yigit wrote:
> On 5/14/2018 9:42 PM, Ferruh Yigit wrote:
>> On 5/14/2018 6:01 AM, Andy Green wrote:

>>> Fixes: f714a18885a6 ("app/testbbdev: add test application for bbdev")
>>> Cc: stable@dpdk.org

>>> Signed-off-by: Andy Green <andy@warmcat.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>


>>> ---
>>>  app/test-bbdev/test_bbdev_vector.c |    2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/app/test-bbdev/test_bbdev_vector.c b/app/test-bbdev/test_bbdev_vector.c
>>> index a37e35f4d..c574f2135 100644
>>> --- a/app/test-bbdev/test_bbdev_vector.c
>>> +++ b/app/test-bbdev/test_bbdev_vector.c
>>> @@ -892,7 +892,7 @@ test_bbdev_vector_read(const char *filename,
>>>  		}
>>>  
>>>  		memset(entry, 0, strlen(line) + 1);
>>> -		strncpy(entry, line, strlen(line));
>>> +		strcpy(entry, line);
>>
>> agreed that according above code strcpy is OK but still why change it? Is this
>> fixing a build error?
> 
> Yes:
> 
> .../app/test-bbdev/test_bbdev_vector.c:895:3: error: ‘strncpy’ output truncated
> before terminating nul copying as many bytes from a string as its length
> [-Werror=stringop-truncation]
>    strncpy(entry, line, strlen(line));
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
>>
>>>  
>>>  		/* check if entry ends with , or = */
>>>  		if (entry[strlen(entry) - 1] == ','
>>>
>>
> 

[dpdk-dev] [PATCH v6 16/16] app/test-bbdev: strcpy ok for allocated string

[Andy Green]

Signed-off-by: Andy Green <andy@warmcat.com>
Fixes: f714a18885a6 ("app/testbbdev: add test application for bbdev")
Cc: stable@dpdk.org
---
 app/test-bbdev/test_bbdev_vector.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-bbdev/test_bbdev_vector.c b/app/test-bbdev/test_bbdev_vector.c
index c574f2135..81b8ee78f 100644
--- a/app/test-bbdev/test_bbdev_vector.c
+++ b/app/test-bbdev/test_bbdev_vector.c
@@ -891,7 +891,6 @@ test_bbdev_vector_read(const char *filename,
 			goto exit;
 		}
 
-		memset(entry, 0, strlen(line) + 1);
 		strcpy(entry, line);
 
 		/* check if entry ends with , or = */
@@ -914,7 +913,8 @@ test_bbdev_vector_read(const char *filename,
 				}
 
 				entry = entry_extended;
-				strncat(entry, line, strlen(line));
+				/* entry has been allocated accordingly */
+				strcpy(&entry[strlen(entry)], line);
 
 				if (entry[strlen(entry) - 1] != ',')
 					break;

Re: [dpdk-dev] [PATCH v6 16/16] app/test-bbdev: strcpy ok for allocated string

[Ferruh Yigit]

On 5/14/2018 6:01 AM, Andy Green wrote:

.../app/test-bbdev/test_bbdev_vector.c:917:5:
   error: ‘strncat’ output truncated before terminating nul copying as many
bytes from a string as its length [-Werror=stringop-truncation]
     strncat(entry, line, strlen(line));
     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> Fixes: f714a18885a6 ("app/testbbdev: add test application for bbdev")
> Cc: stable@dpdk.org

> Signed-off-by: Andy Green <andy@warmcat.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

> ---
>  app/test-bbdev/test_bbdev_vector.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-bbdev/test_bbdev_vector.c b/app/test-bbdev/test_bbdev_vector.c
> index c574f2135..81b8ee78f 100644
> --- a/app/test-bbdev/test_bbdev_vector.c
> +++ b/app/test-bbdev/test_bbdev_vector.c
> @@ -891,7 +891,6 @@ test_bbdev_vector_read(const char *filename,
>  			goto exit;
>  		}
>  
> -		memset(entry, 0, strlen(line) + 1);

This one seems belong to 15/16, will move there.

>  		strcpy(entry, line);
>  
>  		/* check if entry ends with , or = */
> @@ -914,7 +913,8 @@ test_bbdev_vector_read(const char *filename,
>  				}
>  
>  				entry = entry_extended;
> -				strncat(entry, line, strlen(line));
> +				/* entry has been allocated accordingly */
> +				strcpy(&entry[strlen(entry)], line);
>  
>  				if (entry[strlen(entry) - 1] != ',')
>  					break;
> 

Re: [dpdk-dev] [PATCH v6 00/16] Fix default build on gcc8.0.1

[Ferruh Yigit]

On 5/14/2018 5:59 AM, Andy Green wrote:
> This series allows dpdk master to build on Fedora 28,
> with the x86_64 default config.
> 
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> ---
> 
> Andy Green (16):
>       devtools/check-git: provide more generic grep pattern
>       net/nfp: solve buffer overflow
>       bus/pci: replace strncpy dangerous code
>       bus/dpaa: solve inconsistent struct alignment
>       net/axgbe: solve broken eeprom string comp
>       net/nfp/nfpcore: solve strncpy misuse
>       net/nfp/nfpcore: off-by-one and no NUL on strncpy use
>       net/nfp: don't memcpy out of source range
>       net/qede: strncpy length constant and NUL
>       net/qede: solve broken strncpy
>       net/sfc: make sure that copied stats name is NUL-terminated
>       net/vdev_netvsc: readlink inputs cannot be aliased
>       net/vdev_netvsc: convert snprintf to strlcpy
>       app/proc-info: sprintf overrun bug
>       app/test-bbdev: strcpy ok for allocated string
>       app/test-bbdev: strcpy ok for allocated string

Series applied to dpdk-next-net/master, thanks.

Thanks for the fixes, a few titles changes and a few change done mentioned in
comments to individual patches.