Re: [dpdk-dev] [dpdk-stable] [PATCH v2] net/nfp: fix possible buffer overflow

[Ferruh Yigit <ferruh.yigit@intel.com>]

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