| Message ID | 20240418-pci-epf-test-fix-v2-1-eacd54831444@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | PCI: endpoint: pci-epf-test: Cleanup the usage of 'pci_epf_test::epc_features' | expand |
On Thu, Apr 18, 2024 at 01:29:58PM +0530, Manivannan Sadhasivam wrote: > Instead of getting the epc_features from pci_epc_get_features() API, use > the cached pci_epf_test::epc_features value to avoid the NULL check. Since > the NULL check is already performed in pci_epf_test_bind(), having one more > check in pci_epf_test_core_init() is redundant and it is not possible to > hit the NULL pointer dereference. > > Also with commit a01e7214bef9 ("PCI: endpoint: Remove "core_init_notifier" > flag"), 'epc_features' got dereferenced without the NULL check, leading to > the following false positive smatch 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) > > So let's remove the redundant NULL check and also use the epc_features:: > {msix_capable/msi_capable} flags directly to avoid local variables. > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Closes: https://lore.kernel.org/linux-pci/024b5826-7180-4076-ae08-57d2584cca3f@moroto.mountain/ > Fixes: 5e50ee27d4a5 ("PCI: pci-epf-test: Add support to defer core initialization") > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- Reviewed-by: Niklas Cassel <cassel@kernel.org>
On Thu, Apr 18, 2024 at 01:29:58PM +0530, Manivannan Sadhasivam wrote: > Instead of getting the epc_features from pci_epc_get_features() API, use > the cached pci_epf_test::epc_features value to avoid the NULL check. Since > the NULL check is already performed in pci_epf_test_bind(), having one more > check in pci_epf_test_core_init() is redundant and it is not possible to > hit the NULL pointer dereference. > > Also with commit a01e7214bef9 ("PCI: endpoint: Remove "core_init_notifier" > flag"), 'epc_features' got dereferenced without the NULL check, leading to > the following false positive smatch 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) > > So let's remove the redundant NULL check and also use the epc_features:: > {msix_capable/msi_capable} flags directly to avoid local variables. > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Closes: https://lore.kernel.org/linux-pci/024b5826-7180-4076-ae08-57d2584cca3f@moroto.mountain/ > Fixes: 5e50ee27d4a5 ("PCI: pci-epf-test: Add support to defer core initialization") > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Reviewed-by: Frank Li <Frank.Li@nxp.com> > --- > drivers/pci/endpoint/functions/pci-epf-test.c | 14 +++----------- > 1 file changed, 3 insertions(+), 11 deletions(-) > > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c > index 977fb79c1567..546d2a27955c 100644 > --- a/drivers/pci/endpoint/functions/pci-epf-test.c > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c > @@ -735,20 +735,12 @@ static int pci_epf_test_core_init(struct pci_epf *epf) > { > struct pci_epf_test *epf_test = epf_get_drvdata(epf); > struct pci_epf_header *header = epf->header; > - const struct pci_epc_features *epc_features; > + const struct pci_epc_features *epc_features = epf_test->epc_features; > struct pci_epc *epc = epf->epc; > struct device *dev = &epf->dev; > bool linkup_notifier = false; > - bool msix_capable = false; > - bool msi_capable = true; > int ret; > > - epc_features = pci_epc_get_features(epc, epf->func_no, epf->vfunc_no); > - if (epc_features) { > - msix_capable = epc_features->msix_capable; > - msi_capable = epc_features->msi_capable; > - } > - > if (epf->vfunc_no <= 1) { > ret = pci_epc_write_header(epc, epf->func_no, epf->vfunc_no, header); > if (ret) { > @@ -761,7 +753,7 @@ static int pci_epf_test_core_init(struct pci_epf *epf) > if (ret) > return ret; > > - if (msi_capable) { > + if (epc_features->msi_capable) { > ret = pci_epc_set_msi(epc, epf->func_no, epf->vfunc_no, > epf->msi_interrupts); > if (ret) { > @@ -770,7 +762,7 @@ static int pci_epf_test_core_init(struct pci_epf *epf) > } > } > > - if (msix_capable) { > + if (epc_features->msix_capable) { > ret = pci_epc_set_msix(epc, epf->func_no, epf->vfunc_no, > epf->msix_interrupts, > epf_test->test_reg_bar, > > -- > 2.25.1 >
Hello, > Instead of getting the epc_features from pci_epc_get_features() API, use > the cached pci_epf_test::epc_features value to avoid the NULL check. Since > the NULL check is already performed in pci_epf_test_bind(), having one more > check in pci_epf_test_core_init() is redundant and it is not possible to > hit the NULL pointer dereference. > > Also with commit a01e7214bef9 ("PCI: endpoint: Remove "core_init_notifier" > flag"), 'epc_features' got dereferenced without the NULL check, leading to > the following false positive smatch 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) > > So let's remove the redundant NULL check and also use the epc_features:: > {msix_capable/msi_capable} flags directly to avoid local variables. Applied to endpoint, thank you! [01/02] PCI: endpoint: pci-epf-test: Make use of cached 'epc_features' in pci_epf_test_core_init() https://git.kernel.org/pci/pci/c/3acb072c2433 [02/02] PCI: endpoint: pci-epf-test: Use 'msix_capable' flag directly in pci_epf_test_alloc_space() https://git.kernel.org/pci/pci/c/e79d1b1eb626 Krzysztof
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c index 977fb79c1567..546d2a27955c 100644 --- a/drivers/pci/endpoint/functions/pci-epf-test.c +++ b/drivers/pci/endpoint/functions/pci-epf-test.c @@ -735,20 +735,12 @@ static int pci_epf_test_core_init(struct pci_epf *epf) { struct pci_epf_test *epf_test = epf_get_drvdata(epf); struct pci_epf_header *header = epf->header; - const struct pci_epc_features *epc_features; + const struct pci_epc_features *epc_features = epf_test->epc_features; struct pci_epc *epc = epf->epc; struct device *dev = &epf->dev; bool linkup_notifier = false; - bool msix_capable = false; - bool msi_capable = true; int ret; - epc_features = pci_epc_get_features(epc, epf->func_no, epf->vfunc_no); - if (epc_features) { - msix_capable = epc_features->msix_capable; - msi_capable = epc_features->msi_capable; - } - if (epf->vfunc_no <= 1) { ret = pci_epc_write_header(epc, epf->func_no, epf->vfunc_no, header); if (ret) { @@ -761,7 +753,7 @@ static int pci_epf_test_core_init(struct pci_epf *epf) if (ret) return ret; - if (msi_capable) { + if (epc_features->msi_capable) { ret = pci_epc_set_msi(epc, epf->func_no, epf->vfunc_no, epf->msi_interrupts); if (ret) { @@ -770,7 +762,7 @@ static int pci_epf_test_core_init(struct pci_epf *epf) } } - if (msix_capable) { + if (epc_features->msix_capable) { ret = pci_epc_set_msix(epc, epf->func_no, epf->vfunc_no, epf->msix_interrupts, epf_test->test_reg_bar,
Instead of getting the epc_features from pci_epc_get_features() API, use the cached pci_epf_test::epc_features value to avoid the NULL check. Since the NULL check is already performed in pci_epf_test_bind(), having one more check in pci_epf_test_core_init() is redundant and it is not possible to hit the NULL pointer dereference. Also with commit a01e7214bef9 ("PCI: endpoint: Remove "core_init_notifier" flag"), 'epc_features' got dereferenced without the NULL check, leading to the following false positive smatch 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) So let's remove the redundant NULL check and also use the epc_features:: {msix_capable/msi_capable} flags directly to avoid local variables. Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Closes: https://lore.kernel.org/linux-pci/024b5826-7180-4076-ae08-57d2584cca3f@moroto.mountain/ Fixes: 5e50ee27d4a5 ("PCI: pci-epf-test: Add support to defer core initialization") Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> --- drivers/pci/endpoint/functions/pci-epf-test.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-)