[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZfRc1eUJJSoaGSpn@smile.fi.intel.com>
Date: Fri, 15 Mar 2024 16:36:05 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Chenyuan Yang <chenyuan0y@...il.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
pabeni@...hat.com, horms@...nel.org, anjali.k.kulkarni@...cle.com,
pctammela@...atatu.com, dhowells@...hat.com, kuniyu@...zon.com,
netdev@...r.kernel.org, zzjas98@...il.com
Subject: Re: [net/netlink] Question about potential memleak in
netlink_proto_init()
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
Powered by blists - more mailing lists