* [PATCH] i2c: Fix reference leak in of_i2c_register_devices @ 2025-04-06 13:48 Sunny Patel 2025-04-06 18:06 ` Christophe JAILLET 0 siblings, 1 reply; 5+ messages in thread From: Sunny Patel @ 2025-04-06 13:48 UTC (permalink / raw) To: Wolfram Sang; +Cc: linux-i2c, linux-kernel, Sunny Patel Fix a potential reference leak in of_i2c_register_devices where the reference to the node is not released if device registration fails. This ensures proper reference management and avoids memory leaks. Signed-off-by: Sunny Patel <nueralspacetech@gmail.com> --- drivers/i2c/i2c-core-of.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c index 02feee6c9ba9..7c50905de8f1 100644 --- a/drivers/i2c/i2c-core-of.c +++ b/drivers/i2c/i2c-core-of.c @@ -107,6 +107,7 @@ void of_i2c_register_devices(struct i2c_adapter *adap) "Failed to create I2C device for %pOF\n", node); of_node_clear_flag(node, OF_POPULATED); + of_node_put(node); } } -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] i2c: Fix reference leak in of_i2c_register_devices 2025-04-06 13:48 [PATCH] i2c: Fix reference leak in of_i2c_register_devices Sunny Patel @ 2025-04-06 18:06 ` Christophe JAILLET [not found] ` <CAGNPQObiZA76gva-+_DY8TtpEiJfLL3QDoONYST4npOSkgZ+5g@mail.gmail.com> 0 siblings, 1 reply; 5+ messages in thread From: Christophe JAILLET @ 2025-04-06 18:06 UTC (permalink / raw) To: Sunny Patel, Wolfram Sang; +Cc: linux-i2c, linux-kernel Le 06/04/2025 à 15:48, Sunny Patel a écrit : > Fix a potential reference leak in of_i2c_register_devices where the > reference to the node is not released if device registration fails. > This ensures proper reference management and avoids memory leaks. There is no early exit path in the for_each_available_child_of_node() block, so of_node_put((node) is called for all the nodes that are iterated. Can you elaborate and explain how the reference leak can occur? CJ > > Signed-off-by: Sunny Patel <nueralspacetech@gmail.com> > --- > drivers/i2c/i2c-core-of.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c > index 02feee6c9ba9..7c50905de8f1 100644 > --- a/drivers/i2c/i2c-core-of.c > +++ b/drivers/i2c/i2c-core-of.c > @@ -107,6 +107,7 @@ void of_i2c_register_devices(struct i2c_adapter *adap) > "Failed to create I2C device for %pOF\n", > node); > of_node_clear_flag(node, OF_POPULATED); > + of_node_put(node); > } > } > ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <CAGNPQObiZA76gva-+_DY8TtpEiJfLL3QDoONYST4npOSkgZ+5g@mail.gmail.com>]
* Re: [PATCH] i2c: Fix reference leak in of_i2c_register_devices [not found] ` <CAGNPQObiZA76gva-+_DY8TtpEiJfLL3QDoONYST4npOSkgZ+5g@mail.gmail.com> @ 2025-04-08 19:04 ` Christophe JAILLET 2025-04-14 15:42 ` Sunny Patel 0 siblings, 1 reply; 5+ messages in thread From: Christophe JAILLET @ 2025-04-08 19:04 UTC (permalink / raw) To: Sunny; +Cc: Wolfram Sang, linux-i2c, linux-kernel Hi, first of all, you should not reply with HTML mail, because most mailing lists reject such messages. Le 08/04/2025 à 16:01, Sunny a écrit : > To clarify, while there is no early exit path in the > for_each_available_child_of_node() loop, the reference leak can still > occur because the macro increments the reference count for each node > it processes. If i2c_new_client_device() fails, the reference count > for that node must be explicitly decremented using of_node_put(node). > Without this, the reference count remains elevated, leading to a > reference leak. I think you are wrong. Yes, for_each_available_child_of_node() increments the reference count for each node it processes, but it also decrements it at the end of the iteration, except when there is an early exit (a break or a return). See how the 'parent' and 'child' parameters of of_get_next_available_child() are used, especially whtat happen with the first call with child = NULL. So should i2c_new_client_device() fail or succeed, 'node' is released and the reference count does NOT remain elevated as you state. If you do not agree, please give more details of how you think it works and where the issue is, for exemple with unrolling the loop and noting what and when nodes are get and put. You should then see that it is correct. The of_node_put(node) you are looking for is there: https://elixir.bootlin.com/linux/v6.14-rc6/source/drivers/of/base.c#L702 CJ > > sunny > > On Sun, 6 Apr 2025 at 23:36, Christophe JAILLET > <christophe.jaillet@wanadoo.fr> wrote: > > Le 06/04/2025 à 15:48, Sunny Patel a écrit : > > Fix a potential reference leak in of_i2c_register_devices where the > > reference to the node is not released if device registration fails. > > This ensures proper reference management and avoids memory leaks. > > There is no early exit path in the for_each_available_child_of_node() > block, so of_node_put((node) is called for all the nodes that are > iterated. > > Can you elaborate and explain how the reference leak can occur? > > CJ > > > > > Signed-off-by: Sunny Patel <nueralspacetech@gmail.com> > > --- > > drivers/i2c/i2c-core-of.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c > > index 02feee6c9ba9..7c50905de8f1 100644 > > --- a/drivers/i2c/i2c-core-of.c > > +++ b/drivers/i2c/i2c-core-of.c > > @@ -107,6 +107,7 @@ void of_i2c_register_devices(struct > i2c_adapter *adap) > > "Failed to create I2C device for > %pOF\n", > > node); > > of_node_clear_flag(node, OF_POPULATED); > > + of_node_put(node); > > } > > } > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] i2c: Fix reference leak in of_i2c_register_devices 2025-04-08 19:04 ` Christophe JAILLET @ 2025-04-14 15:42 ` Sunny Patel 2025-04-16 22:11 ` Christophe JAILLET 0 siblings, 1 reply; 5+ messages in thread From: Sunny Patel @ 2025-04-14 15:42 UTC (permalink / raw) To: christophe.jaillet; +Cc: linux-i2c, linux-kernel, Sunny Patel Fix a potential reference leak in of_i2c_register_devices where the reference to the node is not released if device registration fails. This ensures proper reference management and avoids memory leaks. Signed-off-by: Sunny Patel <nueralspacetech@gmail.com> --- drivers/i2c/i2c-core-of.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c index 02feee6c9ba9..7c50905de8f1 100644 --- a/drivers/i2c/i2c-core-of.c +++ b/drivers/i2c/i2c-core-of.c @@ -107,6 +107,7 @@ void of_i2c_register_devices(struct i2c_adapter *adap) "Failed to create I2C device for %pOF\n", node); of_node_clear_flag(node, OF_POPULATED); + of_node_put(node); } } -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] i2c: Fix reference leak in of_i2c_register_devices 2025-04-14 15:42 ` Sunny Patel @ 2025-04-16 22:11 ` Christophe JAILLET 0 siblings, 0 replies; 5+ messages in thread From: Christophe JAILLET @ 2025-04-16 22:11 UTC (permalink / raw) To: Sunny Patel; +Cc: linux-i2c, linux-kernel Le 14/04/2025 à 17:42, Sunny Patel a écrit : > Fix a potential reference leak in of_i2c_register_devices where the > reference to the node is not released if device registration fails. > This ensures proper reference management and avoids memory leaks. > > Signed-off-by: Sunny Patel <nueralspacetech@gmail.com> > --- > drivers/i2c/i2c-core-of.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c > index 02feee6c9ba9..7c50905de8f1 100644 > --- a/drivers/i2c/i2c-core-of.c > +++ b/drivers/i2c/i2c-core-of.c > @@ -107,6 +107,7 @@ void of_i2c_register_devices(struct i2c_adapter *adap) > "Failed to create I2C device for %pOF\n", > node); > of_node_clear_flag(node, OF_POPULATED); > + of_node_put(node); > } > } > Hi, Why resending this patch? it is the same has before,, so my understanding is still the same and I still think that it is just wrong. See [1]. CJ [1]: https://lore.kernel.org/all/1dacfce7-c66d-44c6-9a0c-2dd00bc24ffc@wanadoo.fr/ ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-04-16 22:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-06 13:48 [PATCH] i2c: Fix reference leak in of_i2c_register_devices Sunny Patel
2025-04-06 18:06 ` Christophe JAILLET
[not found] ` <CAGNPQObiZA76gva-+_DY8TtpEiJfLL3QDoONYST4npOSkgZ+5g@mail.gmail.com>
2025-04-08 19:04 ` Christophe JAILLET
2025-04-14 15:42 ` Sunny Patel
2025-04-16 22:11 ` Christophe JAILLET
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox