On Fri, Apr 13, 2018 at 10:57:03AM -0400, David Miller wrote: > From: Guillaume Nault <g.na...@alphalink.fr> > Date: Thu, 12 Apr 2018 20:50:33 +0200 > > > l2tp_tunnel_find_nth() is unsafe: no reference is held on the returned > > tunnel, therefore it can be freed whenever the caller uses it. > > This patch defines l2tp_tunnel_get_nth() which works similarly, but > > also takes a reference on the returned tunnel. The caller then has to > > drop it after it stops using the tunnel. > > > > Convert netlink dumps to make them safe against concurrent tunnel > > deletion. > > > > Fixes: 309795f4bec2 ("l2tp: Add netlink control API for L2TP") > > Signed-off-by: Guillaume Nault <g.na...@alphalink.fr> > > During the entire invocation of l2tp_nl_cmd_tunnel_dump(), the RTNL > mutex is held. > > Therefore no tunnel configuration changes may occur and the tunnel > object will persist and is safe to access. > Yes, but only for updates done with the genl API. For L2TPv2, the tunnel can be created by connecting a PPPOL2TP and a UDP socket. Closing these sockets destroys the tunnel without any RTNL synchronisation.
> The netlink dump should be safe as-is. > > Were you actually able to trigger a crash or KASAN warning or is > this purely from code inspection? > Yes, I have a KASAN use-after-free for this case. I remember I saw a few complains about stack traces in commit messages, so I've stopped putting them there. I can paste (stripped) traces again. Just let me know if you have any preference. Guillaume