* [dpdk-dev] [PATCH v2] net/nfp: fix possible buffer overflow
@ 2019-03-08 10:28 Pallantla Poornima
2019-03-12 9:56 ` Alejandro Lucero
0 siblings, 1 reply; 8+ messages in thread
From: Pallantla Poornima @ 2019-03-08 10:28 UTC (permalink / raw)
To: dev
Cc: reshma.pattan, ferruh.yigit, alejandro.lucero,
Pallantla Poornima, stable
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>
---
v2: updated title as suggested.
---
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..f63def5ef 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);
+ strlcat(port_name, dev->device.name, 100);
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) {
--
2.17.2
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH v2] net/nfp: fix possible buffer overflow
2019-03-08 10:28 [dpdk-dev] [PATCH v2] net/nfp: fix possible buffer overflow Pallantla Poornima
@ 2019-03-12 9:56 ` Alejandro Lucero
2019-03-19 17:43 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
0 siblings, 1 reply; 8+ messages in thread
From: Alejandro Lucero @ 2019-03-12 9:56 UTC (permalink / raw)
To: Pallantla Poornima; +Cc: dev, reshma.pattan, Ferruh Yigit, dpdk stable
On Fri, Mar 8, 2019 at 10:28 AM Pallantla Poornima <
pallantlax.poornima@intel.com> wrote:
> 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>
> ---
> v2: updated title as suggested.
> ---
> 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..f63def5ef 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);
> + strlcat(port_name, dev->device.name, 100);
>
>
> 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) {
> --
> 2.17.2
>
>
I got a compilation error when applying this patch: strlcat can not be
found.
I guess this patch requires to check for system libraries versions.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH v2] net/nfp: fix possible buffer overflow
2019-03-12 9:56 ` Alejandro Lucero
@ 2019-03-19 17:43 ` Ferruh Yigit
2019-03-19 17:43 ` Ferruh Yigit
2019-03-21 10:10 ` Alejandro Lucero
0 siblings, 2 replies; 8+ messages in thread
From: Ferruh Yigit @ 2019-03-19 17:43 UTC (permalink / raw)
To: Alejandro Lucero, Pallantla Poornima; +Cc: dev, reshma.pattan, dpdk stable
On 3/12/2019 9:56 AM, Alejandro Lucero wrote:
> On Fri, Mar 8, 2019 at 10:28 AM Pallantla Poornima <
> pallantlax.poornima@intel.com> wrote:
>
>> 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>
>> ---
>> v2: updated title as suggested.
>> ---
>> 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..f63def5ef 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);
>> + strlcat(port_name, dev->device.name, 100);
>>
>>
>> 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) {
>> --
>> 2.17.2
>>
>>
> I got a compilation error when applying this patch: strlcat can not be
> found.
>
> I guess this patch requires to check for system libraries versions.
>
Hi Alejandro,
Linux doesn't have the 'strlcat' but there is DPDK implementation of it, comes
with '#include <rte_string_fns.h>' header which is already included in this file.
'strlcat' support is added in this release, 19.05, can you be using an old code?
Can you please double check the build with the latest code?
Thanks,
ferruh
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH v2] net/nfp: fix possible buffer overflow
2019-03-19 17:43 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
@ 2019-03-19 17:43 ` Ferruh Yigit
2019-03-21 10:10 ` Alejandro Lucero
1 sibling, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2019-03-19 17:43 UTC (permalink / raw)
To: Alejandro Lucero, Pallantla Poornima; +Cc: dev, reshma.pattan, dpdk stable
On 3/12/2019 9:56 AM, Alejandro Lucero wrote:
> On Fri, Mar 8, 2019 at 10:28 AM Pallantla Poornima <
> pallantlax.poornima@intel.com> wrote:
>
>> 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>
>> ---
>> v2: updated title as suggested.
>> ---
>> 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..f63def5ef 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);
>> + strlcat(port_name, dev->device.name, 100);
>>
>>
>> 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) {
>> --
>> 2.17.2
>>
>>
> I got a compilation error when applying this patch: strlcat can not be
> found.
>
> I guess this patch requires to check for system libraries versions.
>
Hi Alejandro,
Linux doesn't have the 'strlcat' but there is DPDK implementation of it, comes
with '#include <rte_string_fns.h>' header which is already included in this file.
'strlcat' support is added in this release, 19.05, can you be using an old code?
Can you please double check the build with the latest code?
Thanks,
ferruh
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH v2] net/nfp: fix possible buffer overflow
2019-03-19 17:43 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2019-03-19 17:43 ` Ferruh Yigit
@ 2019-03-21 10:10 ` Alejandro Lucero
2019-03-21 10:10 ` Alejandro Lucero
2019-03-21 17:03 ` Ferruh Yigit
1 sibling, 2 replies; 8+ messages in thread
From: Alejandro Lucero @ 2019-03-21 10:10 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Pallantla Poornima, dev, reshma.pattan, dpdk stable
On Tue, Mar 19, 2019 at 5:43 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> On 3/12/2019 9:56 AM, Alejandro Lucero wrote:
> > On Fri, Mar 8, 2019 at 10:28 AM Pallantla Poornima <
> > pallantlax.poornima@intel.com> wrote:
> >
> >> 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>
> >> ---
> >> v2: updated title as suggested.
> >> ---
> >> 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..f63def5ef 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);
> >> + strlcat(port_name, dev->device.name, 100);
> >>
> >>
> >> 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) {
> >> --
> >> 2.17.2
> >>
> >>
> > I got a compilation error when applying this patch: strlcat can not be
> > found.
> >
> > I guess this patch requires to check for system libraries versions.
> >
>
> Hi Alejandro,
>
>
Hi Ferruh,
> Linux doesn't have the 'strlcat' but there is DPDK implementation of it,
> comes
> with '#include <rte_string_fns.h>' header which is already included in
> this file.
>
> 'strlcat' support is added in this release, 19.05, can you be using an old
> code?
> Can you please double check the build with the latest code?
>
>
I have tried again with tip DPDK and it works fine. I would say I used also
tip the first time, but anyway, it compiles now without problem.
I have also performed some basic tests and it is all fine.
So:
Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Tested-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Thanks!
> Thanks,
> ferruh
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH v2] net/nfp: fix possible buffer overflow
2019-03-21 10:10 ` Alejandro Lucero
@ 2019-03-21 10:10 ` Alejandro Lucero
2019-03-21 17:03 ` Ferruh Yigit
1 sibling, 0 replies; 8+ messages in thread
From: Alejandro Lucero @ 2019-03-21 10:10 UTC (permalink / raw)
To: Ferruh Yigit; +Cc: Pallantla Poornima, dev, reshma.pattan, dpdk stable
On Tue, Mar 19, 2019 at 5:43 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> On 3/12/2019 9:56 AM, Alejandro Lucero wrote:
> > On Fri, Mar 8, 2019 at 10:28 AM Pallantla Poornima <
> > pallantlax.poornima@intel.com> wrote:
> >
> >> 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>
> >> ---
> >> v2: updated title as suggested.
> >> ---
> >> 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..f63def5ef 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);
> >> + strlcat(port_name, dev->device.name, 100);
> >>
> >>
> >> 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) {
> >> --
> >> 2.17.2
> >>
> >>
> > I got a compilation error when applying this patch: strlcat can not be
> > found.
> >
> > I guess this patch requires to check for system libraries versions.
> >
>
> Hi Alejandro,
>
>
Hi Ferruh,
> Linux doesn't have the 'strlcat' but there is DPDK implementation of it,
> comes
> with '#include <rte_string_fns.h>' header which is already included in
> this file.
>
> 'strlcat' support is added in this release, 19.05, can you be using an old
> code?
> Can you please double check the build with the latest code?
>
>
I have tried again with tip DPDK and it works fine. I would say I used also
tip the first time, but anyway, it compiles now without problem.
I have also performed some basic tests and it is all fine.
So:
Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Tested-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Thanks!
> Thanks,
> ferruh
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH v2] net/nfp: fix possible buffer overflow
2019-03-21 10:10 ` Alejandro Lucero
2019-03-21 10:10 ` Alejandro Lucero
@ 2019-03-21 17:03 ` Ferruh Yigit
2019-03-21 17:03 ` Ferruh Yigit
1 sibling, 1 reply; 8+ messages in thread
From: Ferruh Yigit @ 2019-03-21 17:03 UTC (permalink / raw)
To: Alejandro Lucero; +Cc: Pallantla Poornima, dev, reshma.pattan, dpdk stable
On 3/21/2019 10:10 AM, Alejandro Lucero wrote:
>
>
> On Tue, Mar 19, 2019 at 5:43 PM Ferruh Yigit <ferruh.yigit@intel.com
> <mailto:ferruh.yigit@intel.com>> wrote:
>
> On 3/12/2019 9:56 AM, Alejandro Lucero wrote:
> > On Fri, Mar 8, 2019 at 10:28 AM Pallantla Poornima <
> > pallantlax.poornima@intel.com <mailto:pallantlax.poornima@intel.com>> wrote:
> >
> >> 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 <mailto:stable@dpdk.org>
> >>
> >> Signed-off-by: Pallantla Poornima <pallantlax.poornima@intel.com>
>
> Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> Tested-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [dpdk-stable] [PATCH v2] net/nfp: fix possible buffer overflow
2019-03-21 17:03 ` Ferruh Yigit
@ 2019-03-21 17:03 ` Ferruh Yigit
0 siblings, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2019-03-21 17:03 UTC (permalink / raw)
To: Alejandro Lucero; +Cc: Pallantla Poornima, dev, reshma.pattan, dpdk stable
On 3/21/2019 10:10 AM, Alejandro Lucero wrote:
>
>
> On Tue, Mar 19, 2019 at 5:43 PM Ferruh Yigit <ferruh.yigit@intel.com
> <mailto:ferruh.yigit@intel.com>> wrote:
>
> On 3/12/2019 9:56 AM, Alejandro Lucero wrote:
> > On Fri, Mar 8, 2019 at 10:28 AM Pallantla Poornima <
> > pallantlax.poornima@intel.com <mailto:pallantlax.poornima@intel.com>> wrote:
> >
> >> 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 <mailto:stable@dpdk.org>
> >>
> >> Signed-off-by: Pallantla Poornima <pallantlax.poornima@intel.com>
>
> Acked-by: Alejandro Lucero <alejandro.lucero@netronome.com>
> Tested-by: Alejandro Lucero <alejandro.lucero@netronome.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-03-21 17:04 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-08 10:28 [dpdk-dev] [PATCH v2] net/nfp: fix possible buffer overflow Pallantla Poornima
2019-03-12 9:56 ` Alejandro Lucero
2019-03-19 17:43 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2019-03-19 17:43 ` Ferruh Yigit
2019-03-21 10:10 ` Alejandro Lucero
2019-03-21 10:10 ` Alejandro Lucero
2019-03-21 17:03 ` Ferruh Yigit
2019-03-21 17:03 ` Ferruh Yigit
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).