diff mbox series

net/nfp: fix sprintf with snprintf

Message ID 1549264928-3935-1-git-send-email-pallantlax.poornima@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series net/nfp: fix sprintf with snprintf |

Checks

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

Commit Message

Poornima, PallantlaX Feb. 4, 2019, 7:22 a.m. UTC 
  sprintf function is not secure as it doesn't check the length of string.
More secure function snprintf is used.

Fixes: 896c265ef9 ("net/nfp: use new CPP interface")
Fixes: c4171b520b ("net/nfp: support PF multiport")
Cc: stable@dpdk.org

Signed-off-by: Pallantla Poornima <pallantlax.poornima@intel.com>
---
 drivers/net/nfp/nfp_net.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

Comments

Ferruh Yigit March 1, 2019, 3:25 p.m. UTC | #1 
On 2/4/2019 7:22 AM, Pallantla Poornima wrote:
> sprintf function is not secure as it doesn't check the length of string.
> More secure function snprintf is used.

Can you please update title to reflect what actually fixed, something like:
net/nfp: fix possible buffer overflow

> 
> Fixes: 896c265ef9 ("net/nfp: use new CPP interface")
> Fixes: c4171b520b ("net/nfp: support PF multiport")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Pallantla Poornima <pallantlax.poornima@intel.com>
> ---
>  drivers/net/nfp/nfp_net.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
> index a791e95e2..dd8cae135 100644
> --- a/drivers/net/nfp/nfp_net.c
> +++ b/drivers/net/nfp/nfp_net.c
> @@ -3318,9 +3318,9 @@ nfp_pf_create_dev(struct rte_pci_device *dev, int port, int ports,
>  		return -ENOMEM;
>  
>  	if (ports > 1)
> -		sprintf(port_name, "%s_port%d", dev->device.name, port);
> +		snprintf(port_name, 100, "%s_port%d", dev->device.name, port);
>  	else
> -		sprintf(port_name, "%s", dev->device.name);
> +		snprintf(port_name, 100, "%s", dev->device.name);

This can be done as strlcat() but I leave this to Alejandro, unless you don't
get his feedback I think good to continue as it is.

<...>

> @@ -3530,8 +3533,9 @@ nfp_fw_setup(struct rte_pci_device *dev, struct nfp_cpp *cpp,
>  
>  	PMD_DRV_LOG(INFO, "Port speed: %u", nfp_eth_table->ports[0].speed);
>  
> -	sprintf(card_desc, "nic_%s_%dx%d.nffw", nfp_fw_model,
> -		nfp_eth_table->count, nfp_eth_table->ports[0].speed / 1000);
> +	snprintf(card_desc, sizeof(card_desc), "nic_%s_%dx%d.nffw",
> +			nfp_fw_model, nfp_eth_table->count,
> +				nfp_eth_table->ports[0].speed / 1000);

Can you please fix the indentation?
diff mbox series

Patch

diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index a791e95e2..dd8cae135 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -3318,9 +3318,9 @@  nfp_pf_create_dev(struct rte_pci_device *dev, int port, int ports,
 		return -ENOMEM;
 
 	if (ports > 1)
-		sprintf(port_name, "%s_port%d", dev->device.name, port);
+		snprintf(port_name, 100, "%s_port%d", dev->device.name, port);
 	else
-		sprintf(port_name, "%s", dev->device.name);
+		snprintf(port_name, 100, "%s", dev->device.name);
 
 
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
@@ -3433,12 +3433,14 @@  nfp_fw_upload(struct rte_pci_device *dev, struct nfp_nsp *nsp, char *card)
 	/* Looking for firmware file in order of priority */
 
 	/* First try to find a firmware image specific for this device */
-	sprintf(serial, "serial-%02x-%02x-%02x-%02x-%02x-%02x-%02x-%02x",
+	snprintf(serial, sizeof(serial),
+			"serial-%02x-%02x-%02x-%02x-%02x-%02x-%02x-%02x",
 		cpp->serial[0], cpp->serial[1], cpp->serial[2], cpp->serial[3],
 		cpp->serial[4], cpp->serial[5], cpp->interface >> 8,
 		cpp->interface & 0xff);
 
-	sprintf(fw_name, "%s/%s.nffw", DEFAULT_FW_PATH, serial);
+	snprintf(fw_name, sizeof(fw_name), "%s/%s.nffw", DEFAULT_FW_PATH,
+			serial);
 
 	PMD_DRV_LOG(DEBUG, "Trying with fw file: %s", fw_name);
 	fw_f = open(fw_name, O_RDONLY);
@@ -3446,7 +3448,8 @@  nfp_fw_upload(struct rte_pci_device *dev, struct nfp_nsp *nsp, char *card)
 		goto read_fw;
 
 	/* Then try the PCI name */
-	sprintf(fw_name, "%s/pci-%s.nffw", DEFAULT_FW_PATH, dev->device.name);
+	snprintf(fw_name, sizeof(fw_name), "%s/pci-%s.nffw", DEFAULT_FW_PATH,
+			dev->device.name);
 
 	PMD_DRV_LOG(DEBUG, "Trying with fw file: %s", fw_name);
 	fw_f = open(fw_name, O_RDONLY);
@@ -3454,7 +3457,7 @@  nfp_fw_upload(struct rte_pci_device *dev, struct nfp_nsp *nsp, char *card)
 		goto read_fw;
 
 	/* Finally try the card type and media */
-	sprintf(fw_name, "%s/%s", DEFAULT_FW_PATH, card);
+	snprintf(fw_name, sizeof(fw_name), "%s/%s", DEFAULT_FW_PATH, card);
 	PMD_DRV_LOG(DEBUG, "Trying with fw file: %s", fw_name);
 	fw_f = open(fw_name, O_RDONLY);
 	if (fw_f < 0) {
@@ -3530,8 +3533,9 @@  nfp_fw_setup(struct rte_pci_device *dev, struct nfp_cpp *cpp,
 
 	PMD_DRV_LOG(INFO, "Port speed: %u", nfp_eth_table->ports[0].speed);
 
-	sprintf(card_desc, "nic_%s_%dx%d.nffw", nfp_fw_model,
-		nfp_eth_table->count, nfp_eth_table->ports[0].speed / 1000);
+	snprintf(card_desc, sizeof(card_desc), "nic_%s_%dx%d.nffw",
+			nfp_fw_model, nfp_eth_table->count,
+				nfp_eth_table->ports[0].speed / 1000);
 
 	nsp = nfp_nsp_open(cpp);
 	if (!nsp) {