This reverts commit 19e863828acf6d8ac8475ba1fd93c0fe17fdc4ef.
Fixes the following oops:
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
Internal error: Oops: 96000004 [#1] PREEMPT SMP
Modules linked in:
CPU: 1 PID: 7 Comm: kworker/u4:0 Not tainted 5.17.0-rc1-00086-ge38b27816fea-dirty #71
Hardware name: CPE0108 (DT)
Workqueue: events_unbound deferred_probe_work_func
pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : j721e_pcie_probe+0x184/0x600
lr : j721e_pcie_probe+0x170/0x600
sp : ffff80000957bae0
x29: ffff80000957bae0 x28: ffff800009357000 x27: ffff00000000c078
x26: ffff00003fe047a8 x25: 0000000000000000 x24: ffff0000000f5280
x23: ffff800008c98f78 x22: ffff800008f90ff0 x21: ffff000000231410
x20: ffff000002ef2780 x19: 0000000000000021 x18: 0000000000000001
x17: 0000000000000000 x16: 0000000000058c00 x15: ffffffffffffffff
x14: ffffffffffffffff x13: 0000000000000010 x12: 0101010101010101
x11: 0000000000000040 x10: ffff8000093e06c8 x9 : ffff8000093e06c0
x8 : ffff000000400270 x7 : 0000000000000000 x6 : ffff000000231590
x5 : ffff80000957b9e0 x4 : 0000000000000000 x3 : ffff0000002314f4
x2 : 0000000000000000 x1 : ffff0000000f5280 x0 : 0000000000000000
Call trace:
j721e_pcie_probe+0x184/0x600
platform_probe+0x68/0xe0
really_probe+0x144/0x320
__driver_probe_device+0xc4/0xe0
driver_probe_device+0x7c/0x110
__device_attach_driver+0x90/0xe0
bus_for_each_drv+0x78/0xd0
__device_attach+0xf0/0x150
device_initial_probe+0x14/0x20
bus_probe_device+0x9c/0xb0
deferred_probe_work_func+0x88/0xc0
process_one_work+0x1bc/0x340
worker_thread+0x1f8/0x420
kthread+0x110/0x120
ret_from_fork+0x10/0x20
Code: f9400280 a90573fb d0005396 913fc2d6 (f9400800)
Fixes: 19e863828acf ("PCI: j721e: Drop redundant struct device *")
Signed-off-by: Christian Gmeiner <[email protected]>
---
drivers/pci/controller/cadence/pci-j721e.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
index 489586a4cdc7..cd43d1898482 100644
--- a/drivers/pci/controller/cadence/pci-j721e.c
+++ b/drivers/pci/controller/cadence/pci-j721e.c
@@ -51,10 +51,11 @@ enum link_status {
#define MAX_LANES 2
struct j721e_pcie {
- struct cdns_pcie *cdns_pcie;
+ struct device *dev;
struct clk *refclk;
u32 mode;
u32 num_lanes;
+ struct cdns_pcie *cdns_pcie;
void __iomem *user_cfg_base;
void __iomem *intd_cfg_base;
u32 linkdown_irq_regfield;
@@ -98,7 +99,7 @@ static inline void j721e_pcie_intd_writel(struct j721e_pcie *pcie, u32 offset,
static irqreturn_t j721e_pcie_link_irq_handler(int irq, void *priv)
{
struct j721e_pcie *pcie = priv;
- struct device *dev = pcie->cdns_pcie->dev;
+ struct device *dev = pcie->dev;
u32 reg;
reg = j721e_pcie_intd_readl(pcie, STATUS_REG_SYS_2);
@@ -164,7 +165,7 @@ static const struct cdns_pcie_ops j721e_pcie_ops = {
static int j721e_pcie_set_mode(struct j721e_pcie *pcie, struct regmap *syscon,
unsigned int offset)
{
- struct device *dev = pcie->cdns_pcie->dev;
+ struct device *dev = pcie->dev;
u32 mask = J721E_MODE_RC;
u32 mode = pcie->mode;
u32 val = 0;
@@ -183,7 +184,7 @@ static int j721e_pcie_set_mode(struct j721e_pcie *pcie, struct regmap *syscon,
static int j721e_pcie_set_link_speed(struct j721e_pcie *pcie,
struct regmap *syscon, unsigned int offset)
{
- struct device *dev = pcie->cdns_pcie->dev;
+ struct device *dev = pcie->dev;
struct device_node *np = dev->of_node;
int link_speed;
u32 val = 0;
@@ -204,7 +205,7 @@ static int j721e_pcie_set_link_speed(struct j721e_pcie *pcie,
static int j721e_pcie_set_lane_count(struct j721e_pcie *pcie,
struct regmap *syscon, unsigned int offset)
{
- struct device *dev = pcie->cdns_pcie->dev;
+ struct device *dev = pcie->dev;
u32 lanes = pcie->num_lanes;
u32 val = 0;
int ret;
@@ -219,7 +220,7 @@ static int j721e_pcie_set_lane_count(struct j721e_pcie *pcie,
static int j721e_pcie_ctrl_init(struct j721e_pcie *pcie)
{
- struct device *dev = pcie->cdns_pcie->dev;
+ struct device *dev = pcie->dev;
struct device_node *node = dev->of_node;
struct of_phandle_args args;
unsigned int offset = 0;
@@ -376,6 +377,7 @@ static int j721e_pcie_probe(struct platform_device *pdev)
if (!pcie)
return -ENOMEM;
+ pcie->dev = dev;
pcie->mode = mode;
pcie->linkdown_irq_regfield = data->linkdown_irq_regfield;
--
2.34.1
On Mon, Jan 24, 2022 at 6:21 AM Christian Gmeiner
<[email protected]> wrote:
>
> This reverts commit 19e863828acf6d8ac8475ba1fd93c0fe17fdc4ef.
>
> Fixes the following oops:
Perhaps explain why the 2nd struct device was not redundant. Is this
not just a case of the dev pointer not getting set early enough?
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
> Internal error: Oops: 96000004 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 1 PID: 7 Comm: kworker/u4:0 Not tainted 5.17.0-rc1-00086-ge38b27816fea-dirty #71
> Hardware name: CPE0108 (DT)
> Workqueue: events_unbound deferred_probe_work_func
> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : j721e_pcie_probe+0x184/0x600
> lr : j721e_pcie_probe+0x170/0x600
> sp : ffff80000957bae0
> x29: ffff80000957bae0 x28: ffff800009357000 x27: ffff00000000c078
> x26: ffff00003fe047a8 x25: 0000000000000000 x24: ffff0000000f5280
> x23: ffff800008c98f78 x22: ffff800008f90ff0 x21: ffff000000231410
> x20: ffff000002ef2780 x19: 0000000000000021 x18: 0000000000000001
> x17: 0000000000000000 x16: 0000000000058c00 x15: ffffffffffffffff
> x14: ffffffffffffffff x13: 0000000000000010 x12: 0101010101010101
> x11: 0000000000000040 x10: ffff8000093e06c8 x9 : ffff8000093e06c0
> x8 : ffff000000400270 x7 : 0000000000000000 x6 : ffff000000231590
> x5 : ffff80000957b9e0 x4 : 0000000000000000 x3 : ffff0000002314f4
> x2 : 0000000000000000 x1 : ffff0000000f5280 x0 : 0000000000000000
> Call trace:
> j721e_pcie_probe+0x184/0x600
> platform_probe+0x68/0xe0
> really_probe+0x144/0x320
> __driver_probe_device+0xc4/0xe0
> driver_probe_device+0x7c/0x110
> __device_attach_driver+0x90/0xe0
> bus_for_each_drv+0x78/0xd0
> __device_attach+0xf0/0x150
> device_initial_probe+0x14/0x20
> bus_probe_device+0x9c/0xb0
> deferred_probe_work_func+0x88/0xc0
> process_one_work+0x1bc/0x340
> worker_thread+0x1f8/0x420
> kthread+0x110/0x120
> ret_from_fork+0x10/0x20
> Code: f9400280 a90573fb d0005396 913fc2d6 (f9400800)
>
> Fixes: 19e863828acf ("PCI: j721e: Drop redundant struct device *")
> Signed-off-by: Christian Gmeiner <[email protected]>
> ---
> drivers/pci/controller/cadence/pci-j721e.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
> index 489586a4cdc7..cd43d1898482 100644
> --- a/drivers/pci/controller/cadence/pci-j721e.c
> +++ b/drivers/pci/controller/cadence/pci-j721e.c
> @@ -51,10 +51,11 @@ enum link_status {
> #define MAX_LANES 2
>
> struct j721e_pcie {
> - struct cdns_pcie *cdns_pcie;
> + struct device *dev;
> struct clk *refclk;
> u32 mode;
> u32 num_lanes;
> + struct cdns_pcie *cdns_pcie;
> void __iomem *user_cfg_base;
> void __iomem *intd_cfg_base;
> u32 linkdown_irq_regfield;
> @@ -98,7 +99,7 @@ static inline void j721e_pcie_intd_writel(struct j721e_pcie *pcie, u32 offset,
> static irqreturn_t j721e_pcie_link_irq_handler(int irq, void *priv)
> {
> struct j721e_pcie *pcie = priv;
> - struct device *dev = pcie->cdns_pcie->dev;
> + struct device *dev = pcie->dev;
> u32 reg;
>
> reg = j721e_pcie_intd_readl(pcie, STATUS_REG_SYS_2);
> @@ -164,7 +165,7 @@ static const struct cdns_pcie_ops j721e_pcie_ops = {
> static int j721e_pcie_set_mode(struct j721e_pcie *pcie, struct regmap *syscon,
> unsigned int offset)
> {
> - struct device *dev = pcie->cdns_pcie->dev;
> + struct device *dev = pcie->dev;
> u32 mask = J721E_MODE_RC;
> u32 mode = pcie->mode;
> u32 val = 0;
> @@ -183,7 +184,7 @@ static int j721e_pcie_set_mode(struct j721e_pcie *pcie, struct regmap *syscon,
> static int j721e_pcie_set_link_speed(struct j721e_pcie *pcie,
> struct regmap *syscon, unsigned int offset)
> {
> - struct device *dev = pcie->cdns_pcie->dev;
> + struct device *dev = pcie->dev;
> struct device_node *np = dev->of_node;
> int link_speed;
> u32 val = 0;
> @@ -204,7 +205,7 @@ static int j721e_pcie_set_link_speed(struct j721e_pcie *pcie,
> static int j721e_pcie_set_lane_count(struct j721e_pcie *pcie,
> struct regmap *syscon, unsigned int offset)
> {
> - struct device *dev = pcie->cdns_pcie->dev;
> + struct device *dev = pcie->dev;
> u32 lanes = pcie->num_lanes;
> u32 val = 0;
> int ret;
> @@ -219,7 +220,7 @@ static int j721e_pcie_set_lane_count(struct j721e_pcie *pcie,
>
> static int j721e_pcie_ctrl_init(struct j721e_pcie *pcie)
> {
> - struct device *dev = pcie->cdns_pcie->dev;
> + struct device *dev = pcie->dev;
> struct device_node *node = dev->of_node;
> struct of_phandle_args args;
> unsigned int offset = 0;
> @@ -376,6 +377,7 @@ static int j721e_pcie_probe(struct platform_device *pdev)
> if (!pcie)
> return -ENOMEM;
>
> + pcie->dev = dev;
> pcie->mode = mode;
> pcie->linkdown_irq_regfield = data->linkdown_irq_regfield;
>
> --
> 2.34.1
>
On Mon, Jan 24, 2022 at 01:21:22PM +0100, Christian Gmeiner wrote:
> This reverts commit 19e863828acf6d8ac8475ba1fd93c0fe17fdc4ef.
>
> Fixes the following oops:
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
My fault, sorry for breaking this. Thanks a lot for the report!
19e863828acf ("PCI: j721e: Drop redundant struct device *") failed to
consider the uses of pcie->cdns_pcie->dev before pcie->cdns_pcie is
initialized. I'll figure out what to do about it.
> Internal error: Oops: 96000004 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 1 PID: 7 Comm: kworker/u4:0 Not tainted 5.17.0-rc1-00086-ge38b27816fea-dirty #71
> Hardware name: CPE0108 (DT)
> Workqueue: events_unbound deferred_probe_work_func
> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : j721e_pcie_probe+0x184/0x600
> lr : j721e_pcie_probe+0x170/0x600
> sp : ffff80000957bae0
> x29: ffff80000957bae0 x28: ffff800009357000 x27: ffff00000000c078
> x26: ffff00003fe047a8 x25: 0000000000000000 x24: ffff0000000f5280
> x23: ffff800008c98f78 x22: ffff800008f90ff0 x21: ffff000000231410
> x20: ffff000002ef2780 x19: 0000000000000021 x18: 0000000000000001
> x17: 0000000000000000 x16: 0000000000058c00 x15: ffffffffffffffff
> x14: ffffffffffffffff x13: 0000000000000010 x12: 0101010101010101
> x11: 0000000000000040 x10: ffff8000093e06c8 x9 : ffff8000093e06c0
> x8 : ffff000000400270 x7 : 0000000000000000 x6 : ffff000000231590
> x5 : ffff80000957b9e0 x4 : 0000000000000000 x3 : ffff0000002314f4
> x2 : 0000000000000000 x1 : ffff0000000f5280 x0 : 0000000000000000
> Call trace:
> j721e_pcie_probe+0x184/0x600
> platform_probe+0x68/0xe0
> really_probe+0x144/0x320
> __driver_probe_device+0xc4/0xe0
> driver_probe_device+0x7c/0x110
> __device_attach_driver+0x90/0xe0
> bus_for_each_drv+0x78/0xd0
> __device_attach+0xf0/0x150
> device_initial_probe+0x14/0x20
> bus_probe_device+0x9c/0xb0
> deferred_probe_work_func+0x88/0xc0
> process_one_work+0x1bc/0x340
> worker_thread+0x1f8/0x420
> kthread+0x110/0x120
> ret_from_fork+0x10/0x20
> Code: f9400280 a90573fb d0005396 913fc2d6 (f9400800)
>
> Fixes: 19e863828acf ("PCI: j721e: Drop redundant struct device *")
> Signed-off-by: Christian Gmeiner <[email protected]>
> ---
> drivers/pci/controller/cadence/pci-j721e.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
> index 489586a4cdc7..cd43d1898482 100644
> --- a/drivers/pci/controller/cadence/pci-j721e.c
> +++ b/drivers/pci/controller/cadence/pci-j721e.c
> @@ -51,10 +51,11 @@ enum link_status {
> #define MAX_LANES 2
>
> struct j721e_pcie {
> - struct cdns_pcie *cdns_pcie;
> + struct device *dev;
> struct clk *refclk;
> u32 mode;
> u32 num_lanes;
> + struct cdns_pcie *cdns_pcie;
> void __iomem *user_cfg_base;
> void __iomem *intd_cfg_base;
> u32 linkdown_irq_regfield;
> @@ -98,7 +99,7 @@ static inline void j721e_pcie_intd_writel(struct j721e_pcie *pcie, u32 offset,
> static irqreturn_t j721e_pcie_link_irq_handler(int irq, void *priv)
> {
> struct j721e_pcie *pcie = priv;
> - struct device *dev = pcie->cdns_pcie->dev;
> + struct device *dev = pcie->dev;
> u32 reg;
>
> reg = j721e_pcie_intd_readl(pcie, STATUS_REG_SYS_2);
> @@ -164,7 +165,7 @@ static const struct cdns_pcie_ops j721e_pcie_ops = {
> static int j721e_pcie_set_mode(struct j721e_pcie *pcie, struct regmap *syscon,
> unsigned int offset)
> {
> - struct device *dev = pcie->cdns_pcie->dev;
> + struct device *dev = pcie->dev;
> u32 mask = J721E_MODE_RC;
> u32 mode = pcie->mode;
> u32 val = 0;
> @@ -183,7 +184,7 @@ static int j721e_pcie_set_mode(struct j721e_pcie *pcie, struct regmap *syscon,
> static int j721e_pcie_set_link_speed(struct j721e_pcie *pcie,
> struct regmap *syscon, unsigned int offset)
> {
> - struct device *dev = pcie->cdns_pcie->dev;
> + struct device *dev = pcie->dev;
> struct device_node *np = dev->of_node;
> int link_speed;
> u32 val = 0;
> @@ -204,7 +205,7 @@ static int j721e_pcie_set_link_speed(struct j721e_pcie *pcie,
> static int j721e_pcie_set_lane_count(struct j721e_pcie *pcie,
> struct regmap *syscon, unsigned int offset)
> {
> - struct device *dev = pcie->cdns_pcie->dev;
> + struct device *dev = pcie->dev;
> u32 lanes = pcie->num_lanes;
> u32 val = 0;
> int ret;
> @@ -219,7 +220,7 @@ static int j721e_pcie_set_lane_count(struct j721e_pcie *pcie,
>
> static int j721e_pcie_ctrl_init(struct j721e_pcie *pcie)
> {
> - struct device *dev = pcie->cdns_pcie->dev;
> + struct device *dev = pcie->dev;
> struct device_node *node = dev->of_node;
> struct of_phandle_args args;
> unsigned int offset = 0;
> @@ -376,6 +377,7 @@ static int j721e_pcie_probe(struct platform_device *pdev)
> if (!pcie)
> return -ENOMEM;
>
> + pcie->dev = dev;
> pcie->mode = mode;
> pcie->linkdown_irq_regfield = data->linkdown_irq_regfield;
>
> --
> 2.34.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Jan 24, 2022 at 01:21:22PM +0100, Christian Gmeiner wrote:
> This reverts commit 19e863828acf6d8ac8475ba1fd93c0fe17fdc4ef.
>
> Fixes the following oops:
> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
Hi Christian,
Would you mind trying this patch?
commit 9d36a93af8fe ("PCI: j721e: Initialize pcie->cdns_pcie before using it")
Author: Bjorn Helgaas <[email protected]>
Date: Thu Jan 27 15:49:49 2022 -0600
PCI: j721e: Initialize pcie->cdns_pcie before using it
Christian reported a NULL pointer dereference in j721e_pcie_probe() caused
by 19e863828acf ("PCI: j721e: Drop redundant struct device *"), which
removed struct j721e_pcie.dev since there's another copy in struct
cdns_pcie.dev reachable via j721e_pcie->cdns_pcie->dev.
The problem is that j721e_pcie->cdns_pcie was dereferenced before being
initialized:
j721e_pcie_probe
pcie = devm_kzalloc() # struct j721e_pcie
j721e_pcie_ctrl_init(pcie)
dev = pcie->cdns_pcie->dev <-- dereference cdns_pcie
switch (mode) {
case PCI_MODE_RC:
cdns_pcie = ... # alloc as part of pci_host_bridge
pcie->cdns_pcie = cdns_pcie <-- initialize pcie->cdns_pcie
Initialize pcie->cdns_pcie before it is used.
Fixes: 19e863828acf ("PCI: j721e: Drop redundant struct device *")
Reported-by: Christian Gmeiner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Bjorn Helgaas <[email protected]>
diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
index 489586a4cdc7..5d950c1d9fd0 100644
--- a/drivers/pci/controller/cadence/pci-j721e.c
+++ b/drivers/pci/controller/cadence/pci-j721e.c
@@ -372,10 +372,48 @@ static int j721e_pcie_probe(struct platform_device *pdev)
mode = (u32)data->mode;
+ switch (mode) {
+ case PCI_MODE_RC:
+ if (!IS_ENABLED(CONFIG_PCIE_CADENCE_HOST))
+ return -ENODEV;
+
+ bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc));
+ if (!bridge)
+ return -ENOMEM;
+
+ if (!data->byte_access_allowed)
+ bridge->ops = &cdns_ti_pcie_host_ops;
+ rc = pci_host_bridge_priv(bridge);
+ rc->quirk_retrain_flag = data->quirk_retrain_flag;
+ rc->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
+
+ cdns_pcie = &rc->pcie;
+ break;
+ case PCI_MODE_EP:
+ if (!IS_ENABLED(CONFIG_PCIE_CADENCE_EP))
+ return -ENODEV;
+
+ ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL);
+ if (!ep)
+ return -ENOMEM;
+
+ ep->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
+
+ cdns_pcie = &ep->pcie;
+ break;
+ default:
+ dev_err(dev, "INVALID device type %d\n", mode);
+ return 0;
+ }
+
+ cdns_pcie->dev = dev;
+ cdns_pcie->ops = &j721e_pcie_ops;
+
pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
if (!pcie)
return -ENOMEM;
+ pcie->cdns_pcie = cdns_pcie;
pcie->mode = mode;
pcie->linkdown_irq_regfield = data->linkdown_irq_regfield;
@@ -426,28 +464,6 @@ static int j721e_pcie_probe(struct platform_device *pdev)
switch (mode) {
case PCI_MODE_RC:
- if (!IS_ENABLED(CONFIG_PCIE_CADENCE_HOST)) {
- ret = -ENODEV;
- goto err_get_sync;
- }
-
- bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc));
- if (!bridge) {
- ret = -ENOMEM;
- goto err_get_sync;
- }
-
- if (!data->byte_access_allowed)
- bridge->ops = &cdns_ti_pcie_host_ops;
- rc = pci_host_bridge_priv(bridge);
- rc->quirk_retrain_flag = data->quirk_retrain_flag;
- rc->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
-
- cdns_pcie = &rc->pcie;
- cdns_pcie->dev = dev;
- cdns_pcie->ops = &j721e_pcie_ops;
- pcie->cdns_pcie = cdns_pcie;
-
gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
if (IS_ERR(gpiod)) {
ret = PTR_ERR(gpiod);
@@ -497,23 +513,6 @@ static int j721e_pcie_probe(struct platform_device *pdev)
break;
case PCI_MODE_EP:
- if (!IS_ENABLED(CONFIG_PCIE_CADENCE_EP)) {
- ret = -ENODEV;
- goto err_get_sync;
- }
-
- ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL);
- if (!ep) {
- ret = -ENOMEM;
- goto err_get_sync;
- }
- ep->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
-
- cdns_pcie = &ep->pcie;
- cdns_pcie->dev = dev;
- cdns_pcie->ops = &j721e_pcie_ops;
- pcie->cdns_pcie = cdns_pcie;
-
ret = cdns_pcie_init_phy(dev, cdns_pcie);
if (ret) {
dev_err(dev, "Failed to init phy\n");
@@ -525,8 +524,6 @@ static int j721e_pcie_probe(struct platform_device *pdev)
goto err_pcie_setup;
break;
- default:
- dev_err(dev, "INVALID device type %d\n", mode);
}
return 0;
Am Do., 27. Jan. 2022 um 23:29 Uhr schrieb Bjorn Helgaas <[email protected]>:
>
> On Mon, Jan 24, 2022 at 01:21:22PM +0100, Christian Gmeiner wrote:
> > This reverts commit 19e863828acf6d8ac8475ba1fd93c0fe17fdc4ef.
> >
> > Fixes the following oops:
> > Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
>
> Hi Christian,
>
> Would you mind trying this patch?
>
Works - thanks. You can add my Tested-by.
> commit 9d36a93af8fe ("PCI: j721e: Initialize pcie->cdns_pcie before using it")
> Author: Bjorn Helgaas <[email protected]>
> Date: Thu Jan 27 15:49:49 2022 -0600
>
> PCI: j721e: Initialize pcie->cdns_pcie before using it
>
> Christian reported a NULL pointer dereference in j721e_pcie_probe() caused
> by 19e863828acf ("PCI: j721e: Drop redundant struct device *"), which
> removed struct j721e_pcie.dev since there's another copy in struct
> cdns_pcie.dev reachable via j721e_pcie->cdns_pcie->dev.
>
> The problem is that j721e_pcie->cdns_pcie was dereferenced before being
> initialized:
>
> j721e_pcie_probe
> pcie = devm_kzalloc() # struct j721e_pcie
> j721e_pcie_ctrl_init(pcie)
> dev = pcie->cdns_pcie->dev <-- dereference cdns_pcie
> switch (mode) {
> case PCI_MODE_RC:
> cdns_pcie = ... # alloc as part of pci_host_bridge
> pcie->cdns_pcie = cdns_pcie <-- initialize pcie->cdns_pcie
>
> Initialize pcie->cdns_pcie before it is used.
>
> Fixes: 19e863828acf ("PCI: j721e: Drop redundant struct device *")
> Reported-by: Christian Gmeiner <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> Signed-off-by: Bjorn Helgaas <[email protected]>
>
> diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
> index 489586a4cdc7..5d950c1d9fd0 100644
> --- a/drivers/pci/controller/cadence/pci-j721e.c
> +++ b/drivers/pci/controller/cadence/pci-j721e.c
> @@ -372,10 +372,48 @@ static int j721e_pcie_probe(struct platform_device *pdev)
>
> mode = (u32)data->mode;
>
> + switch (mode) {
> + case PCI_MODE_RC:
> + if (!IS_ENABLED(CONFIG_PCIE_CADENCE_HOST))
> + return -ENODEV;
> +
> + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc));
> + if (!bridge)
> + return -ENOMEM;
> +
> + if (!data->byte_access_allowed)
> + bridge->ops = &cdns_ti_pcie_host_ops;
> + rc = pci_host_bridge_priv(bridge);
> + rc->quirk_retrain_flag = data->quirk_retrain_flag;
> + rc->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
> +
> + cdns_pcie = &rc->pcie;
> + break;
> + case PCI_MODE_EP:
> + if (!IS_ENABLED(CONFIG_PCIE_CADENCE_EP))
> + return -ENODEV;
> +
> + ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL);
> + if (!ep)
> + return -ENOMEM;
> +
> + ep->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
> +
> + cdns_pcie = &ep->pcie;
> + break;
> + default:
> + dev_err(dev, "INVALID device type %d\n", mode);
> + return 0;
> + }
> +
> + cdns_pcie->dev = dev;
> + cdns_pcie->ops = &j721e_pcie_ops;
> +
> pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> if (!pcie)
> return -ENOMEM;
>
> + pcie->cdns_pcie = cdns_pcie;
> pcie->mode = mode;
> pcie->linkdown_irq_regfield = data->linkdown_irq_regfield;
>
> @@ -426,28 +464,6 @@ static int j721e_pcie_probe(struct platform_device *pdev)
>
> switch (mode) {
> case PCI_MODE_RC:
> - if (!IS_ENABLED(CONFIG_PCIE_CADENCE_HOST)) {
> - ret = -ENODEV;
> - goto err_get_sync;
> - }
> -
> - bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc));
> - if (!bridge) {
> - ret = -ENOMEM;
> - goto err_get_sync;
> - }
> -
> - if (!data->byte_access_allowed)
> - bridge->ops = &cdns_ti_pcie_host_ops;
> - rc = pci_host_bridge_priv(bridge);
> - rc->quirk_retrain_flag = data->quirk_retrain_flag;
> - rc->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
> -
> - cdns_pcie = &rc->pcie;
> - cdns_pcie->dev = dev;
> - cdns_pcie->ops = &j721e_pcie_ops;
> - pcie->cdns_pcie = cdns_pcie;
> -
> gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> if (IS_ERR(gpiod)) {
> ret = PTR_ERR(gpiod);
> @@ -497,23 +513,6 @@ static int j721e_pcie_probe(struct platform_device *pdev)
>
> break;
> case PCI_MODE_EP:
> - if (!IS_ENABLED(CONFIG_PCIE_CADENCE_EP)) {
> - ret = -ENODEV;
> - goto err_get_sync;
> - }
> -
> - ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL);
> - if (!ep) {
> - ret = -ENOMEM;
> - goto err_get_sync;
> - }
> - ep->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
> -
> - cdns_pcie = &ep->pcie;
> - cdns_pcie->dev = dev;
> - cdns_pcie->ops = &j721e_pcie_ops;
> - pcie->cdns_pcie = cdns_pcie;
> -
> ret = cdns_pcie_init_phy(dev, cdns_pcie);
> if (ret) {
> dev_err(dev, "Failed to init phy\n");
> @@ -525,8 +524,6 @@ static int j721e_pcie_probe(struct platform_device *pdev)
> goto err_pcie_setup;
>
> break;
> - default:
> - dev_err(dev, "INVALID device type %d\n", mode);
> }
>
> return 0;
--
greets
--
Christian Gmeiner, MSc
https://christian-gmeiner.info/privacypolicy
On Mon, Jan 31, 2022 at 12:06:21PM +0100, Christian Gmeiner wrote:
> Am Do., 27. Jan. 2022 um 23:29 Uhr schrieb Bjorn Helgaas <[email protected]>:
> >
> > On Mon, Jan 24, 2022 at 01:21:22PM +0100, Christian Gmeiner wrote:
> > > This reverts commit 19e863828acf6d8ac8475ba1fd93c0fe17fdc4ef.
> > >
> > > Fixes the following oops:
> > > Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
> >
> > Hi Christian,
> >
> > Would you mind trying this patch?
> >
>
> Works - thanks. You can add my Tested-by.
Thanks, applied to for-linus for v5.17.
>
> > commit 9d36a93af8fe ("PCI: j721e: Initialize pcie->cdns_pcie before using it")
> > Author: Bjorn Helgaas <[email protected]>
> > Date: Thu Jan 27 15:49:49 2022 -0600
> >
> > PCI: j721e: Initialize pcie->cdns_pcie before using it
> >
> > Christian reported a NULL pointer dereference in j721e_pcie_probe() caused
> > by 19e863828acf ("PCI: j721e: Drop redundant struct device *"), which
> > removed struct j721e_pcie.dev since there's another copy in struct
> > cdns_pcie.dev reachable via j721e_pcie->cdns_pcie->dev.
> >
> > The problem is that j721e_pcie->cdns_pcie was dereferenced before being
> > initialized:
> >
> > j721e_pcie_probe
> > pcie = devm_kzalloc() # struct j721e_pcie
> > j721e_pcie_ctrl_init(pcie)
> > dev = pcie->cdns_pcie->dev <-- dereference cdns_pcie
> > switch (mode) {
> > case PCI_MODE_RC:
> > cdns_pcie = ... # alloc as part of pci_host_bridge
> > pcie->cdns_pcie = cdns_pcie <-- initialize pcie->cdns_pcie
> >
> > Initialize pcie->cdns_pcie before it is used.
> >
> > Fixes: 19e863828acf ("PCI: j721e: Drop redundant struct device *")
> > Reported-by: Christian Gmeiner <[email protected]>
> > Link: https://lore.kernel.org/r/[email protected]
> > Signed-off-by: Bjorn Helgaas <[email protected]>
> >
> > diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
> > index 489586a4cdc7..5d950c1d9fd0 100644
> > --- a/drivers/pci/controller/cadence/pci-j721e.c
> > +++ b/drivers/pci/controller/cadence/pci-j721e.c
> > @@ -372,10 +372,48 @@ static int j721e_pcie_probe(struct platform_device *pdev)
> >
> > mode = (u32)data->mode;
> >
> > + switch (mode) {
> > + case PCI_MODE_RC:
> > + if (!IS_ENABLED(CONFIG_PCIE_CADENCE_HOST))
> > + return -ENODEV;
> > +
> > + bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc));
> > + if (!bridge)
> > + return -ENOMEM;
> > +
> > + if (!data->byte_access_allowed)
> > + bridge->ops = &cdns_ti_pcie_host_ops;
> > + rc = pci_host_bridge_priv(bridge);
> > + rc->quirk_retrain_flag = data->quirk_retrain_flag;
> > + rc->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
> > +
> > + cdns_pcie = &rc->pcie;
> > + break;
> > + case PCI_MODE_EP:
> > + if (!IS_ENABLED(CONFIG_PCIE_CADENCE_EP))
> > + return -ENODEV;
> > +
> > + ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL);
> > + if (!ep)
> > + return -ENOMEM;
> > +
> > + ep->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
> > +
> > + cdns_pcie = &ep->pcie;
> > + break;
> > + default:
> > + dev_err(dev, "INVALID device type %d\n", mode);
> > + return 0;
> > + }
> > +
> > + cdns_pcie->dev = dev;
> > + cdns_pcie->ops = &j721e_pcie_ops;
> > +
> > pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> > if (!pcie)
> > return -ENOMEM;
> >
> > + pcie->cdns_pcie = cdns_pcie;
> > pcie->mode = mode;
> > pcie->linkdown_irq_regfield = data->linkdown_irq_regfield;
> >
> > @@ -426,28 +464,6 @@ static int j721e_pcie_probe(struct platform_device *pdev)
> >
> > switch (mode) {
> > case PCI_MODE_RC:
> > - if (!IS_ENABLED(CONFIG_PCIE_CADENCE_HOST)) {
> > - ret = -ENODEV;
> > - goto err_get_sync;
> > - }
> > -
> > - bridge = devm_pci_alloc_host_bridge(dev, sizeof(*rc));
> > - if (!bridge) {
> > - ret = -ENOMEM;
> > - goto err_get_sync;
> > - }
> > -
> > - if (!data->byte_access_allowed)
> > - bridge->ops = &cdns_ti_pcie_host_ops;
> > - rc = pci_host_bridge_priv(bridge);
> > - rc->quirk_retrain_flag = data->quirk_retrain_flag;
> > - rc->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
> > -
> > - cdns_pcie = &rc->pcie;
> > - cdns_pcie->dev = dev;
> > - cdns_pcie->ops = &j721e_pcie_ops;
> > - pcie->cdns_pcie = cdns_pcie;
> > -
> > gpiod = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> > if (IS_ERR(gpiod)) {
> > ret = PTR_ERR(gpiod);
> > @@ -497,23 +513,6 @@ static int j721e_pcie_probe(struct platform_device *pdev)
> >
> > break;
> > case PCI_MODE_EP:
> > - if (!IS_ENABLED(CONFIG_PCIE_CADENCE_EP)) {
> > - ret = -ENODEV;
> > - goto err_get_sync;
> > - }
> > -
> > - ep = devm_kzalloc(dev, sizeof(*ep), GFP_KERNEL);
> > - if (!ep) {
> > - ret = -ENOMEM;
> > - goto err_get_sync;
> > - }
> > - ep->quirk_detect_quiet_flag = data->quirk_detect_quiet_flag;
> > -
> > - cdns_pcie = &ep->pcie;
> > - cdns_pcie->dev = dev;
> > - cdns_pcie->ops = &j721e_pcie_ops;
> > - pcie->cdns_pcie = cdns_pcie;
> > -
> > ret = cdns_pcie_init_phy(dev, cdns_pcie);
> > if (ret) {
> > dev_err(dev, "Failed to init phy\n");
> > @@ -525,8 +524,6 @@ static int j721e_pcie_probe(struct platform_device *pdev)
> > goto err_pcie_setup;
> >
> > break;
> > - default:
> > - dev_err(dev, "INVALID device type %d\n", mode);
> > }
> >
> > return 0;
>
>
>
> --
> greets
> --
> Christian Gmeiner, MSc
>
> https://christian-gmeiner.info/privacypolicy