Re: [PATCH] net: ti: icssg-prueth: Fix device_node refcount leak in prueth_probe()

[Xiangwu Lai]

Hi Simon,

Thanks for the review, and for looking into the neighbouring error
paths.

On Mon, Nov 09, 2026 at 04:52:35PM +0000, Simon Horman wrote:
> But I wonder if this nearby check also leaks a reference,
> because while it drops one, wouldn't there be two if the
> condition is met?

I don't think that branch can be entered at all, so it does not
leak in practice. eth0_node and eth1_node are both initialised to
NULL, and each iteration of the port scan loop assigns at most one
of them (the reg == 0 and reg == 1 cases are mutually exclusive),
while for_each_child_of_node() visits each child exactly once. So
the same node can never end up stored in both slots, and the
both-NULL case is already handled by the preceding "neither port0
nor port1 node available" check. As far as I can tell, the equality
comparison is dead code.

The DT error the check describes, two port nodes carrying the same
reg value, does leak a reference though, just not through that
branch: if both ports have reg = <0>, the second assignment simply
overwrites eth0_node, the reference taken on the first node is
lost, and eth1_node stays NULL, so the equality check never fires.

On Mon, Nov 09, 2026 at 04:59:02PM +0000, Simon Horman wrote:
> Sorry, one more thing. Which is not strictly related. But as we're on the
> topic of error paths in this function, I am not convinced of the validity
> of the value of reg being used even when the call to of_property_read_u32
> fails in this loop:

You are right, the error is only logged and the uninitialised value
of reg is then used to select eth0_node or eth1_node. Depending on
the garbage, the node is routed into one of the slots (including
the overwrite case above) or, when the value is neither 0 nor 1,
the reference taken by the of_node_get() right before is leaked, as
the node is not stored anywhere. This is the pre-existing loop
issue I mentioned in my reply to the bots: the "port reg should be
0 or 1" branch leaks the reference even for a valid out-of-range
reg value.

Understood on the patch-set, and thanks for the hint. I will send a
v2 that combines the fixes for the two drivers, keeping them
unchanged and adding a per-driver patch for the port scan loop that
skips a port when reading its reg property fails, only takes a
reference for reg values 0 and 1, and rejects duplicate reg values,
which is what the existing check seems to intend but never actually
catches. Happy to adjust the split if you would prefer the loop
changes as a separate series.

Best regards,
Xiangwu Lai