[PATCH] accel/rocket: clear rdev on device init failure

[PATCH] accel/rocket: clear rdev on device init failure

[Guangshuo Li]

rocket_probe() stores the return value from rocket_device_init() in the
file-scoped rdev pointer before checking whether it is an error pointer.
If rocket_device_init() fails, rdev is left as an ERR_PTR.

The rdev pointer is also used as the marker for whether the shared rocket
device has already been initialized. A later core probe can therefore see
a non-NULL rdev, skip rocket_device_init(), and dereference the error
pointer when reading rdev->num_cores.

Save the error code and clear rdev before returning from the
rocket_device_init() failure path.

Fixes: ed98261b4168 ("accel/rocket: Add a new driver for Rockchip's NPU")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/accel/rocket/rocket_drv.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
index 8bbbce594883..67e7f543fe4c 100644
--- a/drivers/accel/rocket/rocket_drv.c
+++ b/drivers/accel/rocket/rocket_drv.c
@@ -165,8 +165,11 @@ static int rocket_probe(struct platform_device *pdev)
 		/* First core probing, initialize DRM device. */
 		rdev = rocket_device_init(drm_dev, &rocket_drm_driver);
 		if (IS_ERR(rdev)) {
+			int err = PTR_ERR(rdev);
+
 			dev_err(&pdev->dev, "failed to initialize rocket device\n");
-			return PTR_ERR(rdev);
+			rdev = NULL;
+			return err;
 		}
 	}
 
-- 
2.43.0

Re: [PATCH] accel/rocket: clear rdev on device init failure

[Igor Paunovic]

Hi Guangshuo,

I have been working on this driver on RK3588 (Orange Pi 5 Plus) and
went through your fix carefully:

- The analysis matches the code: rocket_probe() publishes the ERR_PTR
  into the file-scoped rdev before the IS_ERR() check, and since
  __driver_attach() keeps probing the remaining devices after a failed
  probe, the next rknn-core dereferences the poisoned pointer at
  rdev->num_cores. Every RK3588 DT carries three rknn-core nodes, so
  the follow-up probe is guaranteed.

- rocket_device_init() cannot return -EPROBE_DEFER (it does no clock,
  regulator or supplier lookups), so clearing rdev here cannot
  interfere with the deferral path in rocket_core_init(), which
  already resets rdev correctly further down in rocket_probe().

- The fix mirrors the shape the driver already uses on the
  rocket_core_init() failure path (fini + rdev = NULL) and changes
  nothing on the success path.

Reviewed-by: Igor Paunovic <royalnet026@gmail.com>

I also ran the patched driver on RK3588: normal three-core probe,
runtime PM suspend/resume and a Teflon/MobileNetV1 inference run are
all unaffected.

Thanks,
Igor