Hi Ma,
On Thu, 13 Mar 2025 at 09:03, Ma Ke <make24@xxxxxxxxxxx> wrote:
> Once device_register() failed, we should call put_device() to
> decrement reference count for cleanup. Or it could cause memory leak.
>
> As comment of device_register() says, 'NOTE: _Never_ directly free
> @dev after calling this function, even if it returned an error! Always
> use put_device() to give up the reference initialized in this function
> instead.'
>
> Found by code review.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: a3d4d6435b56 ("[POWERPC] ps3: add ps3 platform system bus support")
> Signed-off-by: Ma Ke <make24@xxxxxxxxxxx>
Thanks for your patch!
> --- a/arch/powerpc/platforms/ps3/system-bus.c
> +++ b/arch/powerpc/platforms/ps3/system-bus.c
> @@ -769,6 +769,9 @@ int ps3_system_bus_device_register(struct ps3_system_bus_device *dev)
> pr_debug("%s:%d add %s\n", __func__, __LINE__, dev_name(&dev->core));
>
> result = device_register(&dev->core);
> + if (result)
> + put_device(&dev->core);
Good catch!
> +
> return result;
> }
However, there is an issue with that:
ps3_system_bus_device_register() sets
dev->core.release = ps3_system_bus_release_device;
and:
static void ps3_system_bus_release_device(struct device *_dev)
{
struct ps3_system_bus_device *dev = ps3_dev_to_system_bus_dev(_dev);
kfree(dev);
}
As the ps3_system_bus_device is typically embedded in another struct,
which is allocated/freed separately, releasing the device will cause
a double free?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
