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
