* [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for ->get_features() [not found] <CGME20210112140234epcas5p4f97e9cf12e68df9fb55d1270bd14280c@epcas5p4.samsung.com> @ 2021-01-12 14:02 ` Shradha Todi 2021-01-13 5:05 ` Kishon Vijay Abraham I 2021-01-13 7:21 ` Leon Romanovsky 0 siblings, 2 replies; 7+ messages in thread From: Shradha Todi @ 2021-01-12 14:02 UTC (permalink / raw) To: linux-kernel, linux-pci Cc: bhelgaas, kishon, lorenzo.pieralisi, pankaj.dubey, sriram.dash, niyas.ahmed, p.rajanbabu, l.mehra, hari.tv, Shradha Todi 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. When the patch is not applied and EPC features is not implemented in the platform driver, we see the following dump due to kernel NULL pointer dereference. [ 105.135936] Call trace: [ 105.138363] pci_epf_test_bind+0xf4/0x388 [ 105.142354] pci_epf_bind+0x3c/0x80 [ 105.145817] pci_epc_epf_link+0xa8/0xcc [ 105.149632] configfs_symlink+0x1a4/0x48c [ 105.153616] vfs_symlink+0x104/0x184 [ 105.157169] do_symlinkat+0x80/0xd4 [ 105.160636] __arm64_sys_symlinkat+0x1c/0x24 [ 105.164885] el0_svc_common.constprop.3+0xb8/0x170 [ 105.169649] el0_svc_handler+0x70/0x88 [ 105.173377] el0_svc+0x8/0x640 [ 105.176411] Code: d2800581 b9403ab9 f9404ebb 8b394f60 (f9400400) [ 105.182478] ---[ end trace a438e3c5a24f9df0 ]--- Fixes: 2c04c5b8eef79 ("PCI: pci-epf-test: Use pci_epc_get_features() to get EPC features") 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 | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c index e4e51d8..1b30774 100644 --- a/drivers/pci/endpoint/functions/pci-epf-test.c +++ b/drivers/pci/endpoint/functions/pci-epf-test.c @@ -830,13 +830,16 @@ 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; - core_init_notifier = epc_features->core_init_notifier; - test_reg_bar = pci_epc_get_first_free_bar(epc_features); - pci_epf_configure_bar(epf, epc_features); + if (!epc_features) { + dev_err(&epf->dev, "epc_features not implemented\n"); + return -EOPNOTSUPP; } + linkup_notifier = epc_features->linkup_notifier; + core_init_notifier = epc_features->core_init_notifier; + 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.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for ->get_features() 2021-01-12 14:02 ` [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for ->get_features() Shradha Todi @ 2021-01-13 5:05 ` Kishon Vijay Abraham I 2021-01-13 7:21 ` Leon Romanovsky 1 sibling, 0 replies; 7+ messages in thread From: Kishon Vijay Abraham I @ 2021-01-13 5:05 UTC (permalink / raw) To: Shradha Todi, linux-kernel, linux-pci Cc: bhelgaas, lorenzo.pieralisi, pankaj.dubey, sriram.dash, niyas.ahmed, p.rajanbabu, l.mehra, hari.tv On 12/01/21 7:32 pm, Shradha Todi wrote: > 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. > > When the patch is not applied and EPC features is not implemented in the > platform driver, we see the following dump due to kernel NULL pointer > dereference. > > [ 105.135936] Call trace: > [ 105.138363] pci_epf_test_bind+0xf4/0x388 > [ 105.142354] pci_epf_bind+0x3c/0x80 > [ 105.145817] pci_epc_epf_link+0xa8/0xcc > [ 105.149632] configfs_symlink+0x1a4/0x48c > [ 105.153616] vfs_symlink+0x104/0x184 > [ 105.157169] do_symlinkat+0x80/0xd4 > [ 105.160636] __arm64_sys_symlinkat+0x1c/0x24 > [ 105.164885] el0_svc_common.constprop.3+0xb8/0x170 > [ 105.169649] el0_svc_handler+0x70/0x88 > [ 105.173377] el0_svc+0x8/0x640 > [ 105.176411] Code: d2800581 b9403ab9 f9404ebb 8b394f60 (f9400400) > [ 105.182478] ---[ end trace a438e3c5a24f9df0 ]--- > > Fixes: 2c04c5b8eef79 ("PCI: pci-epf-test: Use pci_epc_get_features() to get EPC features") > 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> Reviewed-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/pci/endpoint/functions/pci-epf-test.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c > index e4e51d8..1b30774 100644 > --- a/drivers/pci/endpoint/functions/pci-epf-test.c > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c > @@ -830,13 +830,16 @@ 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; > - core_init_notifier = epc_features->core_init_notifier; > - test_reg_bar = pci_epc_get_first_free_bar(epc_features); > - pci_epf_configure_bar(epf, epc_features); > + if (!epc_features) { > + dev_err(&epf->dev, "epc_features not implemented\n"); > + return -EOPNOTSUPP; > } > > + linkup_notifier = epc_features->linkup_notifier; > + core_init_notifier = epc_features->core_init_notifier; > + 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; > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for ->get_features() 2021-01-12 14:02 ` [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for ->get_features() Shradha Todi 2021-01-13 5:05 ` Kishon Vijay Abraham I @ 2021-01-13 7:21 ` Leon Romanovsky 2021-01-19 9:55 ` Shradha Todi 1 sibling, 1 reply; 7+ messages in thread From: Leon Romanovsky @ 2021-01-13 7:21 UTC (permalink / raw) To: Shradha Todi Cc: linux-kernel, linux-pci, bhelgaas, kishon, lorenzo.pieralisi, pankaj.dubey, sriram.dash, niyas.ahmed, p.rajanbabu, l.mehra, hari.tv On Tue, Jan 12, 2021 at 07:32:25PM +0530, Shradha Todi wrote: > 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. > > When the patch is not applied and EPC features is not implemented in the > platform driver, we see the following dump due to kernel NULL pointer > dereference. > > [ 105.135936] Call trace: > [ 105.138363] pci_epf_test_bind+0xf4/0x388 > [ 105.142354] pci_epf_bind+0x3c/0x80 > [ 105.145817] pci_epc_epf_link+0xa8/0xcc > [ 105.149632] configfs_symlink+0x1a4/0x48c > [ 105.153616] vfs_symlink+0x104/0x184 > [ 105.157169] do_symlinkat+0x80/0xd4 > [ 105.160636] __arm64_sys_symlinkat+0x1c/0x24 > [ 105.164885] el0_svc_common.constprop.3+0xb8/0x170 > [ 105.169649] el0_svc_handler+0x70/0x88 > [ 105.173377] el0_svc+0x8/0x640 > [ 105.176411] Code: d2800581 b9403ab9 f9404ebb 8b394f60 (f9400400) > [ 105.182478] ---[ end trace a438e3c5a24f9df0 ]--- Description and call trace don't correlate with the proposed code change. The code in pci_epf_test_bind() doesn't dereference epc_features, at least in direct manner. Thanks ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for ->get_features() 2021-01-13 7:21 ` Leon Romanovsky @ 2021-01-19 9:55 ` Shradha Todi 2021-01-20 5:29 ` Leon Romanovsky 2021-03-23 11:05 ` Lorenzo Pieralisi 0 siblings, 2 replies; 7+ messages in thread From: Shradha Todi @ 2021-01-19 9:55 UTC (permalink / raw) To: 'Leon Romanovsky' Cc: linux-kernel, linux-pci, bhelgaas, kishon, lorenzo.pieralisi, pankaj.dubey, sriram.dash, niyas.ahmed, p.rajanbabu, l.mehra, hari.tv > -----Original Message----- > From: Leon Romanovsky <leon@kernel.org> > Subject: Re: [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for - > >get_features() > > On Tue, Jan 12, 2021 at 07:32:25PM +0530, Shradha Todi wrote: > > 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. > > > > When the patch is not applied and EPC features is not implemented in > > the platform driver, we see the following dump due to kernel NULL > > pointer dereference. > > > > [ 105.135936] Call trace: > > [ 105.138363] pci_epf_test_bind+0xf4/0x388 [ 105.142354] > > pci_epf_bind+0x3c/0x80 [ 105.145817] pci_epc_epf_link+0xa8/0xcc [ > > 105.149632] configfs_symlink+0x1a4/0x48c [ 105.153616] > > vfs_symlink+0x104/0x184 [ 105.157169] do_symlinkat+0x80/0xd4 [ > > 105.160636] __arm64_sys_symlinkat+0x1c/0x24 [ 105.164885] > > el0_svc_common.constprop.3+0xb8/0x170 > > [ 105.169649] el0_svc_handler+0x70/0x88 [ 105.173377] > > el0_svc+0x8/0x640 [ 105.176411] Code: d2800581 b9403ab9 f9404ebb > > 8b394f60 (f9400400) [ 105.182478] ---[ end trace a438e3c5a24f9df0 > > ]--- > > > Description and call trace don't correlate with the proposed code change. > > The code in pci_epf_test_bind() doesn't dereference epc_features, at least in > direct manner. > > Thanks Thanks for the review. Yes, you're right. The dereference does not happen in the pci_epf_test_bind() itself, but in pci_epf_test_alloc_space() being called within. We will update the line "causing NULL pointer dereference in pci_epf_test_bind function. " in the commit message to "causing NULL pointer dereference in pci_epf_test_alloc_space function. " Would that be good enough? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for ->get_features() 2021-01-19 9:55 ` Shradha Todi @ 2021-01-20 5:29 ` Leon Romanovsky 2021-01-27 9:11 ` Shradha Todi 2021-03-23 11:05 ` Lorenzo Pieralisi 1 sibling, 1 reply; 7+ messages in thread From: Leon Romanovsky @ 2021-01-20 5:29 UTC (permalink / raw) To: Shradha Todi Cc: linux-kernel, linux-pci, bhelgaas, kishon, lorenzo.pieralisi, pankaj.dubey, sriram.dash, niyas.ahmed, p.rajanbabu, l.mehra, hari.tv On Tue, Jan 19, 2021 at 03:25:10PM +0530, Shradha Todi wrote: > > -----Original Message----- > > From: Leon Romanovsky <leon@kernel.org> > > Subject: Re: [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for - > > >get_features() > > > > On Tue, Jan 12, 2021 at 07:32:25PM +0530, Shradha Todi wrote: > > > 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. > > > > > > When the patch is not applied and EPC features is not implemented in > > > the platform driver, we see the following dump due to kernel NULL > > > pointer dereference. > > > > > > [ 105.135936] Call trace: > > > [ 105.138363] pci_epf_test_bind+0xf4/0x388 [ 105.142354] > > > pci_epf_bind+0x3c/0x80 [ 105.145817] pci_epc_epf_link+0xa8/0xcc [ > > > 105.149632] configfs_symlink+0x1a4/0x48c [ 105.153616] > > > vfs_symlink+0x104/0x184 [ 105.157169] do_symlinkat+0x80/0xd4 [ > > > 105.160636] __arm64_sys_symlinkat+0x1c/0x24 [ 105.164885] > > > el0_svc_common.constprop.3+0xb8/0x170 > > > [ 105.169649] el0_svc_handler+0x70/0x88 [ 105.173377] > > > el0_svc+0x8/0x640 [ 105.176411] Code: d2800581 b9403ab9 f9404ebb > > > 8b394f60 (f9400400) [ 105.182478] ---[ end trace a438e3c5a24f9df0 > > > ]--- > > > > > > Description and call trace don't correlate with the proposed code change. > > > > The code in pci_epf_test_bind() doesn't dereference epc_features, at least > in > > direct manner. > > > > Thanks > > Thanks for the review. Yes, you're right. The dereference does not happen in > the pci_epf_test_bind() itself, but in pci_epf_test_alloc_space() being > called within. We will update the line "causing NULL pointer dereference in > pci_epf_test_bind function. " in the commit message to "causing NULL pointer > dereference in pci_epf_test_alloc_space function. " Would that be good > enough? I have no idea, just saw that the rest of the code at least partially prepared to deal with epc_features == NULL, maybe pci_epf_test_alloc_space() should be update to handle epc_features == NULL. Thanks > ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for ->get_features() 2021-01-20 5:29 ` Leon Romanovsky @ 2021-01-27 9:11 ` Shradha Todi 0 siblings, 0 replies; 7+ messages in thread From: Shradha Todi @ 2021-01-27 9:11 UTC (permalink / raw) To: 'Leon Romanovsky' Cc: linux-kernel, linux-pci, bhelgaas, kishon, lorenzo.pieralisi, pankaj.dubey, sriram.dash, niyas.ahmed, p.rajanbabu, l.mehra, hari.tv > -----Original Message----- > From: Leon Romanovsky <leon@kernel.org> > Subject: Re: [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for - > >get_features() > > On Tue, Jan 19, 2021 at 03:25:10PM +0530, Shradha Todi wrote: > > > -----Original Message----- > > > From: Leon Romanovsky <leon@kernel.org> > > > Subject: Re: [PATCH v4] PCI: endpoint: Fix NULL pointer dereference > > > for - > > > >get_features() > > > > > > On Tue, Jan 12, 2021 at 07:32:25PM +0530, Shradha Todi wrote: > > > > 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. > > > > > > > > When the patch is not applied and EPC features is not implemented > > > > in the platform driver, we see the following dump due to kernel > > > > NULL pointer dereference. > > > > > > > > [ 105.135936] Call trace: > > > > [ 105.138363] pci_epf_test_bind+0xf4/0x388 [ 105.142354] > > > > pci_epf_bind+0x3c/0x80 [ 105.145817] pci_epc_epf_link+0xa8/0xcc > > > > [ 105.149632] configfs_symlink+0x1a4/0x48c [ 105.153616] > > > > vfs_symlink+0x104/0x184 [ 105.157169] do_symlinkat+0x80/0xd4 [ > > > > 105.160636] __arm64_sys_symlinkat+0x1c/0x24 [ 105.164885] > > > > el0_svc_common.constprop.3+0xb8/0x170 > > > > [ 105.169649] el0_svc_handler+0x70/0x88 [ 105.173377] > > > > el0_svc+0x8/0x640 [ 105.176411] Code: d2800581 b9403ab9 f9404ebb > > > > 8b394f60 (f9400400) [ 105.182478] ---[ end trace a438e3c5a24f9df0 > > > > ]--- > > > > > > > > > Description and call trace don't correlate with the proposed code change. > > > > > > The code in pci_epf_test_bind() doesn't dereference epc_features, at > > > least > > in > > > direct manner. > > > > > > Thanks > > > > Thanks for the review. Yes, you're right. The dereference does not > > happen in the pci_epf_test_bind() itself, but in > > pci_epf_test_alloc_space() being called within. We will update the > > line "causing NULL pointer dereference in pci_epf_test_bind function. > > " in the commit message to "causing NULL pointer dereference in > > pci_epf_test_alloc_space function. " Would that be good enough? > > I have no idea, just saw that the rest of the code at least partially prepared to > deal with epc_features == NULL, maybe > pci_epf_test_alloc_space() should be update to handle epc_features == NULL. > > Thanks > In current implementation, epc_features has been used in quite a few places in the file without proper checks. Anyway, there is no point in proceeding to bind the driver with no EPC features implemented, so we feel it's best to return as soon as possible instead of going ahead and returning error at a later point. (eg: during allocation). I checked that alloc function is not being called anywhere else except for the bind function. Thanks > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for ->get_features() 2021-01-19 9:55 ` Shradha Todi 2021-01-20 5:29 ` Leon Romanovsky @ 2021-03-23 11:05 ` Lorenzo Pieralisi 1 sibling, 0 replies; 7+ messages in thread From: Lorenzo Pieralisi @ 2021-03-23 11:05 UTC (permalink / raw) To: Shradha Todi Cc: 'Leon Romanovsky', linux-kernel, linux-pci, bhelgaas, kishon, pankaj.dubey, sriram.dash, niyas.ahmed, p.rajanbabu, l.mehra, hari.tv On Tue, Jan 19, 2021 at 03:25:10PM +0530, Shradha Todi wrote: > > -----Original Message----- > > From: Leon Romanovsky <leon@kernel.org> > > Subject: Re: [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for - > > >get_features() > > > > On Tue, Jan 12, 2021 at 07:32:25PM +0530, Shradha Todi wrote: > > > 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. > > > > > > When the patch is not applied and EPC features is not implemented in > > > the platform driver, we see the following dump due to kernel NULL > > > pointer dereference. > > > > > > [ 105.135936] Call trace: > > > [ 105.138363] pci_epf_test_bind+0xf4/0x388 [ 105.142354] > > > pci_epf_bind+0x3c/0x80 [ 105.145817] pci_epc_epf_link+0xa8/0xcc [ > > > 105.149632] configfs_symlink+0x1a4/0x48c [ 105.153616] > > > vfs_symlink+0x104/0x184 [ 105.157169] do_symlinkat+0x80/0xd4 [ > > > 105.160636] __arm64_sys_symlinkat+0x1c/0x24 [ 105.164885] > > > el0_svc_common.constprop.3+0xb8/0x170 > > > [ 105.169649] el0_svc_handler+0x70/0x88 [ 105.173377] > > > el0_svc+0x8/0x640 [ 105.176411] Code: d2800581 b9403ab9 f9404ebb > > > 8b394f60 (f9400400) [ 105.182478] ---[ end trace a438e3c5a24f9df0 > > > ]--- > > > > > > Description and call trace don't correlate with the proposed code change. > > > > The code in pci_epf_test_bind() doesn't dereference epc_features, at least > in > > direct manner. > > > > Thanks > > Thanks for the review. Yes, you're right. The dereference does not happen in > the pci_epf_test_bind() itself, but in pci_epf_test_alloc_space() being > called within. We will update the line "causing NULL pointer dereference in > pci_epf_test_bind function. " in the commit message to "causing NULL pointer > dereference in pci_epf_test_alloc_space function. " Would that be good > enough? Remove the timestamp information as well from the commit log, that's completely irrelevant information. I shall mark this as "changes requested", waiting for a new version (please keep the review tags). Lorenzo ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-03-23 11:06 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20210112140234epcas5p4f97e9cf12e68df9fb55d1270bd14280c@epcas5p4.samsung.com>
2021-01-12 14:02 ` [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for ->get_features() Shradha Todi
2021-01-13 5:05 ` Kishon Vijay Abraham I
2021-01-13 7:21 ` Leon Romanovsky
2021-01-19 9:55 ` Shradha Todi
2021-01-20 5:29 ` Leon Romanovsky
2021-01-27 9:11 ` Shradha Todi
2021-03-23 11:05 ` Lorenzo Pieralisi
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).