>> 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@xxxxxxxxxxx> >> --- >> .../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