On Fri, May 27, 2022 at 01:27:56PM +0530, Shijith Thotton wrote:
> 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."
> + 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
