linux-kernel - 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: Windows password security audit tool. GUI, reports in PDF.

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

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

__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);


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");