linux-kernel - Re: [PATCH] [POWERPC] ps3: fix error handling in ps3_system_bus_device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMuHMdWC90pwMqbVzgjXiCdQhHPNCn7H63qpLR_gnkd_KnmX6w@mail.gmail.com>
Date: Thu, 13 Mar 2025 10:24:43 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Ma Ke <make24@...as.ac.cn>
Cc: geoff@...radead.org, maddy@...ux.ibm.com, mpe@...erman.id.au, 
	npiggin@...il.com, christophe.leroy@...roup.eu, naveen@...nel.org, 
	arnd.bergmann@...ibm.com, linuxppc-dev@...ts.ozlabs.org, 
	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org, 
	stable@...r.kernel.org
Subject: Re: [PATCH] [POWERPC] ps3: fix error handling in ps3_system_bus_device_register()

Hi Ma,

On Thu, 13 Mar 2025 at 09:03, Ma Ke <make24@...as.ac.cn> 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@...r.kernel.org
> Fixes: a3d4d6435b56 ("[POWERPC] ps3: add ps3 platform system bus support")
> Signed-off-by: Ma Ke <make24@...as.ac.cn>

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@...ux-m68k.org

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