| Message ID | 20260815140931.53297-1-dragonliu2018@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: qcom-cci: fix device_node refcount leak in cci_probe()/cci_remove() | expand |
On 8/15/26 4:09 PM, Liu Zhenlong wrote: > cci_probe() calls of_node_get() to take an extra reference on the > child device_node when assigning it to the adapter device. The > matching of_node_put() calls exist in both the error cleanup path > and cci_remove(), but they are placed after i2c_del_adapter(). > > i2c_del_adapter() clears adap->dev with memset() at the end (commit > bd4bc3dbded9 ("i2c: Clear i2c_adapter.dev on adapter removal")), which > zeroes adap->dev.of_node before of_node_put() runs, turning it into a > no-op. The reference taken by of_node_get() is never released, leaking > the device_node on every cleanup of already-registered adapters and > every adapter removal. > > The commit that added of_node_get() and the matching of_node_put() > calls placed the puts after i2c_del_adapter(), so the bug has been > present since the fix was introduced. > > Cache the pointer before calling i2c_del_adapter(), the same approach > used in i2c-mux (i2c_mux_del_adapters) and mtd (commit 56570bdad5e3 > ("mtd: core: Fix refcount error in del_mtd_device()")). > > The of_node_put() in the i2c_add_adapter() failure path (before any > i2c_del_adapter() runs) is correct and left unchanged. > > Fixes: 02a4a69667a2 ("i2c: qcom-cci: don't put a device tree node before i2c_add_adapter()") > Cc: stable@vger.kernel.org > Assisted-by: Claude:claude-opus-5 > Signed-off-by: Liu Zhenlong <dragonliu2018@gmail.com> > --- > drivers/i2c/busses/i2c-qcom-cci.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c > index bdeda3979c48..61c535b8196f 100644 > --- a/drivers/i2c/busses/i2c-qcom-cci.c > +++ b/drivers/i2c/busses/i2c-qcom-cci.c > @@ -618,8 +618,10 @@ static int cci_probe(struct platform_device *pdev) > > for (--i ; i >= 0; i--) { > if (cci->master[i].cci) { > + struct device_node *node = cci->master[i].adap.dev.of_node; > + > i2c_del_adapter(&cci->master[i].adap); > - of_node_put(cci->master[i].adap.dev.of_node); > + of_node_put(node); The fix seems correct, but the way it's done is still fragile - someone/a bot will surely come around in a couple weeks with a ""simplification"" undoing your change. Would a devm action here be a better choice? Konrad
diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c index bdeda3979c48..61c535b8196f 100644 --- a/drivers/i2c/busses/i2c-qcom-cci.c +++ b/drivers/i2c/busses/i2c-qcom-cci.c @@ -618,8 +618,10 @@ static int cci_probe(struct platform_device *pdev) for (--i ; i >= 0; i--) { if (cci->master[i].cci) { + struct device_node *node = cci->master[i].adap.dev.of_node; + i2c_del_adapter(&cci->master[i].adap); - of_node_put(cci->master[i].adap.dev.of_node); + of_node_put(node); } } disable_clocks: @@ -635,8 +637,10 @@ static void cci_remove(struct platform_device *pdev) for (i = 0; i < cci->data->num_masters; i++) { if (cci->master[i].cci) { + struct device_node *node = cci->master[i].adap.dev.of_node; + i2c_del_adapter(&cci->master[i].adap); - of_node_put(cci->master[i].adap.dev.of_node); + of_node_put(node); cci_halt(cci, i); } }
cci_probe() calls of_node_get() to take an extra reference on the child device_node when assigning it to the adapter device. The matching of_node_put() calls exist in both the error cleanup path and cci_remove(), but they are placed after i2c_del_adapter(). i2c_del_adapter() clears adap->dev with memset() at the end (commit bd4bc3dbded9 ("i2c: Clear i2c_adapter.dev on adapter removal")), which zeroes adap->dev.of_node before of_node_put() runs, turning it into a no-op. The reference taken by of_node_get() is never released, leaking the device_node on every cleanup of already-registered adapters and every adapter removal. The commit that added of_node_get() and the matching of_node_put() calls placed the puts after i2c_del_adapter(), so the bug has been present since the fix was introduced. Cache the pointer before calling i2c_del_adapter(), the same approach used in i2c-mux (i2c_mux_del_adapters) and mtd (commit 56570bdad5e3 ("mtd: core: Fix refcount error in del_mtd_device()")). The of_node_put() in the i2c_add_adapter() failure path (before any i2c_del_adapter() runs) is correct and left unchanged. Fixes: 02a4a69667a2 ("i2c: qcom-cci: don't put a device tree node before i2c_add_adapter()") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-5 Signed-off-by: Liu Zhenlong <dragonliu2018@gmail.com> --- drivers/i2c/busses/i2c-qcom-cci.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)