lkml.org 
[lkml]   [2022]   [May]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [EXT] Re: [PATCH] crypto: octeontx2: fix potential null pointer access
Date
>> Added missing checks to avoid null pointer dereference.
>>
>> The patch fixes below issues reported by klocwork tool:
>
>Don't fix false positives to make a tool happy. Fix the tool. (Unless
>the patch makes the code simpler, then it's fine).
>
>> 1. Pointer 'pcim_iomap_table(pdev)' returned from call to function
>> 'pcim_iomap_table' at line 365 may be NULL and will be dereferenced
>> at line 365 in otx2_cptvf_main.c. Also there is a similar error on
>> line 734 in otx2_cptpf_main.c.
>> 2. Pointer 'strsep( &val, ":" )' returned from call to function 'strsep'
>> at line 1608 may be NULL and will be dereferenced at line 1608. Also
>> there are 2 similar errors on lines 1620, 1632 in otx2_cptpf_ucode.c.
>>
>> Signed-off-by: Shijith Thotton <sthotton@marvell.com>
>> ---
>> .../crypto/marvell/octeontx2/otx2_cptpf_main.c | 9 ++++++++-
>> .../marvell/octeontx2/otx2_cptpf_ucode.c | 18 +++++++++++++++---
>> .../crypto/marvell/octeontx2/otx2_cptvf_main.c | 9 ++++++++-
>> 3 files changed, 31 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c
>b/drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c
>> index a402ccfac557..ae57cee424f0 100644
>> --- a/drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c
>> +++ b/drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c
>> @@ -703,6 +703,7 @@ static int otx2_cptpf_probe(struct pci_dev *pdev,
>> {
>> struct device *dev = &pdev->dev;
>> struct otx2_cptpf_dev *cptpf;
>> + void __iomem * const *iomap;
>> int err;
>>
>> cptpf = devm_kzalloc(dev, sizeof(*cptpf), GFP_KERNEL);
>> @@ -731,7 +732,13 @@ static int otx2_cptpf_probe(struct pci_dev *pdev,
>> pci_set_drvdata(pdev, cptpf);
>> cptpf->pdev = pdev;
>>
>> - cptpf->reg_base = pcim_iomap_table(pdev)[PCI_PF_REG_BAR_NUM];
>> + iomap = pcim_iomap_table(pdev);
>
>I don't know if a check is required here or not... The comments to
>pcim_iomap_table() say it is, "guaranteed to succeed once allocated."
>

Will keep the check just to be safe, as allocation/kmalloc could fail.

>> + if (!iomap) {
>> + dev_err(dev, "Failed to get iomap table\n");
>> + err = -ENODEV;
>> + goto clear_drvdata;
>> + }
>> + cptpf->reg_base = iomap[PCI_PF_REG_BAR_NUM];
>>
>> /* Check if AF driver is up, otherwise defer probe */
>> err = cpt_is_pf_usable(cptpf);
>> diff --git a/drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c
>b/drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c
>> index 9cba2f714c7e..b91401929fc6 100644
>> --- a/drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c
>> +++ b/drivers/crypto/marvell/octeontx2/otx2_cptpf_ucode.c
>> @@ -1605,7 +1605,11 @@ int otx2_cpt_dl_custom_egrp_create(struct
>otx2_cptpf_dev *cptpf,
>> if (!strncasecmp(val, "se", 2) && strchr(val, ':')) {
> ^^^^^^^^^^^^^^^^
>We know it can't be NULL.
>
>> if (has_se || ucode_idx)
>> goto err_print;
>> - tmp = strim(strsep(&val, ":"));
>> + tmp = strsep(&val, ":");
>> + if (tmp != NULL)
>> + tmp = strim(tmp);
>> + else
>> + goto err_print;
>> if (!val)
>> goto err_print;
>> if (strlen(tmp) != 2)
>
>The rest is all the same. Likely or definitely false positives.
>
>regards,
>dan carpenter

\
 
 \ /
  Last update: 2022-05-27 11:42    [W:0.080 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site