linux-kernel - Re: [EXT] Re: [PATCH] crypto: octeontx2: fix potential null pointer access

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 27 May 2022 13:04:02 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Shijith Thotton <sthotton@...vell.com>
Cc:     Arnaud Ebalard <arno@...isbad.org>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Boris Brezillon <bbrezillon@...nel.org>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        Jerin Jacob Kollanukkaran <jerinj@...vell.com>,
        Sunil Kovvuri Goutham <sgoutham@...vell.com>,
        Srujana Challa <schalla@...vell.com>,
        "David S. Miller" <davem@...emloft.net>,
        Harman Kalra <hkalra@...vell.com>,
        Yang Yingliang <yangyingliang@...wei.com>,
        Kees Cook <keescook@...omium.org>,
        Jiapeng Chong <jiapeng.chong@...ux.alibaba.com>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [EXT] Re: [PATCH] crypto: octeontx2: fix potential null pointer
 access

On Fri, May 27, 2022 at 09:40:16AM +0000, Shijith Thotton wrote:
> >> @@ -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.

No, it cannot fail.

I don't care if you add pointless NULL checks to make the static checker
happy, but it's important to understand what the code is doing.

drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c
   701  static int otx2_cptpf_probe(struct pci_dev *pdev,
   702                              const struct pci_device_id *ent)
   703  {
   704          struct device *dev = &pdev->dev;
   705          struct otx2_cptpf_dev *cptpf;
   706          int err;
   707  
   708          cptpf = devm_kzalloc(dev, sizeof(*cptpf), GFP_KERNEL);
   709          if (!cptpf)
   710                  return -ENOMEM;
   711  
   712          err = pcim_enable_device(pdev);
   713          if (err) {
   714                  dev_err(dev, "Failed to enable PCI device\n");
   715                  goto clear_drvdata;
   716          }
   717  
   718          err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(48));
   719          if (err) {
   720                  dev_err(dev, "Unable to get usable DMA configuration\n");
   721                  goto clear_drvdata;
   722          }
   723          /* Map PF's configuration registers */
   724          err = pcim_iomap_regions_request_all(pdev, 1 << PCI_PF_REG_BAR_NUM,
   725                                               OTX2_CPT_DRV_NAME);

The pcim_iomap_table() is allocated here inside the pcim_iomap_regions()
function.

   726          if (err) {
   727                  dev_err(dev, "Couldn't get PCI resources 0x%x\n", err);
   728                  goto clear_drvdata;
   729          }
   730          pci_set_master(pdev);
   731          pci_set_drvdata(pdev, cptpf);
   732          cptpf->pdev = pdev;
   733  
   734          cptpf->reg_base = pcim_iomap_table(pdev)[PCI_PF_REG_BAR_NUM];

It cannot fail here.  It is not allocated here.  We just look it up and
use it.

   735  
   736          /* Check if AF driver is up, otherwise defer probe */
   737          err = cpt_is_pf_usable(cptpf);
   738          if (err)
   739                  goto clear_drvdata;

regards,
dan carpenter