linux-kernel - Re: Buggy __free(kfree) usage pattern already in tree

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 15 Sep 2023 12:09:28 +0200
From:   Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
To:     Alexey Dobriyan <adobriyan@...il.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        linux-kernel@...r.kernel.org,
        Linus Walleij <linus.walleij@...aro.org>,
        torvalds@...ux-foundation.org, akpm@...ux-foundation.org
Subject: Re: Buggy __free(kfree) usage pattern already in tree

On Fri, 15 Sept 2023 at 11:56, Alexey Dobriyan <adobriyan@...il.com> wrote:
>
> __free() got some usage and some of the usage is buggy:
>
>    832  static struct fwnode_handle *
>    833  gpio_sim_make_bank_swnode(struct gpio_sim_bank *bank,
>    834                            struct fwnode_handle *parent)
>    835  {
>    838          char **line_names __free(kfree) = NULL;
>                 // returns NULL or ERR_PTR(-E)
>    848          line_names = gpio_sim_make_line_names(bank, &line_names_size);
>    849          if (IS_ERR(line_names))
>    850                  return ERR_CAST(line_names);
>

Thanks for the report, I'll send a fix.

Bart

>
> This pattern will result in calling kfree() on error value.
> And there are no compiler or sparse checking these things.
>
> This test module demonstrates the landmine:
>
> [  812.981089] ------------[ cut here ]------------
> [  812.981597] WARNING: CPU: 0 PID: 1326 at mm/slab_common.c:991 free_large_kmalloc+0x50/0x80
> [  813.013266] ---[ end trace 0000000000000000 ]---
> [  813.013800] object pointer: 0xfffffffffffffff4
>
> #include <linux/module.h>
> #include <linux/slab.h>
> #include <linux/cleanup.h>
>
> struct S {
>         int x;
> };
>
> static struct S* f(void)
> {
>         struct S* s = kmalloc(sizeof(struct S), GFP_KERNEL);
>         s = NULL;
>         return s ?: ERR_PTR(-ENOMEM);
> }
>
> static int __init xxx_module_init(void)
> {
>         struct S *s __free(kfree) = NULL;
>         s = f();
>         if (IS_ERR(s)) {
>                 return PTR_ERR(s);
>         }
>         return 0;
> }
>
> static void __exit xxx_module_exit(void)
> {
> }
> module_init(xxx_module_init);
> module_exit(xxx_module_exit);
> MODULE_LICENSE("GPL");