spi: lpspi: Avoid potential use-after-free in probe()

Commit Message

A. Sverdlin March 12, 2024, 11:20 a.m. UTC

From: Alexander Sverdlin <[e-mail]>

fsl_lpspi_probe() is allocating/disposing memory manually with
spi_alloc_host()/spi_alloc_target(), but uses
devm_spi_register_controller(). In case of error after the latter call the
memory will be explicitly freed in the probe function by
spi_controller_put() call, but used afterwards by "devm" management outside
probe() (spi_unregister_controller() <- devm_spi_unregister() below).

Unable to handle kernel NULL pointer dereference at virtual address 0000000000000070
...
Call trace:
 kernfs_find_ns
 kernfs_find_and_get_ns
 sysfs_remove_group
 sysfs_remove_groups
 device_remove_attrs
 device_del
 spi_unregister_controller
 devm_spi_unregister
 release_nodes
 devres_release_all
 really_probe
 driver_probe_device
 __device_attach_driver
 bus_for_each_drv
 __device_attach
 device_initial_probe
 bus_probe_device
 deferred_probe_work_func
 process_one_work
 worker_thread
 kthread
 ret_from_fork

Fixes: 5314987de5e5 ("spi: imx: add lpspi bus driver")
Signed-off-by: Alexander Sverdlin <[e-mail]>
---
 drivers/spi/spi-fsl-lpspi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Mark Brown March 12, 2024, 1:46 p.m. UTC | #1

On Tue, Mar 12, 2024 at 12:20:48PM +0100, A. Sverdlin wrote:

> Unable to handle kernel NULL pointer dereference at virtual address 0000000000000070
> ...
> Call trace:
>  kernfs_find_ns
>  kernfs_find_and_get_ns
>  sysfs_remove_group
>  sysfs_remove_groups
>  device_remove_attrs
>  device_del
>  spi_unregister_controller
>  devm_spi_unregister
>  release_nodes
>  devres_release_all
>  really_probe
>  driver_probe_device
>  __device_attach_driver

Please think hard before including complete backtraces in upstream
reports, they are very large and contain almost no useful information
relative to their size so often obscure the relevant content in your
message. If part of the backtrace is usefully illustrative (it often is
for search engines if nothing else) then it's usually better to pull out
the relevant sections.

Mark Brown March 12, 2024, 4:57 p.m. UTC | #2

On Tue, 12 Mar 2024 12:20:48 +0100, A. Sverdlin wrote:
> fsl_lpspi_probe() is allocating/disposing memory manually with
> spi_alloc_host()/spi_alloc_target(), but uses
> devm_spi_register_controller(). In case of error after the latter call the
> memory will be explicitly freed in the probe function by
> spi_controller_put() call, but used afterwards by "devm" management outside
> probe() (spi_unregister_controller() <- devm_spi_unregister() below).
> 
> [...]

Applied to

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

Thanks!

[1/1] spi: lpspi: Avoid potential use-after-free in probe()
      commit: 2ae0ab0143fcc06190713ed81a6486ed0ad3c861

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

Patch

diff --git a/drivers/spi/spi-fsl-lpspi.c b/drivers/spi/spi-fsl-lpspi.c
index 11991eb126364..079035db7dd85 100644
--- a/drivers/spi/spi-fsl-lpspi.c
+++ b/drivers/spi/spi-fsl-lpspi.c
@@ -830,11 +830,11 @@  static int fsl_lpspi_probe(struct platform_device *pdev)
 
 	is_target = of_property_read_bool((&pdev->dev)->of_node, "spi-slave");
 	if (is_target)
-		controller = spi_alloc_target(&pdev->dev,
-					      sizeof(struct fsl_lpspi_data));
+		controller = devm_spi_alloc_target(&pdev->dev,
+						   sizeof(struct fsl_lpspi_data));
 	else
-		controller = spi_alloc_host(&pdev->dev,
-					    sizeof(struct fsl_lpspi_data));
+		controller = devm_spi_alloc_host(&pdev->dev,
+						 sizeof(struct fsl_lpspi_data));
 
 	if (!controller)
 		return -ENOMEM;