Re: [PATCH] i2c: qcom-cci: fix device_node refcount leak in cci_probe()/cci_remove()

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: [email protected]
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Liu Zhenlong <[email protected]>
> ---
>  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