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

[Bartosz Golaszewski]

On Fri, 15 Sept 2023 at 11:56, Alexey Dobriyan <adobriyan@xxxxxxxxx> 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");