| Message ID | 20191001214026.21718-1-dinguyen@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: drivers/amba: release the resource to allow for deferred probe | expand |
On Tue, Oct 01, 2019 at 04:40:26PM -0500, Dinh Nguyen wrote: > With commit "79bdcb202a35 ARM: 8906/1: drivers/amba: add reset control to > amba bus probe", the amba bus driver needs to be deferred probe because the > reset driver is probed later than the amba bus. However with a deferred > probe, the call to request_resource() in the driver returns -EBUSY. The > reason is the driver has not released the resource from the previous probe > attempt. > > This patch releases the resource when amba_device_try_add() returns > -EPROBE_DEFER. This allows the deferred probe to continue. > > Fixes: 79bdcb202a35 ("ARM: 8906/1: drivers/amba: add reset control to > amba bus probe") > Signed-off-by: Dinh Nguyen <dinguyen@kernel.org> > --- > drivers/amba/bus.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c > index f39f075abff9..f246b847c991 100644 > --- a/drivers/amba/bus.c > +++ b/drivers/amba/bus.c > @@ -535,6 +535,7 @@ int amba_device_add(struct amba_device *dev, struct resource *parent) > > if (ret == -EPROBE_DEFER) { > struct deferred_device *ddev; > + release_resource(&dev->res); This is in the wrong place, and misses more serious leaks. > ddev = kmalloc(sizeof(*ddev), GFP_KERNEL); > if (!ddev) What we have is bad error cleanup code in amba_device_try_add(). Consider what would happen if dev_pm_domain_attach() inside that function were to return with -EPROBE_DEFER with your patch in place - we would call release_resource() twice on the same resource. Clearly, that's incorrect. The problem is that an error from of_reset_control_array_get_optional_shared() just returns, leaving everything that amba_device_try_add() already did still in place. So, for example, a subsequent call to amba_device_try_add() will remap the resource, leaking the previous mapping.
diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c index f39f075abff9..f246b847c991 100644 --- a/drivers/amba/bus.c +++ b/drivers/amba/bus.c @@ -535,6 +535,7 @@ int amba_device_add(struct amba_device *dev, struct resource *parent) if (ret == -EPROBE_DEFER) { struct deferred_device *ddev; + release_resource(&dev->res); ddev = kmalloc(sizeof(*ddev), GFP_KERNEL); if (!ddev)
With commit "79bdcb202a35 ARM: 8906/1: drivers/amba: add reset control to amba bus probe", the amba bus driver needs to be deferred probe because the reset driver is probed later than the amba bus. However with a deferred probe, the call to request_resource() in the driver returns -EBUSY. The reason is the driver has not released the resource from the previous probe attempt. This patch releases the resource when amba_device_try_add() returns -EPROBE_DEFER. This allows the deferred probe to continue. Fixes: 79bdcb202a35 ("ARM: 8906/1: drivers/amba: add reset control to amba bus probe") Signed-off-by: Dinh Nguyen <dinguyen@kernel.org> --- drivers/amba/bus.c | 1 + 1 file changed, 1 insertion(+)