Thread (5 messages) 5 messages, 4 authors, 2024-03-16

Re: [net/netlink] Question about potential memleak in netlink_proto_init()

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2024-03-15 14:36:12

On Thu, Mar 14, 2024 at 07:47:18PM -0500, Chenyuan Yang wrote:
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)
The better pattern (and widely used in kernel) is

	while (i--)
Please kindly correct us if we missed any key information. Looking forward to
your response!
-- 
With Best Regards,
Andy Shevchenko

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help