* [PATCH] PCI: endpoint: Fix NULL pointer dereference for ->get_features() [not found] <CGME20200311103443epcas5p2e97b8f3a8e52dc6f02eb551e0c97f132@epcas5p2.samsung.com> @ 2020-03-11 10:28 ` Shradha Todi 2020-03-19 13:36 ` Sriram Dash 0 siblings, 1 reply; 6+ messages in thread From: Shradha Todi @ 2020-03-11 10:28 UTC (permalink / raw) To: linux-pci, linux-kernel Cc: kishon, lorenzo.pieralisi, bhelgaas, pankaj.dubey, Shradha Todi, Sriram Dash get_features ops of pci_epc_ops may return NULL, causing NULL pointer dereference in pci_epf_test_bind function. Let us add a check for pci_epc_feature pointer in pci_epf_test_bind before we access it to avoid any such NULL pointer dereference and return -ENOTSUPP in case pci_epc_feature is not found. Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com> Signed-off-by: Sriram Dash <sriram.dash@samsung.com> Signed-off-by: Shradha Todi <shradha.t@samsung.com> --- drivers/pci/endpoint/functions/pci-epf-test.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c index c9121b1b9fa9..af4537a487bf 100644 --- a/drivers/pci/endpoint/functions/pci-epf-test.c +++ b/drivers/pci/endpoint/functions/pci-epf-test.c @@ -510,14 +510,17 @@ static int pci_epf_test_bind(struct pci_epf *epf) return -EINVAL; epc_features = pci_epc_get_features(epc, epf->func_no); - if (epc_features) { - linkup_notifier = epc_features->linkup_notifier; - msix_capable = epc_features->msix_capable; - msi_capable = epc_features->msi_capable; - test_reg_bar = pci_epc_get_first_free_bar(epc_features); - pci_epf_configure_bar(epf, epc_features); + if (!epc_features) { + dev_err(dev, "epc_features not implemented\n"); + return -ENOTSUPP; } + linkup_notifier = epc_features->linkup_notifier; + msix_capable = epc_features->msix_capable; + msi_capable = epc_features->msi_capable; + test_reg_bar = pci_epc_get_first_free_bar(epc_features); + pci_epf_configure_bar(epf, epc_features); + epf_test->test_reg_bar = test_reg_bar; epf_test->epc_features = epc_features; -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH] PCI: endpoint: Fix NULL pointer dereference for ->get_features() 2020-03-11 10:28 ` [PATCH] PCI: endpoint: Fix NULL pointer dereference for ->get_features() Shradha Todi @ 2020-03-19 13:36 ` Sriram Dash 2020-03-20 10:20 ` Kishon Vijay Abraham I 0 siblings, 1 reply; 6+ messages in thread From: Sriram Dash @ 2020-03-19 13:36 UTC (permalink / raw) To: 'Shradha Todi', kishon Cc: lorenzo.pieralisi, bhelgaas, pankaj.dubey, linux-pci, linux-kernel > From: Shradha Todi <shradha.t@samsung.com> > Subject: [PATCH] PCI: endpoint: Fix NULL pointer dereference for - > >get_features() > > get_features ops of pci_epc_ops may return NULL, causing NULL pointer > dereference in pci_epf_test_bind function. Let us add a check for > pci_epc_feature pointer in pci_epf_test_bind before we access it to avoid any > such NULL pointer dereference and return -ENOTSUPP in case pci_epc_feature > is not found. > > Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com> > Signed-off-by: Sriram Dash <sriram.dash@samsung.com> > Signed-off-by: Shradha Todi <shradha.t@samsung.com> > --- Hi Kishon, Any update on this? > drivers/pci/endpoint/functions/pci-epf-test.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c > b/drivers/pci/endpoint/functions/pci-epf-test.c > index c9121b1b9fa9..af4537a487bf 100644 > --- a/drivers/pci/endpoint/functions/pci-epf-test.c > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c > @@ -510,14 +510,17 @@ static int pci_epf_test_bind(struct pci_epf *epf) > return -EINVAL; > > epc_features = pci_epc_get_features(epc, epf->func_no); > - if (epc_features) { > - linkup_notifier = epc_features->linkup_notifier; > - msix_capable = epc_features->msix_capable; > - msi_capable = epc_features->msi_capable; > - test_reg_bar = pci_epc_get_first_free_bar(epc_features); > - pci_epf_configure_bar(epf, epc_features); > + if (!epc_features) { > + dev_err(dev, "epc_features not implemented\n"); > + return -ENOTSUPP; > } > > + linkup_notifier = epc_features->linkup_notifier; > + msix_capable = epc_features->msix_capable; > + msi_capable = epc_features->msi_capable; > + test_reg_bar = pci_epc_get_first_free_bar(epc_features); > + pci_epf_configure_bar(epf, epc_features); > + > epf_test->test_reg_bar = test_reg_bar; > epf_test->epc_features = epc_features; > > -- > 2.17.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI: endpoint: Fix NULL pointer dereference for ->get_features() 2020-03-19 13:36 ` Sriram Dash @ 2020-03-20 10:20 ` Kishon Vijay Abraham I 2020-04-02 14:31 ` Sriram Dash 0 siblings, 1 reply; 6+ messages in thread From: Kishon Vijay Abraham I @ 2020-03-20 10:20 UTC (permalink / raw) To: Sriram Dash, 'Shradha Todi' Cc: lorenzo.pieralisi, bhelgaas, pankaj.dubey, linux-pci, linux-kernel Hi Sriram, On 3/19/2020 7:06 PM, Sriram Dash wrote: >> From: Shradha Todi <shradha.t@samsung.com> >> Subject: [PATCH] PCI: endpoint: Fix NULL pointer dereference for - >>> get_features() >> >> get_features ops of pci_epc_ops may return NULL, causing NULL pointer >> dereference in pci_epf_test_bind function. Let us add a check for >> pci_epc_feature pointer in pci_epf_test_bind before we access it to avoid any >> such NULL pointer dereference and return -ENOTSUPP in case pci_epc_feature >> is not found. >> >> Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com> >> Signed-off-by: Sriram Dash <sriram.dash@samsung.com> >> Signed-off-by: Shradha Todi <shradha.t@samsung.com> >> --- > > Hi Kishon, > > Any update on this? Don't we access epc_features only after checking if epc_features is not NULL in pci_epf_test_bind() function? However we are accessing epc_features in multiple other functions all over pci-epf-test. So the patch itself is correct though the commit log has to be fixed. You should also check if all the endpoint controller drivers existing currently provides epc_features. Thanks Kishon > > >> drivers/pci/endpoint/functions/pci-epf-test.c | 15 +++++++++------ >> 1 file changed, 9 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c >> b/drivers/pci/endpoint/functions/pci-epf-test.c >> index c9121b1b9fa9..af4537a487bf 100644 >> --- a/drivers/pci/endpoint/functions/pci-epf-test.c >> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c >> @@ -510,14 +510,17 @@ static int pci_epf_test_bind(struct pci_epf *epf) >> return -EINVAL; >> >> epc_features = pci_epc_get_features(epc, epf->func_no); >> - if (epc_features) { >> - linkup_notifier = epc_features->linkup_notifier; >> - msix_capable = epc_features->msix_capable; >> - msi_capable = epc_features->msi_capable; >> - test_reg_bar = pci_epc_get_first_free_bar(epc_features); >> - pci_epf_configure_bar(epf, epc_features); >> + if (!epc_features) { >> + dev_err(dev, "epc_features not implemented\n"); >> + return -ENOTSUPP; >> } >> >> + linkup_notifier = epc_features->linkup_notifier; >> + msix_capable = epc_features->msix_capable; >> + msi_capable = epc_features->msi_capable; >> + test_reg_bar = pci_epc_get_first_free_bar(epc_features); >> + pci_epf_configure_bar(epf, epc_features); >> + >> epf_test->test_reg_bar = test_reg_bar; >> epf_test->epc_features = epc_features; >> >> -- >> 2.17.1 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] PCI: endpoint: Fix NULL pointer dereference for ->get_features() 2020-03-20 10:20 ` Kishon Vijay Abraham I @ 2020-04-02 14:31 ` Sriram Dash 2020-07-06 11:17 ` Lorenzo Pieralisi 0 siblings, 1 reply; 6+ messages in thread From: Sriram Dash @ 2020-04-02 14:31 UTC (permalink / raw) To: 'Kishon Vijay Abraham I', 'Shradha Todi' Cc: lorenzo.pieralisi, bhelgaas, pankaj.dubey, linux-pci, linux-kernel > From: Kishon Vijay Abraham I <kishon@ti.com> > Subject: Re: [PATCH] PCI: endpoint: Fix NULL pointer dereference for - > >get_features() > > Hi Sriram, > > On 3/19/2020 7:06 PM, Sriram Dash wrote: > >> From: Shradha Todi <shradha.t@samsung.com> > >> Subject: [PATCH] PCI: endpoint: Fix NULL pointer dereference for - > >>> get_features() > >> > >> get_features ops of pci_epc_ops may return NULL, causing NULL pointer > >> dereference in pci_epf_test_bind function. Let us add a check for > >> pci_epc_feature pointer in pci_epf_test_bind before we access it to > >> avoid any such NULL pointer dereference and return -ENOTSUPP in case > >> pci_epc_feature is not found. > >> > >> Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com> > >> Signed-off-by: Sriram Dash <sriram.dash@samsung.com> > >> Signed-off-by: Shradha Todi <shradha.t@samsung.com> > >> --- > > > > Hi Kishon, > > > > Any update on this? > > Don't we access epc_features only after checking if epc_features is not NULL in > pci_epf_test_bind() function? However we are accessing epc_features in > multiple other functions all over pci-epf-test. We access the epc_feature after checking the NULL condition in the bind function. However, we do not stop if the epc_feature is NULL and proceed for allocation in the pci_epf_test_alloc_space function, for example. During this allocation, we do not check for NULL condition for epc_feature and hence, if any controller driver is not providing the epc features, it will panic accessing epc_features. > > So the patch itself is correct though the commit log has to be fixed. You should > also check if all the endpoint controller drivers existing currently provides > epc_features. At the moment, there is no issue for existing controller drivers as I can see almost all drivers are providing epc_features. But, this is not a mandatory feature and some controller drivers may not have epc_features implemented, may be in the near future. But because we are dealing with the configfs, the application need not bother about the driver details underneath. IMO, the code should be fixed regardless and should not cause panic in any case. > > Thanks > Kishon > > > > > >> drivers/pci/endpoint/functions/pci-epf-test.c | 15 +++++++++------ > >> 1 file changed, 9 insertions(+), 6 deletions(-) > >> > >> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c > >> b/drivers/pci/endpoint/functions/pci-epf-test.c > >> index c9121b1b9fa9..af4537a487bf 100644 > >> --- a/drivers/pci/endpoint/functions/pci-epf-test.c > >> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c > >> @@ -510,14 +510,17 @@ static int pci_epf_test_bind(struct pci_epf *epf) > >> return -EINVAL; > >> > >> epc_features = pci_epc_get_features(epc, epf->func_no); > >> - if (epc_features) { > >> - linkup_notifier = epc_features->linkup_notifier; > >> - msix_capable = epc_features->msix_capable; > >> - msi_capable = epc_features->msi_capable; > >> - test_reg_bar = pci_epc_get_first_free_bar(epc_features); > >> - pci_epf_configure_bar(epf, epc_features); > >> + if (!epc_features) { > >> + dev_err(dev, "epc_features not implemented\n"); > >> + return -ENOTSUPP; > >> } > >> > >> + linkup_notifier = epc_features->linkup_notifier; > >> + msix_capable = epc_features->msix_capable; > >> + msi_capable = epc_features->msi_capable; > >> + test_reg_bar = pci_epc_get_first_free_bar(epc_features); > >> + pci_epf_configure_bar(epf, epc_features); > >> + > >> epf_test->test_reg_bar = test_reg_bar; > >> epf_test->epc_features = epc_features; > >> > >> -- > >> 2.17.1 > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PCI: endpoint: Fix NULL pointer dereference for ->get_features() 2020-04-02 14:31 ` Sriram Dash @ 2020-07-06 11:17 ` Lorenzo Pieralisi 2020-07-06 13:17 ` Sriram Dash 0 siblings, 1 reply; 6+ messages in thread From: Lorenzo Pieralisi @ 2020-07-06 11:17 UTC (permalink / raw) To: Sriram Dash Cc: 'Kishon Vijay Abraham I', 'Shradha Todi', bhelgaas, pankaj.dubey, linux-pci, linux-kernel On Thu, Apr 02, 2020 at 08:01:59PM +0530, Sriram Dash wrote: [...] > > So the patch itself is correct though the commit log has to be fixed. You should > > also check if all the endpoint controller drivers existing currently provides > > epc_features. > > At the moment, there is no issue for existing controller drivers as I > can see almost all drivers are providing epc_features. But, this is > not a mandatory feature and some controller drivers may not have > epc_features implemented, may be in the near future. But because we > are dealing with the configfs, the application need not bother about > the driver details underneath. > > IMO, the code should be fixed regardless and should not cause panic in > any case. What's this patch status please ? Thanks, Lorenzo > > Thanks > > Kishon > > > > > > > > >> drivers/pci/endpoint/functions/pci-epf-test.c | 15 +++++++++------ > > >> 1 file changed, 9 insertions(+), 6 deletions(-) > > >> > > >> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c > > >> b/drivers/pci/endpoint/functions/pci-epf-test.c > > >> index c9121b1b9fa9..af4537a487bf 100644 > > >> --- a/drivers/pci/endpoint/functions/pci-epf-test.c > > >> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c > > >> @@ -510,14 +510,17 @@ static int pci_epf_test_bind(struct pci_epf *epf) > > >> return -EINVAL; > > >> > > >> epc_features = pci_epc_get_features(epc, epf->func_no); > > >> - if (epc_features) { > > >> - linkup_notifier = epc_features->linkup_notifier; > > >> - msix_capable = epc_features->msix_capable; > > >> - msi_capable = epc_features->msi_capable; > > >> - test_reg_bar = pci_epc_get_first_free_bar(epc_features); > > >> - pci_epf_configure_bar(epf, epc_features); > > >> + if (!epc_features) { > > >> + dev_err(dev, "epc_features not implemented\n"); > > >> + return -ENOTSUPP; > > >> } > > >> > > >> + linkup_notifier = epc_features->linkup_notifier; > > >> + msix_capable = epc_features->msix_capable; > > >> + msi_capable = epc_features->msi_capable; > > >> + test_reg_bar = pci_epc_get_first_free_bar(epc_features); > > >> + pci_epf_configure_bar(epf, epc_features); > > >> + > > >> epf_test->test_reg_bar = test_reg_bar; > > >> epf_test->epc_features = epc_features; > > >> > > >> -- > > >> 2.17.1 > > > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] PCI: endpoint: Fix NULL pointer dereference for ->get_features() 2020-07-06 11:17 ` Lorenzo Pieralisi @ 2020-07-06 13:17 ` Sriram Dash 0 siblings, 0 replies; 6+ messages in thread From: Sriram Dash @ 2020-07-06 13:17 UTC (permalink / raw) To: 'Lorenzo Pieralisi' Cc: 'Kishon Vijay Abraham I', 'Shradha Todi', bhelgaas, pankaj.dubey, linux-pci, linux-kernel > From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Sent: 06 July 2020 16:47 > Subject: Re: [PATCH] PCI: endpoint: Fix NULL pointer dereference for - > >get_features() > > On Thu, Apr 02, 2020 at 08:01:59PM +0530, Sriram Dash wrote: > > [...] > > > > So the patch itself is correct though the commit log has to be > > > fixed. You should also check if all the endpoint controller drivers > > > existing currently provides epc_features. > > > > At the moment, there is no issue for existing controller drivers as I > > can see almost all drivers are providing epc_features. But, this is > > not a mandatory feature and some controller drivers may not have > > epc_features implemented, may be in the near future. But because we > > are dealing with the configfs, the application need not bother about > > the driver details underneath. > > > > IMO, the code should be fixed regardless and should not cause panic in > > any case. > > What's this patch status please ? > Its not in the mainline tree as of now. However, we feel its important for the drivers not using epc_features. > Thanks, > Lorenzo > > > > Thanks > > > Kishon > > > > > > > > > > > >> drivers/pci/endpoint/functions/pci-epf-test.c | 15 > > > >> +++++++++------ > > > >> 1 file changed, 9 insertions(+), 6 deletions(-) > > > >> > > > >> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c > > > >> b/drivers/pci/endpoint/functions/pci-epf-test.c > > > >> index c9121b1b9fa9..af4537a487bf 100644 > > > >> --- a/drivers/pci/endpoint/functions/pci-epf-test.c > > > >> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c > > > >> @@ -510,14 +510,17 @@ static int pci_epf_test_bind(struct pci_epf *epf) > > > >> return -EINVAL; > > > >> > > > >> epc_features = pci_epc_get_features(epc, epf->func_no); > > > >> - if (epc_features) { > > > >> - linkup_notifier = epc_features->linkup_notifier; > > > >> - msix_capable = epc_features->msix_capable; > > > >> - msi_capable = epc_features->msi_capable; > > > >> - test_reg_bar = pci_epc_get_first_free_bar(epc_features); > > > >> - pci_epf_configure_bar(epf, epc_features); > > > >> + if (!epc_features) { > > > >> + dev_err(dev, "epc_features not implemented\n"); > > > >> + return -ENOTSUPP; > > > >> } > > > >> > > > >> + linkup_notifier = epc_features->linkup_notifier; > > > >> + msix_capable = epc_features->msix_capable; > > > >> + msi_capable = epc_features->msi_capable; > > > >> + test_reg_bar = pci_epc_get_first_free_bar(epc_features); > > > >> + pci_epf_configure_bar(epf, epc_features); > > > >> + > > > >> epf_test->test_reg_bar = test_reg_bar; > > > >> epf_test->epc_features = epc_features; > > > >> > > > >> -- > > > >> 2.17.1 > > > > > > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-06 13:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20200311103443epcas5p2e97b8f3a8e52dc6f02eb551e0c97f132@epcas5p2.samsung.com>
2020-03-11 10:28 ` [PATCH] PCI: endpoint: Fix NULL pointer dereference for ->get_features() Shradha Todi
2020-03-19 13:36 ` Sriram Dash
2020-03-20 10:20 ` Kishon Vijay Abraham I
2020-04-02 14:31 ` Sriram Dash
2020-07-06 11:17 ` Lorenzo Pieralisi
2020-07-06 13:17 ` Sriram Dash
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).