Re: [PATCH] appletalk: Fix potential refcount leak
From: Eric Dumazet <edumazet@google.com>
Date: 2022-10-24 17:01:41
On Mon, Oct 24, 2022 at 8:25 AM Liang He [off-list ref] wrote:
quoted hunk ↗ jump to hunk
In atrtr_create(), we have added a dev_hold for the new reference. However, based on the code, if the 'rt' is not NULL and its 'dev' is not NULL, we should use dev_put() for the replaced reference. Signed-off-by: Liang He <redacted> --- net/appletalk/ddp.c | 1 + 1 file changed, 1 insertion(+)diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c index a06f4d4a6f47..7e317d6448d1 100644 --- a/net/appletalk/ddp.c +++ b/net/appletalk/ddp.c@@ -564,6 +564,7 @@ static int atrtr_create(struct rtentry *r, struct net_device *devhint) /* Fill in the routing entry */ rt->target = ta->sat_addr; dev_hold(devhint); + dev_put(rt->dev); rt->dev = devhint; rt->flags = r->rt_flags; rt->gateway = ga->sat_addr;
IMO appletalk is probably completely broken. atalk_routes_lock is not held while other threads might use rt->dev and would not expect rt->dev to be changed under them (atalk_route_packet() ) I would vote to remove it completely, unless someone is willing to test any change in it.