DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs
@ 2018-05-08  4:29 Andy Green
  2018-05-08  4:29 ` [dpdk-dev] [PATCH 01/18] lib: ret_table: workaround hash function cast error Andy Green
                   ` (17 more replies)
  0 siblings, 18 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:29 UTC (permalink / raw)
  To: dev

The following series gets current master able to build
on Fedora 28 + x86_64 using gcc 8.0.1.  There were a
number of build-breaking problems, mostly around correct
string processing but all of them reflecting some issue
in the code.

Of the two major issues, one around struct alignment in
dpaa I think is properly fixed by the patch 3/18 here,
but for the apparently broken hash function cast in 1/18
I just stopped it breaking the build; unless I miss the
trick it seems it needs fixing properly after some
further discussion.

Because of the kind of widespread string processing /
off-by-one / buffer overflow issue fixed here, I think it'd
be a really good idea to run this stuff through Coverity.

They will give you a free account for OSS projects here

https://scan.coverity.com/

---

Andy Green (18):
      lib: ret_table: workaround hash function cast error
      drivers: bus: pci: fix strncpy dangerous code
      drivers: bus: dpaa: fix inconsistent struct alignment
      drivers: net: axgbe: fix broken eeprom string comp
      drivers: net: nfp: nfpcore: fix strncpy misuse
      drivers: net: nfp: nfpcore fix off-by-one and no NUL on strncpy use
      drivers: net: nfp: don't memcpy out of source range
      drivers: net: nfp: fix buffer overflow in fw_name
      drivers: net: qede: fix strncpy constant and NUL
      drivers: net: qede: fix broken strncpy
      drivers:net:sfc: fix strncpy length
      drivers: net: sfc: fix another strncpy size and NUL
      drivers: net: vdev: readlink inputs cannot be aliased
      drivers: net: vdev: fix 3 x strncpy misuse
      test-pmd: can't find include
      app: fix sprintf overrun bug
      app: test-bbdev: strcpy ok for allocated string
      app: test-bbdev: strcpy ok for allocated string 2


 app/proc-info/main.c                       |    9 +++++++--
 app/test-bbdev/test_bbdev_vector.c         |    5 +++--
 app/test-pmd/Makefile                      |    1 +
 drivers/bus/dpaa/base/qbman/qman.c         |   14 +++++++-------
 drivers/bus/dpaa/include/fsl_qman.h        |   24 +++++++++++++-----------
 drivers/bus/pci/linux/pci.c                |    3 ++-
 drivers/net/axgbe/axgbe_phy_impl.c         |    4 ++--
 drivers/net/nfp/nfp_net.c                  |    4 ++--
 drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c |    3 ++-
 drivers/net/nfp/nfpcore/nfp_resource.c     |    6 +++---
 drivers/net/qede/base/ecore_int.c          |   10 ++++++----
 drivers/net/qede/qede_main.c               |    3 ++-
 drivers/net/sfc/sfc_ethdev.c               |    9 ++++++---
 drivers/net/vdev_netvsc/vdev_netvsc.c      |   16 ++++++++++------
 lib/librte_table/rte_table_hash_cuckoo.c   |    2 +-
 15 files changed, 67 insertions(+), 46 deletions(-)

--

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 01/18] lib: ret_table: workaround hash function cast error
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
@ 2018-05-08  4:29 ` Andy Green
  2018-05-08  4:29 ` [dpdk-dev] [PATCH 02/18] drivers: bus: pci: fix strncpy dangerous code Andy Green
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:29 UTC (permalink / raw)
  To: dev

/home/agreen/projects/dpdk/lib/librte_table/rte_table_hash_cuckoo.c:110:16: error: cast between incompatible function types from ‘rte_table_hash_op_hash’ {aka ‘long unsigned int (*)(void *, void *, unsigned int,  long unsigned int)’} to ‘uint32_t (*)(const void *, uint32_t,  uint32_t)’ {aka ‘unsigned int (*)(const void *, unsigned int,  unsigned int)’} [-Werror=cast-function-type]
   .hash_func = (rte_hash_function)(p->f_hash),

The code seems to be quite broken.

It's casting this

typedef uint64_t (*rte_table_hash_op_hash)(
        void *key,
        void *key_mask,
        uint32_t key_size,
        uint64_t seed);

to this

typedef uint32_t (*rte_hash_function)(const void *key, uint32_t key_len,
                                      uint32_t init_val);

if the definition with 4 args is later called with a pointer
giving it three args, obviously it working is just an accident.  I
grepped around a bit and could not see it being cast back to
the original type before use; the uses I saw have three args.

I simply patch it to stop the build breaking, rather than fix it,
since I am not sure what a fix should look like considering the
whole code.
---
 lib/librte_table/rte_table_hash_cuckoo.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_table/rte_table_hash_cuckoo.c b/lib/librte_table/rte_table_hash_cuckoo.c
index dcb4fe978..eca72b506 100644
--- a/lib/librte_table/rte_table_hash_cuckoo.c
+++ b/lib/librte_table/rte_table_hash_cuckoo.c
@@ -107,7 +107,7 @@ rte_table_hash_cuckoo_create(void *params,
 	struct rte_hash_parameters hash_cuckoo_params = {
 		.entries = p->n_keys,
 		.key_len = p->key_size,
-		.hash_func = (rte_hash_function)(p->f_hash),
+		.hash_func = (rte_hash_function)(void *)(p->f_hash),
 		.hash_func_init_val = p->seed,
 		.socket_id = socket_id,
 		.name = p->name

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 02/18] drivers: bus: pci: fix strncpy dangerous code
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
  2018-05-08  4:29 ` [dpdk-dev] [PATCH 01/18] lib: ret_table: workaround hash function cast error Andy Green
@ 2018-05-08  4:29 ` Andy Green
  2018-05-08  8:57   ` Bruce Richardson
  2018-05-08  4:29 ` [dpdk-dev] [PATCH 03/18] drivers: bus: dpaa: fix inconsistent struct alignment Andy Green
                   ` (15 subsequent siblings)
  17 siblings, 1 reply; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:29 UTC (permalink / raw)
  To: dev

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);
---
 drivers/bus/pci/linux/pci.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 4630a8057..b5bdfd33e 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -54,7 +54,8 @@ 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);
+		strncpy(dri_name, name + 1, sizeof(dri_name) - 1);
+		dri_name[sizeof(dri_name) - 1] = '\0';
 		return 0;
 	}
 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 03/18] drivers: bus: dpaa: fix inconsistent struct alignment
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
  2018-05-08  4:29 ` [dpdk-dev] [PATCH 01/18] lib: ret_table: workaround hash function cast error Andy Green
  2018-05-08  4:29 ` [dpdk-dev] [PATCH 02/18] drivers: bus: pci: fix strncpy dangerous code Andy Green
@ 2018-05-08  4:29 ` Andy Green
  2018-05-08  4:29 ` [dpdk-dev] [PATCH 04/18] drivers: net: axgbe: fix broken eeprom string comp Andy Green
                   ` (14 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:29 UTC (permalink / raw)
  To: dev

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.
---
 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..cf6dfce11 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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 04/18] drivers: net: axgbe: fix broken eeprom string comp
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
                   ` (2 preceding siblings ...)
  2018-05-08  4:29 ` [dpdk-dev] [PATCH 03/18] drivers: bus: dpaa: fix inconsistent struct alignment Andy Green
@ 2018-05-08  4:29 ` Andy Green
  2018-05-08  4:29 ` [dpdk-dev] [PATCH 05/18] drivers: net: nfp: nfpcore: fix strncpy misuse Andy Green
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:29 UTC (permalink / raw)
  To: dev

/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))
---
 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;

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 05/18] drivers: net: nfp: nfpcore: fix strncpy misuse
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
                   ` (3 preceding siblings ...)
  2018-05-08  4:29 ` [dpdk-dev] [PATCH 04/18] drivers: net: axgbe: fix broken eeprom string comp Andy Green
@ 2018-05-08  4:29 ` Andy Green
  2018-05-08  8:58   ` Bruce Richardson
  2018-05-08  4:29 ` [dpdk-dev] [PATCH 06/18] drivers: net: nfp: nfpcore fix off-by-one and no NUL on strncpy use Andy Green
                   ` (12 subsequent siblings)
  17 siblings, 1 reply; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:29 UTC (permalink / raw)
  To: dev


---
 drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c |    3 ++-
 1 file changed, 2 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..9f6704a7f 100644
--- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
+++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
@@ -846,7 +846,8 @@ nfp6000_init(struct nfp_cpp *cpp, const char *devname)
 
 
 	memset(desc->busdev, 0, BUSDEV_SZ);
-	strncpy(desc->busdev, devname, strlen(devname));
+	strncpy(desc->busdev, devname, sizeof(desc->busdev) - 1);
+	desc->busdev[sizeof(desc->busdev) - 1] = '\0';
 
 	ret = nfp_acquire_process_lock(desc);
 	if (ret)

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 06/18] drivers: net: nfp: nfpcore fix off-by-one and no NUL on strncpy use
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
                   ` (4 preceding siblings ...)
  2018-05-08  4:29 ` [dpdk-dev] [PATCH 05/18] drivers: net: nfp: nfpcore: fix strncpy misuse Andy Green
@ 2018-05-08  4:29 ` Andy Green
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 07/18] drivers: net: nfp: don't memcpy out of source range Andy Green
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:29 UTC (permalink / raw)
  To: dev

/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));
---
 drivers/net/nfp/nfpcore/nfp_resource.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/nfp/nfpcore/nfp_resource.c b/drivers/net/nfp/nfpcore/nfp_resource.c
index e1df2b2e1..54c6dcf54 100644
--- a/drivers/net/nfp/nfpcore/nfp_resource.c
+++ b/drivers/net/nfp/nfpcore/nfp_resource.c
@@ -65,15 +65,15 @@ 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));
+	strncpy(name_pad, res->name, sizeof(name_pad) - 1);
 
 	/* Search for a matching entry */
 	if (!memcmp(name_pad, NFP_RESOURCE_TBL_NAME "\0\0\0\0\0\0\0\0", 8)) {

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 07/18] drivers: net: nfp: don't memcpy out of source range
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
                   ` (5 preceding siblings ...)
  2018-05-08  4:29 ` [dpdk-dev] [PATCH 06/18] drivers: net: nfp: nfpcore fix off-by-one and no NUL on strncpy use Andy Green
@ 2018-05-08  4:30 ` Andy Green
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 08/18] drivers: net: nfp: fix buffer overflow in fw_name Andy Green
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:30 UTC (permalink / raw)
  To: dev

/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));
---
 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 048324ec9..199aac40b 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);

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 08/18] drivers: net: nfp: fix buffer overflow in fw_name
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
                   ` (6 preceding siblings ...)
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 07/18] drivers: net: nfp: don't memcpy out of source range Andy Green
@ 2018-05-08  4:30 ` Andy Green
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant and NUL Andy Green
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:30 UTC (permalink / raw)
  To: dev

/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);
---
 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 199aac40b..d5f0e54e8 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -3144,7 +3144,7 @@ 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 fw_name[130];
 	char serial[100];
 	struct stat file_stat;
 	off_t fsize, bytes;

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant and NUL
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
                   ` (7 preceding siblings ...)
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 08/18] drivers: net: nfp: fix buffer overflow in fw_name Andy Green
@ 2018-05-08  4:30 ` Andy Green
  2018-05-08 17:59   ` Shaikh, Shahed
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 10/18] drivers: net: qede: fix broken strncpy Andy Green
                   ` (8 subsequent siblings)
  17 siblings, 1 reply; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:30 UTC (permalink / raw)
  To: dev


---
 drivers/net/qede/base/ecore_int.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/qede/base/ecore_int.c b/drivers/net/qede/base/ecore_int.c
index f43781ba4..c809d84ef 100644
--- a/drivers/net/qede/base/ecore_int.c
+++ b/drivers/net/qede/base/ecore_int.c
@@ -1103,10 +1103,12 @@ static enum _ecore_status_t ecore_int_deassertion(struct ecore_hwfn *p_hwfn,
 						OSAL_SNPRINTF(bit_name, 30,
 							      p_aeu->bit_name,
 							      num);
-					else
-						OSAL_STRNCPY(bit_name,
-							     p_aeu->bit_name,
-							     30);
+					else {
+						strncpy(bit_name,
+							p_aeu->bit_name,
+							sizeof(bit_name) - 1);
+						bit_name[sizeof(bit_name) - 1] = '\0';
+					}
 
 					/* We now need to pass bitmask in its
 					 * correct position.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 10/18] drivers: net: qede: fix broken strncpy
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
                   ` (8 preceding siblings ...)
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant and NUL Andy Green
@ 2018-05-08  4:30 ` Andy Green
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 11/18] drivers:net:sfc: fix strncpy length Andy Green
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:30 UTC (permalink / raw)
  To: dev

/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);
    ~~~~~~~~~~~~~~~~~~~~~~~~~
---
 drivers/net/qede/qede_main.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/qede/qede_main.c b/drivers/net/qede/qede_main.c
index 2333ca073..243851da6 100644
--- a/drivers/net/qede/qede_main.c
+++ b/drivers/net/qede/qede_main.c
@@ -305,7 +305,8 @@ static int qed_slowpath_start(struct ecore_dev *edev,
 		    (params->drv_rev << 8) | (params->drv_eng);
 		/* TBD: strlcpy() */
 		strncpy((char *)drv_version.name, (const char *)params->name,
-			MCP_DRV_VER_STR_SIZE - 4);
+			sizeof(drv_version.name) - 1);
+		drv_version.name[sizeof(drv_version.name) - 1] = '\0';
 		rc = ecore_mcp_send_drv_version(hwfn, hwfn->p_main_ptt,
 						&drv_version);
 		if (rc) {

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 11/18] drivers:net:sfc: fix strncpy length
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
                   ` (9 preceding siblings ...)
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 10/18] drivers: net: qede: fix broken strncpy Andy Green
@ 2018-05-08  4:30 ` Andy Green
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 12/18] drivers: net: sfc: fix another strncpy size and NUL Andy Green
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:30 UTC (permalink / raw)
  To: dev


---
 drivers/net/sfc/sfc_ethdev.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index e42d55350..e9bb283e0 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -742,7 +742,7 @@ sfc_xstats_get_names_by_id(struct rte_eth_dev *dev,
 			char *name = xstats_names[nb_written++].name;
 
 			strncpy(name, efx_mac_stat_name(sa->nic, i),
-				sizeof(xstats_names[0].name));
+				sizeof(xstats_names[0].name) - 1);
 			name[sizeof(xstats_names[0].name) - 1] = '\0';
 		}
 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 12/18] drivers: net: sfc: fix another strncpy size and NUL
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
                   ` (10 preceding siblings ...)
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 11/18] drivers:net:sfc: fix strncpy length Andy Green
@ 2018-05-08  4:30 ` Andy Green
  2018-05-08  7:36   ` Andrew Rybchenko
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 13/18] drivers: net: vdev: readlink inputs cannot be aliased Andy Green
                   ` (5 subsequent siblings)
  17 siblings, 1 reply; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:30 UTC (permalink / raw)
  To: dev


---
 drivers/net/sfc/sfc_ethdev.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index e9bb283e0..bd5f17f33 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -662,10 +662,13 @@ 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)
+			if (xstats_names != NULL && nstats < xstats_count) {
 				strncpy(xstats_names[nstats].name,
 					efx_mac_stat_name(sa->nic, i),
-					sizeof(xstats_names[0].name));
+					sizeof(xstats_names[0].name) - 1);
+				xstats_names[0].name[
+					sizeof(xstats_names[0].name) - 1] = '\0';
+			}
 			nstats++;
 		}
 	}

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 13/18] drivers: net: vdev: readlink inputs cannot be aliased
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
                   ` (11 preceding siblings ...)
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 12/18] drivers: net: sfc: fix another strncpy size and NUL Andy Green
@ 2018-05-08  4:30 ` Andy Green
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 14/18] drivers: net: vdev: fix 3 x strncpy misuse Andy Green
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:30 UTC (permalink / raw)
  To: dev

/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);
  ^~~
---
 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 c321a9f1b..c11794137 100644
--- a/drivers/net/vdev_netvsc/vdev_netvsc.c
+++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
@@ -327,12 +327,13 @@ 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(buf) - 1, "/sys/class/net/%s/%s", if_name, relpath);
+	if (ret == -1 || (size_t)ret >= sizeof(buf) - 1)
 		return -ENOBUFS;
-	ret = readlink(buf, buf, size);
+	ret = readlink(in, buf, size);
 	if (ret == -1)
 		return -errno;
 	if ((size_t)ret >= size - 1)

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 14/18] drivers: net: vdev: fix 3 x strncpy misuse
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
                   ` (12 preceding siblings ...)
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 13/18] drivers: net: vdev: readlink inputs cannot be aliased Andy Green
@ 2018-05-08  4:30 ` Andy Green
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 15/18] test-pmd: can't find include Andy Green
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:30 UTC (permalink / raw)
  To: dev


---
 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 c11794137..c36ec0f9a 100644
--- a/drivers/net/vdev_netvsc/vdev_netvsc.c
+++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
@@ -182,7 +182,8 @@ 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));
+		strncpy(req.ifr_name, iface[i].if_name, sizeof(req.ifr_name) - 1);
+		req.ifr_name[sizeof(req.ifr_name) - 1] = '\0';
 		if (ioctl(s, SIOCGIFHWADDR, &req) == -1) {
 			DRV_LOG(WARNING, "cannot retrieve information about"
 					 " interface \"%s\": %s",
@@ -383,7 +384,8 @@ 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));
+		strncpy(ctx->if_name, iface->if_name, sizeof(ctx->if_name) - 1);
+		ctx->if_name[sizeof(ctx->if_name) - 1] = '\0';
 		return 0;
 	}
 	if (!is_same_ether_addr(eth_addr, &ctx->if_addr))
@@ -581,7 +583,8 @@ 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));
+	strncpy(ctx->if_name, iface->if_name, sizeof(ctx->if_name) - 1);
+	ctx->if_name[sizeof(ctx->if_name) - 1] = '\0';
 	ctx->if_index = iface->if_index;
 	ctx->if_addr = *eth_addr;
 	ctx->pipe[0] = -1;

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 15/18] test-pmd: can't find include
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
                   ` (13 preceding siblings ...)
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 14/18] drivers: net: vdev: fix 3 x strncpy misuse Andy Green
@ 2018-05-08  4:30 ` Andy Green
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 16/18] app: fix sprintf overrun bug Andy Green
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:30 UTC (permalink / raw)
  To: dev

/home/agreen/projects/dpdk/app/test-pmd/cmdline.c:64:10: fatal error: rte_pmd_dpaa.h: No such file or directory
 #include <rte_pmd_dpaa.h>
          ^~~~~~~~~~~~~~~~
---
 app/test-pmd/Makefile |    1 +
 1 file changed, 1 insertion(+)

diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile
index 60ae9b9c1..a0fdd0e11 100644
--- a/app/test-pmd/Makefile
+++ b/app/test-pmd/Makefile
@@ -13,6 +13,7 @@ APP = testpmd
 CFLAGS += -DALLOW_EXPERIMENTAL_API
 CFLAGS += -O3
 CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -I$(RTE_SDK)/drivers/net/dpaa
 
 #
 # all source are stored in SRCS-y

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 16/18] app: fix sprintf overrun bug
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
                   ` (14 preceding siblings ...)
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 15/18] test-pmd: can't find include Andy Green
@ 2018-05-08  4:30 ` Andy Green
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 17/18] app: test-bbdev: strcpy ok for allocated string Andy Green
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 18/18] app: test-bbdev: strcpy ok for allocated string 2 Andy Green
  17 siblings, 0 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:30 UTC (permalink / raw)
  To: dev

/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]);
---
 app/proc-info/main.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 539e13243..df46c235e 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -488,14 +488,19 @@ 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));
+			buf[sizeof(buf) - 1] = '\0';
+			if (n > sizeof(buf) - 1)
+				n = sizeof(buf) - 1;
+			ret = write(stdout_fd, buf, n);
 			if (ret < 0)
 				goto err;
 		} else {

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 17/18] app: test-bbdev: strcpy ok for allocated string
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
                   ` (15 preceding siblings ...)
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 16/18] app: fix sprintf overrun bug Andy Green
@ 2018-05-08  4:30 ` Andy Green
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 18/18] app: test-bbdev: strcpy ok for allocated string 2 Andy Green
  17 siblings, 0 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:30 UTC (permalink / raw)
  To: dev


---
 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 addef0572..5ad2a6535 100644
--- a/app/test-bbdev/test_bbdev_vector.c
+++ b/app/test-bbdev/test_bbdev_vector.c
@@ -890,7 +890,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] == ','

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [dpdk-dev] [PATCH 18/18] app: test-bbdev: strcpy ok for allocated string 2
  2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
                   ` (16 preceding siblings ...)
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 17/18] app: test-bbdev: strcpy ok for allocated string Andy Green
@ 2018-05-08  4:30 ` Andy Green
  17 siblings, 0 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  4:30 UTC (permalink / raw)
  To: dev


---
 app/test-bbdev/test_bbdev_vector.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/test-bbdev/test_bbdev_vector.c b/app/test-bbdev/test_bbdev_vector.c
index 5ad2a6535..373f94984 100644
--- a/app/test-bbdev/test_bbdev_vector.c
+++ b/app/test-bbdev/test_bbdev_vector.c
@@ -912,7 +912,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;

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH 12/18] drivers: net: sfc: fix another strncpy size and NUL
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 12/18] drivers: net: sfc: fix another strncpy size and NUL Andy Green
@ 2018-05-08  7:36   ` Andrew Rybchenko
  2018-05-08  8:18     ` Andy Green
  0 siblings, 1 reply; 32+ messages in thread
From: Andrew Rybchenko @ 2018-05-08  7:36 UTC (permalink / raw)
  To: Andy Green, dev

Many thanks. I guess the most of below notes are applicable to many other
patches in the series.

Signed-off-by: ,  Fixes: and Cc: stable@dpdk.org tags are missing. See [1].

Changeset summary should start from "net/sfc: "
I.e. something like:
net/sfc: fix strncpy size and NUL

(it looks like "another" is useless in the original subject)

In general all patches should pass ./devtools/check-git-log.sh and 
./devtools/checkpatches.sh
(which requires path to Linux kernel checkpatches.pl).

Andrew.

[1] 
http://dpdk.org/doc/guides/contributing/patches.html#commit-messages-subject-line

On 05/08/2018 07:30 AM, Andy Green wrote:
> ---
>   drivers/net/sfc/sfc_ethdev.c |    7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
> index e9bb283e0..bd5f17f33 100644
> --- a/drivers/net/sfc/sfc_ethdev.c
> +++ b/drivers/net/sfc/sfc_ethdev.c
> @@ -662,10 +662,13 @@ 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)
> +			if (xstats_names != NULL && nstats < xstats_count) {
>   				strncpy(xstats_names[nstats].name,
>   					efx_mac_stat_name(sa->nic, i),
> -					sizeof(xstats_names[0].name));
> +					sizeof(xstats_names[0].name) - 1);
> +				xstats_names[0].name[
> +					sizeof(xstats_names[0].name) - 1] = '\0';
> +			}

In fact strlcpy() should be used.

>   			nstats++;
>   		}
>   	}
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH 12/18] drivers: net: sfc: fix another strncpy size and NUL
  2018-05-08  7:36   ` Andrew Rybchenko
@ 2018-05-08  8:18     ` Andy Green
  2018-05-08  9:02       ` Bruce Richardson
  2018-05-08  9:08       ` Andrew Rybchenko
  0 siblings, 2 replies; 32+ messages in thread
From: Andy Green @ 2018-05-08  8:18 UTC (permalink / raw)
  To: Andrew Rybchenko, dev



On 05/08/2018 03:36 PM, Andrew Rybchenko wrote:
> Many thanks. I guess the most of below notes are applicable to many other
> patches in the series.
> 
> Signed-off-by: ,  Fixes: and Cc: stable@dpdk.org tags are missing. See [1].

Everybody's project has different prejudices.

> Changeset summary should start from "net/sfc: "
> I.e. something like:
> net/sfc: fix strncpy size and NUL

Yeah if that's what you like.

> (it looks like "another" is useless in the original subject)

It captures my feeling at having to wade through making 18 fixes before 
I could compile the project on current Fedora.

> In general all patches should pass ./devtools/check-git-log.sh and 
> ./devtools/checkpatches.sh
> (which requires path to Linux kernel checkpatches.pl).

Can you help me understand why adding CRLFs at 80 cols on the gcc errors 
I pasted helps anything at all?  The patches actually fix problems in 
the code.

If you don't care about Coverity, let me know and I will register this 
project there and send you fixes when I have time.

> Andrew.
> 
> [1] 
> http://dpdk.org/doc/guides/contributing/patches.html#commit-messages-subject-line
> 
> On 05/08/2018 07:30 AM, Andy Green wrote:
>> ---
>>   drivers/net/sfc/sfc_ethdev.c |    7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
>> index e9bb283e0..bd5f17f33 100644
>> --- a/drivers/net/sfc/sfc_ethdev.c
>> +++ b/drivers/net/sfc/sfc_ethdev.c
>> @@ -662,10 +662,13 @@ 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)
>> +			if (xstats_names != NULL && nstats < xstats_count) {
>>   				strncpy(xstats_names[nstats].name,
>>   					efx_mac_stat_name(sa->nic, i),
>> -					sizeof(xstats_names[0].name));
>> +					sizeof(xstats_names[0].name) - 1);
>> +				xstats_names[0].name[
>> +					sizeof(xstats_names[0].name) - 1] = '\0';
>> +			}
> 
> In fact strlcpy() should be used.
Fair enough.  Last time I looked it wasn't in glibc but seems it is now.

-Andy

>>   			nstats++;
>>   		}
>>   	}
>>
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH 02/18] drivers: bus: pci: fix strncpy dangerous code
  2018-05-08  4:29 ` [dpdk-dev] [PATCH 02/18] drivers: bus: pci: fix strncpy dangerous code Andy Green
@ 2018-05-08  8:57   ` Bruce Richardson
  0 siblings, 0 replies; 32+ messages in thread
From: Bruce Richardson @ 2018-05-08  8:57 UTC (permalink / raw)
  To: Andy Green; +Cc: dev

On Tue, May 08, 2018 at 12:29:38PM +0800, Andy Green wrote:
> 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);
> ---
>  drivers/bus/pci/linux/pci.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> index 4630a8057..b5bdfd33e 100644
> --- a/drivers/bus/pci/linux/pci.c
> +++ b/drivers/bus/pci/linux/pci.c
> @@ -54,7 +54,8 @@ 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);
> +		strncpy(dri_name, name + 1, sizeof(dri_name) - 1);
> +		dri_name[sizeof(dri_name) - 1] = '\0';
>  		return 0;
>  	}
While this fix is correct, a better fix would be to use strlcpy from
rte_string_fns.h.

	strlcpy(dri_name, name + 1, sizeof(dri_name));

Regards,
/Bruce

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH 05/18] drivers: net: nfp: nfpcore: fix strncpy misuse
  2018-05-08  4:29 ` [dpdk-dev] [PATCH 05/18] drivers: net: nfp: nfpcore: fix strncpy misuse Andy Green
@ 2018-05-08  8:58   ` Bruce Richardson
  2018-05-08  9:00     ` Andy Green
  0 siblings, 1 reply; 32+ messages in thread
From: Bruce Richardson @ 2018-05-08  8:58 UTC (permalink / raw)
  To: Andy Green; +Cc: dev

On Tue, May 08, 2018 at 12:29:53PM +0800, Andy Green wrote:
> 
> ---
>  drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c |    3 ++-
>  1 file changed, 2 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..9f6704a7f 100644
> --- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
> +++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
> @@ -846,7 +846,8 @@ nfp6000_init(struct nfp_cpp *cpp, const char *devname)
>  
>  
>  	memset(desc->busdev, 0, BUSDEV_SZ);
> -	strncpy(desc->busdev, devname, strlen(devname));
> +	strncpy(desc->busdev, devname, sizeof(desc->busdev) - 1);
> +	desc->busdev[sizeof(desc->busdev) - 1] = '\0';
>  
>  	ret = nfp_acquire_process_lock(desc);
>  	if (ret)
As with previous patch, a better fix is to use strlcpy. This would apply to
just about all uses of strncpy in the code.

/Bruce

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH 05/18] drivers: net: nfp: nfpcore: fix strncpy misuse
  2018-05-08  8:58   ` Bruce Richardson
@ 2018-05-08  9:00     ` Andy Green
  2018-05-08  9:03       ` Bruce Richardson
  0 siblings, 1 reply; 32+ messages in thread
From: Andy Green @ 2018-05-08  9:00 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev



On 05/08/2018 04:58 PM, Bruce Richardson wrote:
> On Tue, May 08, 2018 at 12:29:53PM +0800, Andy Green wrote:
>>
>> ---
>>   drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c |    3 ++-
>>   1 file changed, 2 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..9f6704a7f 100644
>> --- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
>> +++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
>> @@ -846,7 +846,8 @@ nfp6000_init(struct nfp_cpp *cpp, const char *devname)
>>   
>>   
>>   	memset(desc->busdev, 0, BUSDEV_SZ);
>> -	strncpy(desc->busdev, devname, strlen(devname));
>> +	strncpy(desc->busdev, devname, sizeof(desc->busdev) - 1);
>> +	desc->busdev[sizeof(desc->busdev) - 1] = '\0';
>>   
>>   	ret = nfp_acquire_process_lock(desc);
>>   	if (ret)
> As with previous patch, a better fix is to use strlcpy. This would apply to
> just about all uses of strncpy in the code.

OK.

But the strncpy() was already there, it's not introduced by the patch.

I agree just doing it in one hit with strlcpy() is nicer.

-Andy

> /Bruce
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH 12/18] drivers: net: sfc: fix another strncpy size and NUL
  2018-05-08  8:18     ` Andy Green
@ 2018-05-08  9:02       ` Bruce Richardson
  2018-05-08  9:08       ` Andrew Rybchenko
  1 sibling, 0 replies; 32+ messages in thread
From: Bruce Richardson @ 2018-05-08  9:02 UTC (permalink / raw)
  To: Andy Green; +Cc: Andrew Rybchenko, dev

On Tue, May 08, 2018 at 04:18:41PM +0800, Andy Green wrote:
> 
> 
> On 05/08/2018 03:36 PM, Andrew Rybchenko wrote:
> > Many thanks. I guess the most of below notes are applicable to many other
> > patches in the series.
> > 
> > Signed-off-by: ,  Fixes: and Cc: stable@dpdk.org tags are missing. See [1].
> 
> Everybody's project has different prejudices.
> 
> > Changeset summary should start from "net/sfc: "
> > I.e. something like:
> > net/sfc: fix strncpy size and NUL
> 
> Yeah if that's what you like.
> 
> > (it looks like "another" is useless in the original subject)
> 
> It captures my feeling at having to wade through making 18 fixes before I
> could compile the project on current Fedora.
> 
> > In general all patches should pass ./devtools/check-git-log.sh and
> > ./devtools/checkpatches.sh
> > (which requires path to Linux kernel checkpatches.pl).
> 
> Can you help me understand why adding CRLFs at 80 cols on the gcc errors I
> pasted helps anything at all?  The patches actually fix problems in the
> code.
> 

I don't think there is any need to wrap the error messages since they are
pasted verbatim.

> If you don't care about Coverity, let me know and I will register this
> project there and send you fixes when I have time.
> 

FYI: The DPDK project is already registered in coverity and scanned regularly.
We do try and fix as many issues it flags as possible, but not everything
gets dealt with as quickly as we'd like, sadly.

https://scan.coverity.com/projects/dpdk-data-plane-development-kit

Regards,
/Bruce

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH 05/18] drivers: net: nfp: nfpcore: fix strncpy misuse
  2018-05-08  9:00     ` Andy Green
@ 2018-05-08  9:03       ` Bruce Richardson
  0 siblings, 0 replies; 32+ messages in thread
From: Bruce Richardson @ 2018-05-08  9:03 UTC (permalink / raw)
  To: Andy Green; +Cc: dev

On Tue, May 08, 2018 at 05:00:21PM +0800, Andy Green wrote:
> 
> 
> On 05/08/2018 04:58 PM, Bruce Richardson wrote:
> > On Tue, May 08, 2018 at 12:29:53PM +0800, Andy Green wrote:
> > > 
> > > ---
> > >   drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c |    3 ++-
> > >   1 file changed, 2 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..9f6704a7f 100644
> > > --- a/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
> > > +++ b/drivers/net/nfp/nfpcore/nfp_cpp_pcie_ops.c
> > > @@ -846,7 +846,8 @@ nfp6000_init(struct nfp_cpp *cpp, const char *devname)
> > >   	memset(desc->busdev, 0, BUSDEV_SZ);
> > > -	strncpy(desc->busdev, devname, strlen(devname));
> > > +	strncpy(desc->busdev, devname, sizeof(desc->busdev) - 1);
> > > +	desc->busdev[sizeof(desc->busdev) - 1] = '\0';
> > >   	ret = nfp_acquire_process_lock(desc);
> > >   	if (ret)
> > As with previous patch, a better fix is to use strlcpy. This would apply to
> > just about all uses of strncpy in the code.
> 
> OK.
> 
> But the strncpy() was already there, it's not introduced by the patch.
> 
> I agree just doing it in one hit with strlcpy() is nicer.
> 
> -Andy
> 
Thanks for the fixes!

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH 12/18] drivers: net: sfc: fix another strncpy size and NUL
  2018-05-08  8:18     ` Andy Green
  2018-05-08  9:02       ` Bruce Richardson
@ 2018-05-08  9:08       ` Andrew Rybchenko
  1 sibling, 0 replies; 32+ messages in thread
From: Andrew Rybchenko @ 2018-05-08  9:08 UTC (permalink / raw)
  To: Andy Green, dev

On 05/08/2018 11:18 AM, Andy Green wrote:
> On 05/08/2018 03:36 PM, Andrew Rybchenko wrote:
>> (it looks like "another" is useless in the original subject)
>
> It captures my feeling at having to wade through making 18 fixes 
> before I could compile the project on current Fedora.

I see.

>> In general all patches should pass ./devtools/check-git-log.sh and 
>> ./devtools/checkpatches.sh
>> (which requires path to Linux kernel checkpatches.pl).
>
> Can you help me understand why adding CRLFs at 80 cols on the gcc 
> errors I pasted helps anything at all?  The patches actually fix 
> problems in the code.

Seeing GCC errors which patch fixes is useful to see. Yes, I agree that 
it is real problem in the code.

> If you don't care about Coverity, let me know and I will register this 
> project there and send you fixes when I have time.

dpdk is registered at Coverity and we get reports from time to time.

>
>> Andrew.
>>
>> [1] 
>> http://dpdk.org/doc/guides/contributing/patches.html#commit-messages-subject-line
>>
>> On 05/08/2018 07:30 AM, Andy Green wrote:
>>> ---
>>>   drivers/net/sfc/sfc_ethdev.c |    7 +++++--
>>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/sfc/sfc_ethdev.c 
>>> b/drivers/net/sfc/sfc_ethdev.c
>>> index e9bb283e0..bd5f17f33 100644
>>> --- a/drivers/net/sfc/sfc_ethdev.c
>>> +++ b/drivers/net/sfc/sfc_ethdev.c
>>> @@ -662,10 +662,13 @@ 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)
>>> +            if (xstats_names != NULL && nstats < xstats_count) {
>>>                   strncpy(xstats_names[nstats].name,
>>>                       efx_mac_stat_name(sa->nic, i),
>>> -                    sizeof(xstats_names[0].name));
>>> +                    sizeof(xstats_names[0].name) - 1);
>>> +                xstats_names[0].name[
>>> +                    sizeof(xstats_names[0].name) - 1] = '\0';
>>> +            }
>>
>> In fact strlcpy() should be used.
> Fair enough.  Last time I looked it wasn't in glibc but seems it is now.

As far as I know it is not in glibc, but dpdk has internal fallback if 
the function
is not available from external libs.

Andrew.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant and NUL
  2018-05-08  4:30 ` [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant and NUL Andy Green
@ 2018-05-08 17:59   ` Shaikh, Shahed
  2018-05-08 19:53     ` Bruce Richardson
  0 siblings, 1 reply; 32+ messages in thread
From: Shaikh, Shahed @ 2018-05-08 17:59 UTC (permalink / raw)
  To: Andy Green, dev



> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Andy Green
> Sent: Monday, May 7, 2018 11:30 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant and
> NUL
> 
> 
> ---
>  drivers/net/qede/base/ecore_int.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/qede/base/ecore_int.c
> b/drivers/net/qede/base/ecore_int.c
> index f43781ba4..c809d84ef 100644
> --- a/drivers/net/qede/base/ecore_int.c
> +++ b/drivers/net/qede/base/ecore_int.c
> @@ -1103,10 +1103,12 @@ static enum _ecore_status_t
> ecore_int_deassertion(struct ecore_hwfn *p_hwfn,
>  						OSAL_SNPRINTF(bit_name, 30,
>  							      p_aeu->bit_name,
>  							      num);
> -					else
> -						OSAL_STRNCPY(bit_name,
> -							     p_aeu->bit_name,
> -							     30);
> +					else {
> +						strncpy(bit_name,
> +							p_aeu->bit_name,
> +							sizeof(bit_name) - 1);
> +						bit_name[sizeof(bit_name) - 1]
> = '\0';
> +					}

I think you can retain OSAL_STRNCPY and just replace 30 with 'bit_name[sizeof(bit_name) - 1'  and then set last byte with '\0' just like you did.

Thanks,
Shahed
> 
>  					/* We now need to pass bitmask in its
>  					 * correct position.


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant and NUL
  2018-05-08 17:59   ` Shaikh, Shahed
@ 2018-05-08 19:53     ` Bruce Richardson
  2018-05-08 20:02       ` Shaikh, Shahed
  0 siblings, 1 reply; 32+ messages in thread
From: Bruce Richardson @ 2018-05-08 19:53 UTC (permalink / raw)
  To: dev-bounces; +Cc: Andy Green, dev

On Tue, May 08, 2018 at 05:59:47PM +0000, dev-bounces@dpdk.org wrote:
> 
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Andy Green
> > Sent: Monday, May 7, 2018 11:30 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant and
> > NUL
> > 
> > 
> > ---
> >  drivers/net/qede/base/ecore_int.c |   10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/net/qede/base/ecore_int.c
> > b/drivers/net/qede/base/ecore_int.c
> > index f43781ba4..c809d84ef 100644
> > --- a/drivers/net/qede/base/ecore_int.c
> > +++ b/drivers/net/qede/base/ecore_int.c
> > @@ -1103,10 +1103,12 @@ static enum _ecore_status_t
> > ecore_int_deassertion(struct ecore_hwfn *p_hwfn,
> >  						OSAL_SNPRINTF(bit_name, 30,
> >  							      p_aeu->bit_name,
> >  							      num);
> > -					else
> > -						OSAL_STRNCPY(bit_name,
> > -							     p_aeu->bit_name,
> > -							     30);
> > +					else {
> > +						strncpy(bit_name,
> > +							p_aeu->bit_name,
> > +							sizeof(bit_name) - 1);
> > +						bit_name[sizeof(bit_name) - 1]
> > = '\0';
> > +					}
> 
> I think you can retain OSAL_STRNCPY and just replace 30 with 'bit_name[sizeof(bit_name) - 1'  and then set last byte with '\0' just like you did.

Can that actually be fixed inside OSAL_STRNCPY itself, rather than having
each use needing to explicitly null-terminate?

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant and NUL
  2018-05-08 19:53     ` Bruce Richardson
@ 2018-05-08 20:02       ` Shaikh, Shahed
  2018-05-08 22:07         ` Andy Green
  0 siblings, 1 reply; 32+ messages in thread
From: Shaikh, Shahed @ 2018-05-08 20:02 UTC (permalink / raw)
  To: Bruce Richardson, dev-bounces; +Cc: Andy Green, dev

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Bruce Richardson
> Sent: Tuesday, May 8, 2018 2:53 PM
> To: dev-bounces@dpdk.org
> Cc: Andy Green <andy@warmcat.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant
> and NUL
> 
> On Tue, May 08, 2018 at 05:59:47PM +0000, dev-bounces@dpdk.org wrote:
> >
> >
> > > -----Original Message-----
> > > From: dev <dev-bounces@dpdk.org> On Behalf Of Andy Green
> > > Sent: Monday, May 7, 2018 11:30 PM
> > > To: dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy
> > > constant and NUL
> > >
> > >
> > > ---
> > >  drivers/net/qede/base/ecore_int.c |   10 ++++++----
> > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/qede/base/ecore_int.c
> > > b/drivers/net/qede/base/ecore_int.c
> > > index f43781ba4..c809d84ef 100644
> > > --- a/drivers/net/qede/base/ecore_int.c
> > > +++ b/drivers/net/qede/base/ecore_int.c
> > > @@ -1103,10 +1103,12 @@ static enum _ecore_status_t
> > > ecore_int_deassertion(struct ecore_hwfn *p_hwfn,
> > >  						OSAL_SNPRINTF(bit_name, 30,
> > >  							      p_aeu->bit_name,
> > >  							      num);
> > > -					else
> > > -						OSAL_STRNCPY(bit_name,
> > > -							     p_aeu->bit_name,
> > > -							     30);
> > > +					else {
> > > +						strncpy(bit_name,
> > > +							p_aeu->bit_name,
> > > +							sizeof(bit_name) - 1);
> > > +						bit_name[sizeof(bit_name) - 1]
> > > = '\0';
> > > +					}
> >
> > I think you can retain OSAL_STRNCPY and just replace 30 with
> 'bit_name[sizeof(bit_name) - 1'  and then set last byte with '\0' just like you did.
> 
> Can that actually be fixed inside OSAL_STRNCPY itself, rather than having each
> use needing to explicitly null-terminate?

Although there is only instance of OSAL_STRNCPY, it makes sense to modify it.

Thanks,
Shahed

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant and NUL
  2018-05-08 20:02       ` Shaikh, Shahed
@ 2018-05-08 22:07         ` Andy Green
  2018-05-08 23:33           ` Shaikh, Shahed
  0 siblings, 1 reply; 32+ messages in thread
From: Andy Green @ 2018-05-08 22:07 UTC (permalink / raw)
  To: Shaikh, Shahed, Bruce Richardson, dev-bounces; +Cc: dev



On 05/09/2018 04:02 AM, Shaikh, Shahed wrote:
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Bruce Richardson
>> Sent: Tuesday, May 8, 2018 2:53 PM
>> To: dev-bounces@dpdk.org
>> Cc: Andy Green <andy@warmcat.com>; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant
>> and NUL
>>
>> On Tue, May 08, 2018 at 05:59:47PM +0000, dev-bounces@dpdk.org wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Andy Green
>>>> Sent: Monday, May 7, 2018 11:30 PM
>>>> To: dev@dpdk.org
>>>> Subject: [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy
>>>> constant and NUL
>>>>
>>>>
>>>> ---
>>>>   drivers/net/qede/base/ecore_int.c |   10 ++++++----
>>>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/net/qede/base/ecore_int.c
>>>> b/drivers/net/qede/base/ecore_int.c
>>>> index f43781ba4..c809d84ef 100644
>>>> --- a/drivers/net/qede/base/ecore_int.c
>>>> +++ b/drivers/net/qede/base/ecore_int.c
>>>> @@ -1103,10 +1103,12 @@ static enum _ecore_status_t
>>>> ecore_int_deassertion(struct ecore_hwfn *p_hwfn,
>>>>   						OSAL_SNPRINTF(bit_name, 30,
>>>>   							      p_aeu->bit_name,
>>>>   							      num);
>>>> -					else
>>>> -						OSAL_STRNCPY(bit_name,
>>>> -							     p_aeu->bit_name,
>>>> -							     30);
>>>> +					else {
>>>> +						strncpy(bit_name,
>>>> +							p_aeu->bit_name,
>>>> +							sizeof(bit_name) - 1);
>>>> +						bit_name[sizeof(bit_name) - 1]
>>>> = '\0';
>>>> +					}
>>>
>>> I think you can retain OSAL_STRNCPY and just replace 30 with
>> 'bit_name[sizeof(bit_name) - 1'  and then set last byte with '\0' just like you did.
>>
>> Can that actually be fixed inside OSAL_STRNCPY itself, rather than having each
>> use needing to explicitly null-terminate?
> 
> Although there is only instance of OSAL_STRNCPY, it makes sense to modify it.

Doesn't it make more sense to get rid of OSAL_* that bring nothing at 
all to the party?

#define OSAL_SPRINTF(name, pattern, ...) \
         sprintf(name, pattern, ##__VA_ARGS__)
#define OSAL_SNPRINTF(buf, size, format, ...) \
         snprintf(buf, size, format, ##__VA_ARGS__)
#define OSAL_STRLEN(string) strlen(string)
#define OSAL_STRCPY(dst, string) strcpy(dst, string)
#define OSAL_STRNCPY(dst, string, len) strncpy(dst, string, len)
#define OSAL_STRCMP(str1, str2) strcmp(str1, str2)

Do I miss the point or these are just cruft?

-Andy

> Thanks,
> Shahed
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant and NUL
  2018-05-08 22:07         ` Andy Green
@ 2018-05-08 23:33           ` Shaikh, Shahed
  0 siblings, 0 replies; 32+ messages in thread
From: Shaikh, Shahed @ 2018-05-08 23:33 UTC (permalink / raw)
  To: Andy Green, Bruce Richardson, dev-bounces; +Cc: dev


> >>> I think you can retain OSAL_STRNCPY and just replace 30 with
> >> 'bit_name[sizeof(bit_name) - 1'  and then set last byte with '\0' just like you
> did.
> >>
> >> Can that actually be fixed inside OSAL_STRNCPY itself, rather than
> >> having each use needing to explicitly null-terminate?
> >
> > Although there is only instance of OSAL_STRNCPY, it makes sense to modify it.
> 
> Doesn't it make more sense to get rid of OSAL_* that bring nothing at all to the
> party?
> 
> #define OSAL_SPRINTF(name, pattern, ...) \
>          sprintf(name, pattern, ##__VA_ARGS__) #define OSAL_SNPRINTF(buf, size,
> format, ...) \
>          snprintf(buf, size, format, ##__VA_ARGS__) #define OSAL_STRLEN(string)
> strlen(string) #define OSAL_STRCPY(dst, string) strcpy(dst, string) #define
> OSAL_STRNCPY(dst, string, len) strncpy(dst, string, len) #define
> OSAL_STRCMP(str1, str2) strcmp(str1, str2)
> 
> Do I miss the point or these are just cruft?

Hi Andy,

I'll send a cleanup patch for this. For now, you can go ahead with original patch.

Thanks,
Shahed

Acked-by: Shahed Shaikh <shahed.shaikh@cavium.com>

> 
> -Andy
> 
> > Thanks,
> > Shahed
> >

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2018-05-08 23:33 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-08  4:29 [dpdk-dev] [PATCH 00/18] Fix build on gcc8 and various bugs Andy Green
2018-05-08  4:29 ` [dpdk-dev] [PATCH 01/18] lib: ret_table: workaround hash function cast error Andy Green
2018-05-08  4:29 ` [dpdk-dev] [PATCH 02/18] drivers: bus: pci: fix strncpy dangerous code Andy Green
2018-05-08  8:57   ` Bruce Richardson
2018-05-08  4:29 ` [dpdk-dev] [PATCH 03/18] drivers: bus: dpaa: fix inconsistent struct alignment Andy Green
2018-05-08  4:29 ` [dpdk-dev] [PATCH 04/18] drivers: net: axgbe: fix broken eeprom string comp Andy Green
2018-05-08  4:29 ` [dpdk-dev] [PATCH 05/18] drivers: net: nfp: nfpcore: fix strncpy misuse Andy Green
2018-05-08  8:58   ` Bruce Richardson
2018-05-08  9:00     ` Andy Green
2018-05-08  9:03       ` Bruce Richardson
2018-05-08  4:29 ` [dpdk-dev] [PATCH 06/18] drivers: net: nfp: nfpcore fix off-by-one and no NUL on strncpy use Andy Green
2018-05-08  4:30 ` [dpdk-dev] [PATCH 07/18] drivers: net: nfp: don't memcpy out of source range Andy Green
2018-05-08  4:30 ` [dpdk-dev] [PATCH 08/18] drivers: net: nfp: fix buffer overflow in fw_name Andy Green
2018-05-08  4:30 ` [dpdk-dev] [PATCH 09/18] drivers: net: qede: fix strncpy constant and NUL Andy Green
2018-05-08 17:59   ` Shaikh, Shahed
2018-05-08 19:53     ` Bruce Richardson
2018-05-08 20:02       ` Shaikh, Shahed
2018-05-08 22:07         ` Andy Green
2018-05-08 23:33           ` Shaikh, Shahed
2018-05-08  4:30 ` [dpdk-dev] [PATCH 10/18] drivers: net: qede: fix broken strncpy Andy Green
2018-05-08  4:30 ` [dpdk-dev] [PATCH 11/18] drivers:net:sfc: fix strncpy length Andy Green
2018-05-08  4:30 ` [dpdk-dev] [PATCH 12/18] drivers: net: sfc: fix another strncpy size and NUL Andy Green
2018-05-08  7:36   ` Andrew Rybchenko
2018-05-08  8:18     ` Andy Green
2018-05-08  9:02       ` Bruce Richardson
2018-05-08  9:08       ` Andrew Rybchenko
2018-05-08  4:30 ` [dpdk-dev] [PATCH 13/18] drivers: net: vdev: readlink inputs cannot be aliased Andy Green
2018-05-08  4:30 ` [dpdk-dev] [PATCH 14/18] drivers: net: vdev: fix 3 x strncpy misuse Andy Green
2018-05-08  4:30 ` [dpdk-dev] [PATCH 15/18] test-pmd: can't find include Andy Green
2018-05-08  4:30 ` [dpdk-dev] [PATCH 16/18] app: fix sprintf overrun bug Andy Green
2018-05-08  4:30 ` [dpdk-dev] [PATCH 17/18] app: test-bbdev: strcpy ok for allocated string Andy Green
2018-05-08  4:30 ` [dpdk-dev] [PATCH 18/18] app: test-bbdev: strcpy ok for allocated string 2 Andy Green

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).