| Message ID | 68e0f6a4-fd57-45d0-945b-0876f2c8cb86@moroto.mountain |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: endpoint: fix a couple error handling bugs | expand |
On Mon, 10 Jun 2024, Dan Carpenter wrote: > Smatch complains about inconsistent NULL checking in vpci_scan_bus(): > > drivers/pci/endpoint/functions/pci-epf-vntb.c:1024 vpci_scan_bus() > error: we previously assumed 'vpci_bus' could be null (see line 1021) > > Instead of printing an error message and then crashing we should return > an error code and clean up. Also the NULL check is reversed so it > prints an error for success instead of failure. > > Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > drivers/pci/endpoint/functions/pci-epf-vntb.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c > index 8e779eecd62d..7f05a44e9a9f 100644 > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c > @@ -1018,8 +1018,10 @@ static int vpci_scan_bus(void *sysdata) > struct epf_ntb *ndev = sysdata; > > vpci_bus = pci_scan_bus(ndev->vbus_number, &vpci_ops, sysdata); > - if (vpci_bus) > - pr_err("create pci bus\n"); > + if (!vpci_bus) { > + pr_err("create pci bus failed\n"); > + return -EINVAL; > + } > > pci_bus_add_devices(vpci_bus); > > @@ -1338,10 +1340,14 @@ static int epf_ntb_bind(struct pci_epf *epf) > goto err_bar_alloc; > } > > - vpci_scan_bus(ntb); > + ret = vpci_scan_bus(ntb); > + if (ret) > + goto err_unregister; > > return 0; > > +err_unregister: > + pci_unregister_driver(&vntb_pci_driver); > err_bar_alloc: > epf_ntb_config_spad_bar_free(ntb); Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c index 8e779eecd62d..7f05a44e9a9f 100644 --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c @@ -1018,8 +1018,10 @@ static int vpci_scan_bus(void *sysdata) struct epf_ntb *ndev = sysdata; vpci_bus = pci_scan_bus(ndev->vbus_number, &vpci_ops, sysdata); - if (vpci_bus) - pr_err("create pci bus\n"); + if (!vpci_bus) { + pr_err("create pci bus failed\n"); + return -EINVAL; + } pci_bus_add_devices(vpci_bus); @@ -1338,10 +1340,14 @@ static int epf_ntb_bind(struct pci_epf *epf) goto err_bar_alloc; } - vpci_scan_bus(ntb); + ret = vpci_scan_bus(ntb); + if (ret) + goto err_unregister; return 0; +err_unregister: + pci_unregister_driver(&vntb_pci_driver); err_bar_alloc: epf_ntb_config_spad_bar_free(ntb);
Smatch complains about inconsistent NULL checking in vpci_scan_bus(): drivers/pci/endpoint/functions/pci-epf-vntb.c:1024 vpci_scan_bus() error: we previously assumed 'vpci_bus' could be null (see line 1021) Instead of printing an error message and then crashing we should return an error code and clean up. Also the NULL check is reversed so it prints an error for success instead of failure. Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- drivers/pci/endpoint/functions/pci-epf-vntb.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)