* [PATCH] i3c: fix refcount inconsistency in i3c_master_register
@ 2025-10-08 7:27 Shuhao Fu
2025-10-09 16:17 ` Frank Li
0 siblings, 1 reply; 7+ messages in thread
From: Shuhao Fu @ 2025-10-08 7:27 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: Frank Li, linux-i3c, linux-kernel
In `i3c_master_register`, a possible refcount inconsistency has been
identified, causing possible resource leak.
Function `of_node_get` increases the refcount of `parent->of_node`. If
function `i3c_bus_init` fails, the function returns immediately without
a corresponding decrease, resulting in an inconsistent refcounter.
In this patch, an extra goto label is added to ensure the balance of
refcount when `i3c_bus_init` fails.
Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure")
Signed-off-by: Shuhao Fu <sfual@cse.ust.hk>
---
drivers/i3c/master.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index d946db75d..9f4fe98d2 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -2885,7 +2885,7 @@ int i3c_master_register(struct i3c_master_controller *master,
ret = i3c_bus_init(i3cbus, master->dev.of_node);
if (ret)
- return ret;
+ goto err_put_of_node;
device_initialize(&master->dev);
dev_set_name(&master->dev, "i3c-%d", i3cbus->id);
@@ -2973,6 +2973,9 @@ int i3c_master_register(struct i3c_master_controller *master,
err_put_dev:
put_device(&master->dev);
+err_put_of_node:
+ of_node_put(master->dev.of_node);
+
return ret;
}
EXPORT_SYMBOL_GPL(i3c_master_register);
--
2.39.5 (Apple Git-154)
--
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] i3c: fix refcount inconsistency in i3c_master_register 2025-10-08 7:27 [PATCH] i3c: fix refcount inconsistency in i3c_master_register Shuhao Fu @ 2025-10-09 16:17 ` Frank Li 2025-10-10 6:34 ` Shuhao Fu 0 siblings, 1 reply; 7+ messages in thread From: Frank Li @ 2025-10-09 16:17 UTC (permalink / raw) To: Shuhao Fu; +Cc: Alexandre Belloni, linux-i3c, linux-kernel On Wed, Oct 08, 2025 at 03:27:09PM +0800, Shuhao Fu wrote: > In `i3c_master_register`, a possible refcount inconsistency has been > identified, causing possible resource leak. > > Function `of_node_get` increases the refcount of `parent->of_node`. If > function `i3c_bus_init` fails, the function returns immediately without > a corresponding decrease, resulting in an inconsistent refcounter. > > In this patch, an extra goto label is added to ensure the balance of > refcount when `i3c_bus_init` fails. > > Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure") > Signed-off-by: Shuhao Fu <sfual@cse.ust.hk> > --- > drivers/i3c/master.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > index d946db75d..9f4fe98d2 100644 > --- a/drivers/i3c/master.c > +++ b/drivers/i3c/master.c > @@ -2885,7 +2885,7 @@ int i3c_master_register(struct i3c_master_controller *master, > > ret = i3c_bus_init(i3cbus, master->dev.of_node); > if (ret) > - return ret; > + goto err_put_of_node; I think it'd better to set release function for master dev to release of_node because of_node_put() also missed at i3c_master_unregister() you can refer drivers/base/platform.c Frank > > device_initialize(&master->dev); > dev_set_name(&master->dev, "i3c-%d", i3cbus->id); > @@ -2973,6 +2973,9 @@ int i3c_master_register(struct i3c_master_controller *master, > err_put_dev: > put_device(&master->dev); > > +err_put_of_node: > + of_node_put(master->dev.of_node); > + > return ret; > } > EXPORT_SYMBOL_GPL(i3c_master_register); > -- > 2.39.5 (Apple Git-154) > > > -- > linux-i3c mailing list > linux-i3c@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-i3c -- linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] i3c: fix refcount inconsistency in i3c_master_register 2025-10-09 16:17 ` Frank Li @ 2025-10-10 6:34 ` Shuhao Fu 2025-10-13 20:19 ` Frank Li 0 siblings, 1 reply; 7+ messages in thread From: Shuhao Fu @ 2025-10-10 6:34 UTC (permalink / raw) To: Frank Li; +Cc: Alexandre Belloni, linux-i3c, linux-kernel On Thu, Oct 09, 2025 at 12:17:11PM -0400, Frank Li wrote: > On Wed, Oct 08, 2025 at 03:27:09PM +0800, Shuhao Fu wrote: > > In `i3c_master_register`, a possible refcount inconsistency has been > > identified, causing possible resource leak. > > > > Function `of_node_get` increases the refcount of `parent->of_node`. If > > function `i3c_bus_init` fails, the function returns immediately without > > a corresponding decrease, resulting in an inconsistent refcounter. > > > > In this patch, an extra goto label is added to ensure the balance of > > refcount when `i3c_bus_init` fails. > > > > Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure") > > Signed-off-by: Shuhao Fu <sfual@cse.ust.hk> > > --- > > drivers/i3c/master.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > > index d946db75d..9f4fe98d2 100644 > > --- a/drivers/i3c/master.c > > +++ b/drivers/i3c/master.c > > @@ -2885,7 +2885,7 @@ int i3c_master_register(struct i3c_master_controller *master, > > > > ret = i3c_bus_init(i3cbus, master->dev.of_node); > > if (ret) > > - return ret; > > + goto err_put_of_node; > > I think it'd better to set release function for master dev to release > of_node because of_node_put() also missed at i3c_master_unregister() > > you can refer drivers/base/platform.c > > Frank Do you mean that we should do `of_node_release` in `platform_device_release`, instead of respecting the refcounting via `of_node_put`? > > > > > device_initialize(&master->dev); > > dev_set_name(&master->dev, "i3c-%d", i3cbus->id); > > @@ -2973,6 +2973,9 @@ int i3c_master_register(struct i3c_master_controller *master, > > err_put_dev: > > put_device(&master->dev); > > > > +err_put_of_node: > > + of_node_put(master->dev.of_node); > > + > > return ret; > > } > > EXPORT_SYMBOL_GPL(i3c_master_register); > > -- > > 2.39.5 (Apple Git-154) > > > > > > -- > > linux-i3c mailing list > > linux-i3c@lists.infradead.org > > https://apc01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infradead.org%2Fmailman%2Flistinfo%2Flinux-i3c&data=05%7C02%7Csfual%40connect.ust.hk%7Cbfee3f910b654276330508de074f5750%7C6c1d415239d044ca88d9b8d6ddca0708%7C1%7C0%7C638956234525799989%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=aTeKcQJDC2Av%2FQ1MjtDuoOYrWJ6ZwhbjQbHs%2Fci90jw%3D&reserved=0 -- linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] i3c: fix refcount inconsistency in i3c_master_register 2025-10-10 6:34 ` Shuhao Fu @ 2025-10-13 20:19 ` Frank Li 2025-10-13 21:09 ` Shuhao Fu 0 siblings, 1 reply; 7+ messages in thread From: Frank Li @ 2025-10-13 20:19 UTC (permalink / raw) To: Shuhao Fu; +Cc: Alexandre Belloni, linux-i3c, linux-kernel On Fri, Oct 10, 2025 at 02:34:08PM +0800, Shuhao Fu wrote: > On Thu, Oct 09, 2025 at 12:17:11PM -0400, Frank Li wrote: > > On Wed, Oct 08, 2025 at 03:27:09PM +0800, Shuhao Fu wrote: > > > In `i3c_master_register`, a possible refcount inconsistency has been > > > identified, causing possible resource leak. > > > > > > Function `of_node_get` increases the refcount of `parent->of_node`. If > > > function `i3c_bus_init` fails, the function returns immediately without > > > a corresponding decrease, resulting in an inconsistent refcounter. > > > > > > In this patch, an extra goto label is added to ensure the balance of > > > refcount when `i3c_bus_init` fails. > > > > > > Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure") > > > Signed-off-by: Shuhao Fu <sfual@cse.ust.hk> > > > --- > > > drivers/i3c/master.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > > > index d946db75d..9f4fe98d2 100644 > > > --- a/drivers/i3c/master.c > > > +++ b/drivers/i3c/master.c > > > @@ -2885,7 +2885,7 @@ int i3c_master_register(struct i3c_master_controller *master, > > > > > > ret = i3c_bus_init(i3cbus, master->dev.of_node); > > > if (ret) > > > - return ret; > > > + goto err_put_of_node; > > > > I think it'd better to set release function for master dev to release > > of_node because of_node_put() also missed at i3c_master_unregister() > > > > you can refer drivers/base/platform.c > > > > Frank > > Do you mean that we should do `of_node_release` in > `platform_device_release`, instead of respecting the refcounting via > `of_node_put`? Sorry, I checked code again. static void i3c_masterdev_release(struct device *dev) { ... of_node_put(dev->of_node); } i3c_master_register() { ... master->dev.release = i3c_masterdev_release; ... }; Suppose of_node_put() will be auto called when put_device(&master->dev); Do you really meet the problem or just static anaysis? Frank > > > > > > > > > device_initialize(&master->dev); > > > dev_set_name(&master->dev, "i3c-%d", i3cbus->id); > > > @@ -2973,6 +2973,9 @@ int i3c_master_register(struct i3c_master_controller *master, > > > err_put_dev: > > > put_device(&master->dev); > > > > > > +err_put_of_node: > > > + of_node_put(master->dev.of_node); > > > + > > > return ret; > > > } > > > EXPORT_SYMBOL_GPL(i3c_master_register); > > > -- > > > 2.39.5 (Apple Git-154) > > > > > > > > > -- > > > linux-i3c mailing list > > > linux-i3c@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-i3c > > -- > linux-i3c mailing list > linux-i3c@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-i3c -- linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] i3c: fix refcount inconsistency in i3c_master_register 2025-10-13 20:19 ` Frank Li @ 2025-10-13 21:09 ` Shuhao Fu 2025-10-13 22:01 ` Frank Li 0 siblings, 1 reply; 7+ messages in thread From: Shuhao Fu @ 2025-10-13 21:09 UTC (permalink / raw) To: Frank Li; +Cc: Alexandre Belloni, linux-i3c, linux-kernel On Mon, Oct 13, 2025 at 04:19:00PM -0400, Frank Li wrote: > On Fri, Oct 10, 2025 at 02:34:08PM +0800, Shuhao Fu wrote: > > On Thu, Oct 09, 2025 at 12:17:11PM -0400, Frank Li wrote: > > > On Wed, Oct 08, 2025 at 03:27:09PM +0800, Shuhao Fu wrote: > > > > In `i3c_master_register`, a possible refcount inconsistency has been > > > > identified, causing possible resource leak. > > > > > > > > Function `of_node_get` increases the refcount of `parent->of_node`. If > > > > function `i3c_bus_init` fails, the function returns immediately without > > > > a corresponding decrease, resulting in an inconsistent refcounter. > > > > > > > > In this patch, an extra goto label is added to ensure the balance of > > > > refcount when `i3c_bus_init` fails. > > > > > > > > Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure") > > > > Signed-off-by: Shuhao Fu <sfual@cse.ust.hk> > > > > --- > > > > drivers/i3c/master.c | 5 ++++- > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > > > > index d946db75d..9f4fe98d2 100644 > > > > --- a/drivers/i3c/master.c > > > > +++ b/drivers/i3c/master.c > > > > @@ -2885,7 +2885,7 @@ int i3c_master_register(struct i3c_master_controller *master, > > > > > > > > ret = i3c_bus_init(i3cbus, master->dev.of_node); > > > > if (ret) > > > > - return ret; > > > > + goto err_put_of_node; > > > > > > I think it'd better to set release function for master dev to release > > > of_node because of_node_put() also missed at i3c_master_unregister() > > > > > > you can refer drivers/base/platform.c > > > > > > Frank > > > > Do you mean that we should do `of_node_release` in > > `platform_device_release`, instead of respecting the refcounting via > > `of_node_put`? > > Sorry, I checked code again. > > static void i3c_masterdev_release(struct device *dev) > { > ... > of_node_put(dev->of_node); > } > > i3c_master_register() > { > ... > master->dev.release = i3c_masterdev_release; > ... > }; > > Suppose of_node_put() will be auto called when put_device(&master->dev); > > Do you really meet the problem or just static anaysis? > > Frank Honestly, it's from static analysis. My apologies for overlooking the release handle. I checked the code once again. It still looks suspicious as it would not call `put_device` if it fails. I also checked call sites related to `i3c_master_register` and they dont seem to do the clean-up if register fails. Shuhao > > > > > > > > > > > > > device_initialize(&master->dev); > > > > dev_set_name(&master->dev, "i3c-%d", i3cbus->id); > > > > @@ -2973,6 +2973,9 @@ int i3c_master_register(struct i3c_master_controller *master, > > > > err_put_dev: > > > > put_device(&master->dev); > > > > > > > > +err_put_of_node: > > > > + of_node_put(master->dev.of_node); > > > > + > > > > return ret; > > > > } > > > > EXPORT_SYMBOL_GPL(i3c_master_register); > > > > -- > > > > 2.39.5 (Apple Git-154) > > > > > > > > > > > > -- > > > > linux-i3c mailing list > > > > linux-i3c@lists.infradead.org > > > > https://apc01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infradead.org%2Fmailman%2Flistinfo%2Flinux-i3c&data=05%7C02%7Csfual%40connect.ust.hk%7C837a825f1f3443950e6a08de0a95cb5f%7C6c1d415239d044ca88d9b8d6ddca0708%7C1%7C0%7C638959835659671475%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=B0ZvY%2FDKW3VdG%2FVamjGUUg%2BVr%2BZbtHIf4otgBKhje1s%3D&reserved=0 > > > > -- > > linux-i3c mailing list > > linux-i3c@lists.infradead.org > > https://apc01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infradead.org%2Fmailman%2Flistinfo%2Flinux-i3c&data=05%7C02%7Csfual%40connect.ust.hk%7C837a825f1f3443950e6a08de0a95cb5f%7C6c1d415239d044ca88d9b8d6ddca0708%7C1%7C0%7C638959835659696068%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=KpMM%2FsTa6G1o3q%2Bcqx6iTb3VbCDq723lCXgcA9GGetI%3D&reserved=0 -- linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] i3c: fix refcount inconsistency in i3c_master_register 2025-10-13 21:09 ` Shuhao Fu @ 2025-10-13 22:01 ` Frank Li 2025-10-14 1:55 ` Shuhao Fu 0 siblings, 1 reply; 7+ messages in thread From: Frank Li @ 2025-10-13 22:01 UTC (permalink / raw) To: Shuhao Fu; +Cc: Alexandre Belloni, linux-i3c, linux-kernel On Tue, Oct 14, 2025 at 05:09:53AM +0800, Shuhao Fu wrote: > On Mon, Oct 13, 2025 at 04:19:00PM -0400, Frank Li wrote: > > On Fri, Oct 10, 2025 at 02:34:08PM +0800, Shuhao Fu wrote: > > > On Thu, Oct 09, 2025 at 12:17:11PM -0400, Frank Li wrote: > > > > On Wed, Oct 08, 2025 at 03:27:09PM +0800, Shuhao Fu wrote: > > > > > In `i3c_master_register`, a possible refcount inconsistency has been > > > > > identified, causing possible resource leak. > > > > > > > > > > Function `of_node_get` increases the refcount of `parent->of_node`. If > > > > > function `i3c_bus_init` fails, the function returns immediately without > > > > > a corresponding decrease, resulting in an inconsistent refcounter. > > > > > > > > > > In this patch, an extra goto label is added to ensure the balance of > > > > > refcount when `i3c_bus_init` fails. > > > > > > > > > > Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure") > > > > > Signed-off-by: Shuhao Fu <sfual@cse.ust.hk> > > > > > --- > > > > > drivers/i3c/master.c | 5 ++++- > > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > > > > > index d946db75d..9f4fe98d2 100644 > > > > > --- a/drivers/i3c/master.c > > > > > +++ b/drivers/i3c/master.c > > > > > @@ -2885,7 +2885,7 @@ int i3c_master_register(struct i3c_master_controller *master, > > > > > > > > > > ret = i3c_bus_init(i3cbus, master->dev.of_node); > > > > > if (ret) > > > > > - return ret; > > > > > + goto err_put_of_node; > > > > > > > > I think it'd better to set release function for master dev to release > > > > of_node because of_node_put() also missed at i3c_master_unregister() > > > > > > > > you can refer drivers/base/platform.c > > > > > > > > Frank > > > > > > Do you mean that we should do `of_node_release` in > > > `platform_device_release`, instead of respecting the refcounting via > > > `of_node_put`? > > > > Sorry, I checked code again. > > > > static void i3c_masterdev_release(struct device *dev) > > { > > ... > > of_node_put(dev->of_node); > > } > > > > i3c_master_register() > > { > > ... > > master->dev.release = i3c_masterdev_release; > > ... > > }; > > > > Suppose of_node_put() will be auto called when put_device(&master->dev); > > > > Do you really meet the problem or just static anaysis? > > > > Frank > > Honestly, it's from static analysis. > > My apologies for overlooking the release handle. I checked the code once > again. It still looks suspicious as it would not call `put_device` if it > fails. I also checked call sites related to `i3c_master_register` and > they dont seem to do the clean-up if register fails. @@ -2814,10 +2816,6 @@ int i3c_master_register(struct i3c_master_controller *master, INIT_LIST_HEAD(&master->boardinfo.i2c); INIT_LIST_HEAD(&master->boardinfo.i3c); - ret = i3c_bus_init(i3cbus, master->dev.of_node); - if (ret) - return ret; - device_initialize(&master->dev); dev_set_name(&master->dev, "i3c-%d", i3cbus->id); @@ -2825,6 +2823,10 @@ int i3c_master_register(struct i3c_master_controller *master, master->dev.coherent_dma_mask = parent->coherent_dma_mask; master->dev.dma_parms = parent->dma_parms; + ret = i3c_bus_init(i3cbus, master->dev.of_node); + if (ret) + goto err_put_dev; + I inject at error at i3c_bus_init(), above code can trigger i3c_masterdev_release, which call of_node_put(). Frank > > Shuhao > > > > > > > > > > > > > > > > > device_initialize(&master->dev); > > > > > dev_set_name(&master->dev, "i3c-%d", i3cbus->id); > > > > > @@ -2973,6 +2973,9 @@ int i3c_master_register(struct i3c_master_controller *master, > > > > > err_put_dev: > > > > > put_device(&master->dev); > > > > > > > > > > +err_put_of_node: > > > > > + of_node_put(master->dev.of_node); > > > > > + > > > > > return ret; > > > > > } > > > > > EXPORT_SYMBOL_GPL(i3c_master_register); > > > > > -- > > > > > 2.39.5 (Apple Git-154) > > > > > > > > > > > > > > > -- > > > > > linux-i3c mailing list > > > > > linux-i3c@lists.infradead.org > > > > > http://lists.infradead.org/mailman/listinfo/linux-i3c > > > > > > -- > > > linux-i3c mailing list > > > linux-i3c@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-i3c -- linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] i3c: fix refcount inconsistency in i3c_master_register 2025-10-13 22:01 ` Frank Li @ 2025-10-14 1:55 ` Shuhao Fu 0 siblings, 0 replies; 7+ messages in thread From: Shuhao Fu @ 2025-10-14 1:55 UTC (permalink / raw) To: Frank Li; +Cc: Alexandre Belloni, linux-i3c, linux-kernel On Mon, Oct 13, 2025 at 06:01:19PM -0400, Frank Li wrote: > On Tue, Oct 14, 2025 at 05:09:53AM +0800, Shuhao Fu wrote: > > On Mon, Oct 13, 2025 at 04:19:00PM -0400, Frank Li wrote: > > > On Fri, Oct 10, 2025 at 02:34:08PM +0800, Shuhao Fu wrote: > > > > On Thu, Oct 09, 2025 at 12:17:11PM -0400, Frank Li wrote: > > > > > On Wed, Oct 08, 2025 at 03:27:09PM +0800, Shuhao Fu wrote: > > > > > > In `i3c_master_register`, a possible refcount inconsistency has been > > > > > > identified, causing possible resource leak. > > > > > > > > > > > > Function `of_node_get` increases the refcount of `parent->of_node`. If > > > > > > function `i3c_bus_init` fails, the function returns immediately without > > > > > > a corresponding decrease, resulting in an inconsistent refcounter. > > > > > > > > > > > > In this patch, an extra goto label is added to ensure the balance of > > > > > > refcount when `i3c_bus_init` fails. > > > > > > > > > > > > Fixes: 3a379bbcea0a ("i3c: Add core I3C infrastructure") > > > > > > Signed-off-by: Shuhao Fu <sfual@cse.ust.hk> > > > > > > --- > > > > > > drivers/i3c/master.c | 5 ++++- > > > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > > > > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c > > > > > > index d946db75d..9f4fe98d2 100644 > > > > > > --- a/drivers/i3c/master.c > > > > > > +++ b/drivers/i3c/master.c > > > > > > @@ -2885,7 +2885,7 @@ int i3c_master_register(struct i3c_master_controller *master, > > > > > > > > > > > > ret = i3c_bus_init(i3cbus, master->dev.of_node); > > > > > > if (ret) > > > > > > - return ret; > > > > > > + goto err_put_of_node; > > > > > > > > > > I think it'd better to set release function for master dev to release > > > > > of_node because of_node_put() also missed at i3c_master_unregister() > > > > > > > > > > you can refer drivers/base/platform.c > > > > > > > > > > Frank > > > > > > > > Do you mean that we should do `of_node_release` in > > > > `platform_device_release`, instead of respecting the refcounting via > > > > `of_node_put`? > > > > > > Sorry, I checked code again. > > > > > > static void i3c_masterdev_release(struct device *dev) > > > { > > > ... > > > of_node_put(dev->of_node); > > > } > > > > > > i3c_master_register() > > > { > > > ... > > > master->dev.release = i3c_masterdev_release; > > > ... > > > }; > > > > > > Suppose of_node_put() will be auto called when put_device(&master->dev); > > > > > > Do you really meet the problem or just static anaysis? > > > > > > Frank > > > > Honestly, it's from static analysis. > > > > My apologies for overlooking the release handle. I checked the code once > > again. It still looks suspicious as it would not call `put_device` if it > > fails. I also checked call sites related to `i3c_master_register` and > > they dont seem to do the clean-up if register fails. > > > @@ -2814,10 +2816,6 @@ int i3c_master_register(struct i3c_master_controller *master, > INIT_LIST_HEAD(&master->boardinfo.i2c); > INIT_LIST_HEAD(&master->boardinfo.i3c); > > - ret = i3c_bus_init(i3cbus, master->dev.of_node); > - if (ret) > - return ret; > - > device_initialize(&master->dev); > dev_set_name(&master->dev, "i3c-%d", i3cbus->id); > > @@ -2825,6 +2823,10 @@ int i3c_master_register(struct i3c_master_controller *master, > master->dev.coherent_dma_mask = parent->coherent_dma_mask; > master->dev.dma_parms = parent->dma_parms; > > + ret = i3c_bus_init(i3cbus, master->dev.of_node); > + if (ret) > + goto err_put_dev; > + > > I inject at error at i3c_bus_init(), above code can trigger i3c_masterdev_release, > which call of_node_put(). > > Frank > Thank you for fixing the refcounting issue. May I kindly ask for a reported-by tag for this fix "Reported-by: Shuhao Fu <sfual@cse.ust.hk>"? Thanks, Shuhao > > > > Shuhao > > > > > > > > > > > > > > > > > > > > > device_initialize(&master->dev); > > > > > > dev_set_name(&master->dev, "i3c-%d", i3cbus->id); > > > > > > @@ -2973,6 +2973,9 @@ int i3c_master_register(struct i3c_master_controller *master, > > > > > > err_put_dev: > > > > > > put_device(&master->dev); > > > > > > > > > > > > +err_put_of_node: > > > > > > + of_node_put(master->dev.of_node); > > > > > > + > > > > > > return ret; > > > > > > } > > > > > > EXPORT_SYMBOL_GPL(i3c_master_register); > > > > > > -- > > > > > > 2.39.5 (Apple Git-154) > > > > > > > > > > > > > > > > > > -- > > > > > > linux-i3c mailing list > > > > > > linux-i3c@lists.infradead.org > > > > > > https://apc01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infradead.org%2Fmailman%2Flistinfo%2Flinux-i3c&data=05%7C02%7Csfual%40connect.ust.hk%7Cdbe4f1ecc0c84f304fca08de0aa414ff%7C6c1d415239d044ca88d9b8d6ddca0708%7C1%7C0%7C638959897018898845%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=%2BmCDBh4d6Se%2BuO5xJgSDVupMRir7ZFH7f8RtzGUucoE%3D&reserved=0 > > > > > > > > -- > > > > linux-i3c mailing list > > > > linux-i3c@lists.infradead.org > > > > https://apc01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infradead.org%2Fmailman%2Flistinfo%2Flinux-i3c&data=05%7C02%7Csfual%40connect.ust.hk%7Cdbe4f1ecc0c84f304fca08de0aa414ff%7C6c1d415239d044ca88d9b8d6ddca0708%7C1%7C0%7C638959897018922222%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=MdqJwKEKImd8UAQ0hyHjWyZx8vX1YSxU%2FqKDgpF0JPA%3D&reserved=0 -- linux-i3c mailing list linux-i3c@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-i3c ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-10-14 1:55 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-08 7:27 [PATCH] i3c: fix refcount inconsistency in i3c_master_register Shuhao Fu 2025-10-09 16:17 ` Frank Li 2025-10-10 6:34 ` Shuhao Fu 2025-10-13 20:19 ` Frank Li 2025-10-13 21:09 ` Shuhao Fu 2025-10-13 22:01 ` Frank Li 2025-10-14 1:55 ` Shuhao Fu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox