netdev - [net/netlink] Question about potential memleak in netlink_proto

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

Message-ID: <ZfOalln/myRNOkH6@cy-server>
Date: Thu, 14 Mar 2024 19:47:18 -0500
From: Chenyuan Yang <chenyuan0y@...il.com>
To: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, horms@...nel.org, anjali.k.kulkarni@...cle.com,
	pctammela@...atatu.com, andriy.shevchenko@...ux.intel.com,
	dhowells@...hat.com, kuniyu@...zon.com
Cc: netdev@...r.kernel.org, zzjas98@...il.com
Subject: [net/netlink] Question about potential memleak in
 netlink_proto_init()

Dear Netlink Developers,

We are curious whether the function `netlink_proto_init()` might have a memory leak.

The function is https://elixir.bootlin.com/linux/v6.8/source/net/netlink/af_netlink.c#L2908
and the relevant code is
```
static int __init netlink_proto_init(void)
{
	int i;
  ...

	for (i = 0; i < MAX_LINKS; i++) {
		if (rhashtable_init(&nl_table[i].hash,
				    &netlink_rhashtable_params) < 0) {
			while (--i > 0)
				rhashtable_destroy(&nl_table[i].hash);
			kfree(nl_table);
			goto panic;
		}
	}
  ...
}
```

In the for loop, when `rhashtable_init()` fails, the function will free the allocated memory for `nl_table[i].hash` by checking `while (--i > 0)`. However, the first element (`i=1`) of `nl_table` is not freed since `i` is decremented before the check.

Based on our understanding, a possible fix would be
```
-      while (--i > 0)
+      while (--i >= 0)
```

Please kindly correct us if we missed any key information. Looking forward to your response!

Best,
Chenyuan