diff mbox series

[1/5] net/bonding: fix buffer length when printing strings

Message ID 20190403144505.46234-2-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series clean up snprintf use for string copying |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS

Commit Message

Bruce Richardson April 3, 2019, 2:45 p.m. UTC 
  Using the size of the source string is incorrect when printing using
snprintf. Instead pass in the buffer size to be used appropriately.

Fixes: 457ecf2953fc ("bond: add debug info for mode 6")

CC: Declan Doherty <declan.doherty@intel.com>
CC: stable@dpdk.org
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

Comments

Stephen Hemminger April 3, 2019, 3:47 p.m. UTC | #1 
On Wed,  3 Apr 2019 15:45:01 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

>  
>  static void
> -arp_op_name(uint16_t arp_op, char *buf)
> +arp_op_name(uint16_t arp_op, char *buf, size_t buf_len)
>  {
>  	switch (arp_op) {
>  	case ARP_OP_REQUEST:
> -		snprintf(buf, sizeof("ARP Request"), "%s", "ARP Request");
> +		snprintf(buf, buf_len, "%s", "ARP Request");
>  		return;
This should be strlcpy not snprintf
Bruce Richardson April 3, 2019, 3:51 p.m. UTC | #2 
On Wed, Apr 03, 2019 at 08:47:58AM -0700, Stephen Hemminger wrote:
> On Wed,  3 Apr 2019 15:45:01 +0100
> Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> >  
> >  static void
> > -arp_op_name(uint16_t arp_op, char *buf)
> > +arp_op_name(uint16_t arp_op, char *buf, size_t buf_len)
> >  {
> >  	switch (arp_op) {
> >  	case ARP_OP_REQUEST:
> > -		snprintf(buf, sizeof("ARP Request"), "%s", "ARP Request");
> > +		snprintf(buf, buf_len, "%s", "ARP Request");
> >  		return;
> This should be strlcpy not snprintf

Yes, it should, but I just let that get fixed by cocci script in the later
patch. For this one, I just fixed the most egregious error.
Stephen Hemminger April 3, 2019, 3:53 p.m. UTC | #3 
On Wed, 3 Apr 2019 16:51:17 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Wed, Apr 03, 2019 at 08:47:58AM -0700, Stephen Hemminger wrote:
> > On Wed,  3 Apr 2019 15:45:01 +0100
> > Bruce Richardson <bruce.richardson@intel.com> wrote:
> >   
> > >  
> > >  static void
> > > -arp_op_name(uint16_t arp_op, char *buf)
> > > +arp_op_name(uint16_t arp_op, char *buf, size_t buf_len)
> > >  {
> > >  	switch (arp_op) {
> > >  	case ARP_OP_REQUEST:
> > > -		snprintf(buf, sizeof("ARP Request"), "%s", "ARP Request");
> > > +		snprintf(buf, buf_len, "%s", "ARP Request");
> > >  		return;  
> > This should be strlcpy not snprintf  
> 
> Yes, it should, but I just let that get fixed by cocci script in the later
> patch. For this one, I just fixed the most egregious error.

Ok, I see
diff mbox series

Patch

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 58b6e4344..154257ffe 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -489,35 +489,31 @@  uint32_t burstnumberTX;
 #ifdef RTE_LIBRTE_BOND_DEBUG_ALB
 
 static void
-arp_op_name(uint16_t arp_op, char *buf)
+arp_op_name(uint16_t arp_op, char *buf, size_t buf_len)
 {
 	switch (arp_op) {
 	case ARP_OP_REQUEST:
-		snprintf(buf, sizeof("ARP Request"), "%s", "ARP Request");
+		snprintf(buf, buf_len, "%s", "ARP Request");
 		return;
 	case ARP_OP_REPLY:
-		snprintf(buf, sizeof("ARP Reply"), "%s", "ARP Reply");
+		snprintf(buf, buf_len, "%s", "ARP Reply");
 		return;
 	case ARP_OP_REVREQUEST:
-		snprintf(buf, sizeof("Reverse ARP Request"), "%s",
-				"Reverse ARP Request");
+		snprintf(buf, buf_len, "%s", "Reverse ARP Request");
 		return;
 	case ARP_OP_REVREPLY:
-		snprintf(buf, sizeof("Reverse ARP Reply"), "%s",
-				"Reverse ARP Reply");
+		snprintf(buf, buf_len, "%s", "Reverse ARP Reply");
 		return;
 	case ARP_OP_INVREQUEST:
-		snprintf(buf, sizeof("Peer Identify Request"), "%s",
-				"Peer Identify Request");
+		snprintf(buf, buf_len, "%s", "Peer Identify Request");
 		return;
 	case ARP_OP_INVREPLY:
-		snprintf(buf, sizeof("Peer Identify Reply"), "%s",
-				"Peer Identify Reply");
+		snprintf(buf, buf_len, "%s", "Peer Identify Reply");
 		return;
 	default:
 		break;
 	}
-	snprintf(buf, sizeof("Unknown"), "%s", "Unknown");
+	snprintf(buf, buf_len, "%s", "Unknown");
 	return;
 }
 #endif
@@ -621,7 +617,8 @@  mode6_debug(const char __attribute__((unused)) *info, struct ether_hdr *eth_h,
 		arp_h = (struct arp_hdr *)((char *)(eth_h + 1) + offset);
 		ipv4_addr_to_dot(arp_h->arp_data.arp_sip, src_ip, MaxIPv4String);
 		ipv4_addr_to_dot(arp_h->arp_data.arp_tip, dst_ip, MaxIPv4String);
-		arp_op_name(rte_be_to_cpu_16(arp_h->arp_op), ArpOp);
+		arp_op_name(rte_be_to_cpu_16(arp_h->arp_op),
+				ArpOp, sizeof(ArpOp));
 		MODE6_DEBUG(buf, src_ip, dst_ip, eth_h, ArpOp, port, *burstnumber);
 	}
 #endif