[PATCH v3] spi: fsl-dspi: fix NULL pointer dereference

[PATCH v3] spi: fsl-dspi: fix NULL pointer dereference

[Michael Walle]

Since commit 530b5affc675 ("spi: fsl-dspi: fix use-after-free in remove
path") this driver causes a kernel oops:

[    1.891065] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000080
[..]
[    2.056973] Call trace:
[    2.059425]  dspi_setup+0xc8/0x2e0
[    2.062837]  spi_setup+0xcc/0x248
[    2.066160]  spi_add_device+0xb4/0x198
[    2.069918]  of_register_spi_device+0x250/0x370
[    2.074462]  spi_register_controller+0x4f4/0x770
[    2.079094]  dspi_probe+0x5bc/0x7b0
[    2.082594]  platform_drv_probe+0x5c/0xb0
[    2.086615]  really_probe+0xec/0x3c0
[    2.090200]  driver_probe_device+0x60/0xc0
[    2.094308]  device_driver_attach+0x7c/0x88
[    2.098503]  __driver_attach+0x60/0xe8
[    2.102263]  bus_for_each_dev+0x7c/0xd0
[    2.106109]  driver_attach+0x2c/0x38
[    2.109692]  bus_add_driver+0x194/0x1f8
[    2.113538]  driver_register+0x6c/0x128
[    2.117385]  __platform_driver_register+0x50/0x60
[    2.122105]  fsl_dspi_driver_init+0x24/0x30
[    2.126302]  do_one_initcall+0x54/0x2d0
[    2.130149]  kernel_init_freeable+0x1ec/0x258
[    2.134520]  kernel_init+0x1c/0x120
[    2.138018]  ret_from_fork+0x10/0x34
[    2.141606] Code: 97e0b11d aa0003f3 b4000680 f94006e0 (f9404000)
[    2.147723] ---[ end trace 26cf63e6cbba33a8 ]---

This is because since this commit, the allocation of the drivers private
data is done explicitly and in this case spi_alloc_master() won't set the
correct pointer.

Also move the platform_set_drvdata() to have both next to each other.

Fixes: 530b5affc675 ("spi: fsl-dspi: fix use-after-free in remove path")
Signed-off-by: Michael Walle <michael@walle.cc>
Tested-by: Sascha Hauer <s.hauer@pengutronix.de>
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
---
changes since v2:
 - trimmed the commit message, suggested by Krzysztof.

changes since v1:
 - moved platform_set_drvdata(), suggested by Krzysztof.

 drivers/spi/spi-fsl-dspi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index a939618f5e47..3967afa465f0 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -1236,6 +1236,9 @@ static int dspi_probe(struct platform_device *pdev)
 	if (!ctlr)
 		return -ENOMEM;
 
+	spi_controller_set_devdata(ctlr, dspi);
+	platform_set_drvdata(pdev, dspi);
+
 	dspi->pdev = pdev;
 	dspi->ctlr = ctlr;
 
@@ -1371,8 +1374,6 @@ static int dspi_probe(struct platform_device *pdev)
 	if (dspi->devtype_data->trans_mode != DSPI_DMA_MODE)
 		ctlr->ptp_sts_supported = true;
 
-	platform_set_drvdata(pdev, dspi);
-
 	ret = spi_register_controller(ctlr);
 	if (ret != 0) {
 		dev_err(&pdev->dev, "Problem registering DSPI ctlr\n");
-- 
2.20.1

Re: [PATCH v3] spi: fsl-dspi: fix NULL pointer dereference

[Mark Brown]

On Mon, 28 Sep 2020 10:55:00 +0200, Michael Walle wrote:
> Since commit 530b5affc675 ("spi: fsl-dspi: fix use-after-free in remove
> path") this driver causes a kernel oops:
> 
> [    1.891065] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000080
> [..]
> [    2.056973] Call trace:
> [    2.059425]  dspi_setup+0xc8/0x2e0
> [    2.062837]  spi_setup+0xcc/0x248
> [    2.066160]  spi_add_device+0xb4/0x198
> [    2.069918]  of_register_spi_device+0x250/0x370
> [    2.074462]  spi_register_controller+0x4f4/0x770
> [    2.079094]  dspi_probe+0x5bc/0x7b0
> [    2.082594]  platform_drv_probe+0x5c/0xb0
> [    2.086615]  really_probe+0xec/0x3c0
> [    2.090200]  driver_probe_device+0x60/0xc0
> [    2.094308]  device_driver_attach+0x7c/0x88
> [    2.098503]  __driver_attach+0x60/0xe8
> [    2.102263]  bus_for_each_dev+0x7c/0xd0
> [    2.106109]  driver_attach+0x2c/0x38
> [    2.109692]  bus_add_driver+0x194/0x1f8
> [    2.113538]  driver_register+0x6c/0x128
> [    2.117385]  __platform_driver_register+0x50/0x60
> [    2.122105]  fsl_dspi_driver_init+0x24/0x30
> [    2.126302]  do_one_initcall+0x54/0x2d0
> [    2.130149]  kernel_init_freeable+0x1ec/0x258
> [    2.134520]  kernel_init+0x1c/0x120
> [    2.138018]  ret_from_fork+0x10/0x34
> [    2.141606] Code: 97e0b11d aa0003f3 b4000680 f94006e0 (f9404000)
> [    2.147723] ---[ end trace 26cf63e6cbba33a8 ]---
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: fsl-dspi: fix NULL pointer dereference
      commit: 6e3837668e00fb914ac2b43158ef51b027ec385c

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH v3] spi: fsl-dspi: fix NULL pointer dereference

[Naresh Kamboju]

On Mon, 28 Sep 2020 at 14:26, Michael Walle <michael@walle.cc> wrote:
>
> Since commit 530b5affc675 ("spi: fsl-dspi: fix use-after-free in remove
> path") this driver causes a kernel oops:

FYI,
This reported issue is now happening on Linus's master 5.9.0-rc8 on arm64
nxp-ls2088 devices.

Crash log,
-------------
[    1.986452] fsl,ifc-nand 530000000.nand: IFC NAND device at
0x530000000, bank 2
[    1.994751] Unable to handle kernel NULL pointer dereference at
virtual address 0000000000000080
[    2.003542] Mem abort info:
[    2.006331]   ESR = 0x96000004
[    2.009380]   EC = 0x25: DABT (current EL), IL = 32 bits
[    2.014718]   SET = 0, FnV = 0
[    2.017764]   EA = 0, S1PTW = 0
[    2.020908] Data abort info:
[    2.023787]   ISV = 0, ISS = 0x00000004
[    2.027621]   CM = 0, WnR = 0
[    2.030586] [0000000000000080] user address but active_mm is swapper
[    2.036940] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[    2.042505] Modules linked in:
[    2.045553] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.9.0-rc8 #1
[    2.051725] Hardware name: Freescale Layerscape 2088A RDB Board (DT)
[    2.058072] pstate: 40000005 (nZcv daif -PAN -UAO BTYPE=--)
[    2.063643] pc : dspi_setup+0x5c/0x2b0
[    2.067383] lr : dspi_setup+0x278/0x2b0
[    2.071209] sp : ffff80001005b900
[    2.074514] x29: ffff80001005b900 x28: ffff0082ccd3f000
[    2.079819] x27: ffffd73de0a704e0 x26: ffff0082edebc100
[    2.085123] x25: 0000000000000001 x24: ffff0082edea6810
[    2.090427] x23: ffffd73de15b6c40 x22: ffff0082ccd3f600
[    2.095731] x21: ffff0082ee6b0000 x20: ffff0082cce51100
[    2.101036] x19: ffff0082ccd3f800 x18: 0000000000000000
[    2.106340] x17: ffffd73de088e6d8 x16: ffffd73de088e6e0
[    2.111644] x15: ffff0082ee6b0480 x14: ffffffffffffffff
[    2.116947] x13: ffff0082cce51087 x12: ffff0082cce51085
[    2.122251] x11: 0000000000000008 x10: 0101010101010101
[    2.127554] x9 : 0000000000000000 x8 : ffff0082cce51180
[    2.132858] x7 : 0000000000000000 x6 : 000000000000003f
[    2.138162] x5 : 0000000000000040 x4 : ffff80001005b8f0
[    2.143466] x3 : 0000000000000001 x2 : 0000000000000dc0
[    2.148770] x1 : e01dcf85cfcc6200 x0 : 0000000000000000
[    2.154074] Call trace:
[    2.156512]  dspi_setup+0x5c/0x2b0
[    2.159906]  spi_setup+0xcc/0x2e0
[    2.163211]  spi_add_device+0xec/0x218
[    2.166951]  of_register_spi_device+0x224/0x370
[    2.171473]  spi_register_controller+0x598/0x830
[    2.176081]  dspi_probe+0x32c/0x798
[    2.179562]  platform_drv_probe+0x5c/0xb0
[    2.183564]  really_probe+0xf0/0x4d8
[    2.187131]  driver_probe_device+0xfc/0x168
[    2.191305]  device_driver_attach+0x7c/0x88
[    2.195478]  __driver_attach+0xac/0x178
[    2.199306]  bus_for_each_dev+0x78/0xc8
[    2.203134]  driver_attach+0x2c/0x38
[    2.206702]  bus_add_driver+0x14c/0x230
[    2.210529]  driver_register+0x6c/0x128
[    2.214355]  __platform_driver_register+0x50/0x60
[    2.219053]  fsl_dspi_driver_init+0x24/0x30
[    2.223229]  do_one_initcall+0x4c/0x2c0
[    2.227058]  kernel_init_freeable+0x214/0x280
[    2.231409]  kernel_init+0x1c/0x128
[    2.234889]  ret_from_fork+0x10/0x30
[    2.238458] Code: 290a7fff f9403c16 b4001094 f94006c0 (f9404000)
[    2.244553] ---[ end trace cec4aea5fb1ac3ec ]---
[    2.249176] Kernel panic - not syncing: Attempted to kill init!
exitcode=0x0000000b
[    2.249618] ata1: SATA link down (SStatus 0 SControl 300)
[    2.256828] SMP: stopping secondary CPUs
[    2.256836] Kernel Offset: 0x573dcf080000 from 0xffff800010000000
[    2.256837] PHYS_OFFSET: 0xffff8f0ec0000000
[    2.256839] CPU features: 0x0240022,21806008
[    2.256840] Memory Limit: none

>
> [    1.891065] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000080
> [..]
> [    2.056973] Call trace:
> [    2.059425]  dspi_setup+0xc8/0x2e0
> [    2.062837]  spi_setup+0xcc/0x248
> [    2.066160]  spi_add_device+0xb4/0x198
> [    2.069918]  of_register_spi_device+0x250/0x370
> [    2.074462]  spi_register_controller+0x4f4/0x770
> [    2.079094]  dspi_probe+0x5bc/0x7b0
> [    2.082594]  platform_drv_probe+0x5c/0xb0
> [    2.086615]  really_probe+0xec/0x3c0
> [    2.090200]  driver_probe_device+0x60/0xc0
> [    2.094308]  device_driver_attach+0x7c/0x88
> [    2.098503]  __driver_attach+0x60/0xe8
> [    2.102263]  bus_for_each_dev+0x7c/0xd0
> [    2.106109]  driver_attach+0x2c/0x38
> [    2.109692]  bus_add_driver+0x194/0x1f8
> [    2.113538]  driver_register+0x6c/0x128
> [    2.117385]  __platform_driver_register+0x50/0x60
> [    2.122105]  fsl_dspi_driver_init+0x24/0x30
> [    2.126302]  do_one_initcall+0x54/0x2d0
> [    2.130149]  kernel_init_freeable+0x1ec/0x258
> [    2.134520]  kernel_init+0x1c/0x120
> [    2.138018]  ret_from_fork+0x10/0x34
> [    2.141606] Code: 97e0b11d aa0003f3 b4000680 f94006e0 (f9404000)
> [    2.147723] ---[ end trace 26cf63e6cbba33a8 ]---
>
> This is because since this commit, the allocation of the drivers private
> data is done explicitly and in this case spi_alloc_master() won't set the
> correct pointer.
>
> Also move the platform_set_drvdata() to have both next to each other.
>
> Fixes: 530b5affc675 ("spi: fsl-dspi: fix use-after-free in remove path")
> Signed-off-by: Michael Walle <michael@walle.cc>
> Tested-by: Sascha Hauer <s.hauer@pengutronix.de>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
> changes since v2:
>  - trimmed the commit message, suggested by Krzysztof.
>
> changes since v1:
>  - moved platform_set_drvdata(), suggested by Krzysztof.
>
>  drivers/spi/spi-fsl-dspi.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
> index a939618f5e47..3967afa465f0 100644
> --- a/drivers/spi/spi-fsl-dspi.c
> +++ b/drivers/spi/spi-fsl-dspi.c
> @@ -1236,6 +1236,9 @@ static int dspi_probe(struct platform_device *pdev)
>         if (!ctlr)
>                 return -ENOMEM;
>
> +       spi_controller_set_devdata(ctlr, dspi);
> +       platform_set_drvdata(pdev, dspi);
> +
>         dspi->pdev = pdev;
>         dspi->ctlr = ctlr;
>
> @@ -1371,8 +1374,6 @@ static int dspi_probe(struct platform_device *pdev)
>         if (dspi->devtype_data->trans_mode != DSPI_DMA_MODE)
>                 ctlr->ptp_sts_supported = true;
>
> -       platform_set_drvdata(pdev, dspi);
> -
>         ret = spi_register_controller(ctlr);
>         if (ret != 0) {
>                 dev_err(&pdev->dev, "Problem registering DSPI ctlr\n");
> --
> 2.20.1
>

metadata:
  git branch: master
  git repo: https://gitlab.com/Linaro/lkft/mirrors/torvalds/linux-mainline
  git commit: 583090b1b8232e6eae243a9009699666153a13a9
  git describe: v5.9-rc8-206-g583090b1b823
  make_kernelversion: 5.9.0-rc8
  kernel-config:
https://builds.tuxbuild.com/hkbjxXyQ2-4IiR2Z-6P8vw/kernel.config

full log link,
https://qa-reports.linaro.org/lkft/linux-mainline-master/build/v5.9-rc8-206-g583090b1b823/testrun/3283416/suite/linux-log-parser/test/check-kernel-oops-91954/log


-- 
Linaro LKFT
https://lkft.linaro.org

Re: [PATCH v3] spi: fsl-dspi: fix NULL pointer dereference

[Michael Walle]

Am 2020-10-09 13:01, schrieb Naresh Kamboju:
> On Mon, 28 Sep 2020 at 14:26, Michael Walle <michael@walle.cc> wrote:
>> 
>> Since commit 530b5affc675 ("spi: fsl-dspi: fix use-after-free in 
>> remove
>> path") this driver causes a kernel oops:
> 
> FYI,
> This reported issue is now happening on Linus's master 5.9.0-rc8 on 
> arm64
> nxp-ls2088 devices.

Yep, Kontron sl28 is also broken on the latest 5.9-rc8. Somehow
   commit 530b5affc675 ("spi: fsl-dspi: fix use-after-free in remove 
path")
made it into 5.9-rc7 but this fix didn't.

-michael

Re: [PATCH v3] spi: fsl-dspi: fix NULL pointer dereference

[Mark Brown]

On Mon, 28 Sep 2020 10:55:00 +0200, Michael Walle wrote:
> Since commit 530b5affc675 ("spi: fsl-dspi: fix use-after-free in remove
> path") this driver causes a kernel oops:
> 
> [    1.891065] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000080
> [..]
> [    2.056973] Call trace:
> [    2.059425]  dspi_setup+0xc8/0x2e0
> [    2.062837]  spi_setup+0xcc/0x248
> [    2.066160]  spi_add_device+0xb4/0x198
> [    2.069918]  of_register_spi_device+0x250/0x370
> [    2.074462]  spi_register_controller+0x4f4/0x770
> [    2.079094]  dspi_probe+0x5bc/0x7b0
> [    2.082594]  platform_drv_probe+0x5c/0xb0
> [    2.086615]  really_probe+0xec/0x3c0
> [    2.090200]  driver_probe_device+0x60/0xc0
> [    2.094308]  device_driver_attach+0x7c/0x88
> [    2.098503]  __driver_attach+0x60/0xe8
> [    2.102263]  bus_for_each_dev+0x7c/0xd0
> [    2.106109]  driver_attach+0x2c/0x38
> [    2.109692]  bus_add_driver+0x194/0x1f8
> [    2.113538]  driver_register+0x6c/0x128
> [    2.117385]  __platform_driver_register+0x50/0x60
> [    2.122105]  fsl_dspi_driver_init+0x24/0x30
> [    2.126302]  do_one_initcall+0x54/0x2d0
> [    2.130149]  kernel_init_freeable+0x1ec/0x258
> [    2.134520]  kernel_init+0x1c/0x120
> [    2.138018]  ret_from_fork+0x10/0x34
> [    2.141606] Code: 97e0b11d aa0003f3 b4000680 f94006e0 (f9404000)
> [    2.147723] ---[ end trace 26cf63e6cbba33a8 ]---
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] spi: fsl-dspi: fix NULL pointer dereference
      (no commit info)

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH v3] spi: fsl-dspi: fix NULL pointer dereference

[Michael Walle]

Hi Mark,

Am 2020-12-01 14:57, schrieb Mark Brown:
> On Mon, 28 Sep 2020 10:55:00 +0200, Michael Walle wrote:
>> Since commit 530b5affc675 ("spi: fsl-dspi: fix use-after-free in 
>> remove
>> path") this driver causes a kernel oops:
>> 
>> [    1.891065] Unable to handle kernel NULL pointer dereference at 
>> virtual address 0000000000000080
>> [..]
>> [    2.056973] Call trace:
>> [    2.059425]  dspi_setup+0xc8/0x2e0
>> [    2.062837]  spi_setup+0xcc/0x248
>> [    2.066160]  spi_add_device+0xb4/0x198
>> [    2.069918]  of_register_spi_device+0x250/0x370
>> [    2.074462]  spi_register_controller+0x4f4/0x770
>> [    2.079094]  dspi_probe+0x5bc/0x7b0
>> [    2.082594]  platform_drv_probe+0x5c/0xb0
>> [    2.086615]  really_probe+0xec/0x3c0
>> [    2.090200]  driver_probe_device+0x60/0xc0
>> [    2.094308]  device_driver_attach+0x7c/0x88
>> [    2.098503]  __driver_attach+0x60/0xe8
>> [    2.102263]  bus_for_each_dev+0x7c/0xd0
>> [    2.106109]  driver_attach+0x2c/0x38
>> [    2.109692]  bus_add_driver+0x194/0x1f8
>> [    2.113538]  driver_register+0x6c/0x128
>> [    2.117385]  __platform_driver_register+0x50/0x60
>> [    2.122105]  fsl_dspi_driver_init+0x24/0x30
>> [    2.126302]  do_one_initcall+0x54/0x2d0
>> [    2.130149]  kernel_init_freeable+0x1ec/0x258
>> [    2.134520]  kernel_init+0x1c/0x120
>> [    2.138018]  ret_from_fork+0x10/0x34
>> [    2.141606] Code: 97e0b11d aa0003f3 b4000680 f94006e0 (f9404000)
>> [    2.147723] ---[ end trace 26cf63e6cbba33a8 ]---
>> 
>> [...]
> 
> Applied to
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 
> for-next

Is that correct? Some time ago you've already applied that to your spi 
tree:
https://lore.kernel.org/linux-spi/160132174502.55568.11234605078950751454.b4-ty@kernel.org/

-michael

Re: [PATCH v3] spi: fsl-dspi: fix NULL pointer dereference

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 260 bytes --]

On Tue, Dec 01, 2020 at 03:04:27PM +0100, Michael Walle wrote:

> Is that correct? Some time ago you've already applied that to your spi tree:
> https://lore.kernel.org/linux-spi/160132174502.55568.11234605078950751454.b4-ty@kernel.org/

No, it's a bug in b4.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]