All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Cassel <cassel@kernel.org>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: manivannan.sadhasivam@linaro.org, linux-pci@vger.kernel.org
Subject: Re: [bug report] PCI: endpoint: Remove "core_init_notifier" flag
Date: Wed, 17 Apr 2024 17:38:05 +0200	[thread overview]
Message-ID: <Zh_s3WdFURyll54l@ryzen> (raw)
In-Reply-To: <024b5826-7180-4076-ae08-57d2584cca3f@moroto.mountain>

On Wed, Apr 17, 2024 at 03:47:48PM +0300, Dan Carpenter wrote:
> Hello Manivannan Sadhasivam,
> 
> Commit a01e7214bef9 ("PCI: endpoint: Remove "core_init_notifier"
> flag") from Mar 27, 2024 (linux-next), leads to the following Smatch
> static checker warning:
> 
> 	drivers/pci/endpoint/functions/pci-epf-test.c:784 pci_epf_test_core_init()
> 	error: we previously assumed 'epc_features' could be null (see line 747)
> 
> drivers/pci/endpoint/functions/pci-epf-test.c
>     734 static int pci_epf_test_core_init(struct pci_epf *epf)
>     735 {
>     736         struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>     737         struct pci_epf_header *header = epf->header;
>     738         const struct pci_epc_features *epc_features;
>     739         struct pci_epc *epc = epf->epc;
>     740         struct device *dev = &epf->dev;
>     741         bool linkup_notifier = false;
>     742         bool msix_capable = false;
>     743         bool msi_capable = true;
>     744         int ret;

We check pci_epc_get_features() in ->bind(), which comes before ->core_init()
so in practice, this shouldn't be able to be NULL here.


>     745 
>     746         epc_features = pci_epc_get_features(epc, epf->func_no, epf->vfunc_no);
>     747         if (epc_features) {
>                     ^^^^^^^^^^^^
> epc_features can be NULL

We should probably just chge this to:

"""
if (!epc_features)
	return some_error;

msix_capable = epc_features->msix_capable;
msi_capable = epc_features->msi_capable;
"""

Such that we don't need another check further down for linkup_notifier.


Kind regards,
Niklas

> 
>     748                 msix_capable = epc_features->msix_capable;
>     749                 msi_capable = epc_features->msi_capable;
>     750         }
>     751 
>     752         if (epf->vfunc_no <= 1) {
>     753                 ret = pci_epc_write_header(epc, epf->func_no, epf->vfunc_no, header);
>     754                 if (ret) {
>     755                         dev_err(dev, "Configuration header write failed\n");
>     756                         return ret;
>     757                 }
>     758         }
>     759 
>     760         ret = pci_epf_test_set_bar(epf);
>     761         if (ret)
>     762                 return ret;
>     763 
>     764         if (msi_capable) {
>     765                 ret = pci_epc_set_msi(epc, epf->func_no, epf->vfunc_no,
>     766                                       epf->msi_interrupts);
>     767                 if (ret) {
>     768                         dev_err(dev, "MSI configuration failed\n");
>     769                         return ret;
>     770                 }
>     771         }
>     772 
>     773         if (msix_capable) {
>     774                 ret = pci_epc_set_msix(epc, epf->func_no, epf->vfunc_no,
>     775                                        epf->msix_interrupts,
>     776                                        epf_test->test_reg_bar,
>     777                                        epf_test->msix_table_offset);
>     778                 if (ret) {
>     779                         dev_err(dev, "MSI-X configuration failed\n");
>     780                         return ret;
>     781                 }
>     782         }
>     783 
> --> 784         linkup_notifier = epc_features->linkup_notifier;
>                                   ^^^^^^^^^^^^^^
> Unchecked dereference.
> 
>     785         if (!linkup_notifier)
>     786                 queue_work(kpcitest_workqueue, &epf_test->cmd_handler.work);
>     787 
>     788         return 0;
>     789 }
> 
> regards,
> dan carpenter

  reply	other threads:[~2024-04-17 15:38 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-17 12:47 [bug report] PCI: endpoint: Remove "core_init_notifier" flag Dan Carpenter
2024-04-17 15:38 ` Niklas Cassel [this message]
2024-04-17 15:58   ` Manivannan Sadhasivam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Zh_s3WdFURyll54l@ryzen \
    --to=cassel@kernel.org \
    --cc=dan.carpenter@linaro.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=manivannan.sadhasivam@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.