On Fri, Sep 15, 2017 at 11:08:07AM +0200, Sabrina Dubroca wrote: > The tunnel is currently removed from the list during destruction. This > can lead to a double-free of the struct sock if we try to delete the tunnel > twice fast enough. > > The first delete operation does a lookup (l2tp_tunnel_get), finds the > tunnel, calls l2tp_tunnel_delete, which queues it for deletion by > l2tp_tunnel_del_work. > > The second delete operation also finds the tunnel and calls > l2tp_tunnel_delete. If the workqueue has already fired and started > running l2tp_tunnel_del_work, then l2tp_tunnel_delete will queue the > same tunnel a second time, and try to free the socket again. > > Add a dead flag and remove tunnel from its list earlier. Then we can > remove the check of queue_work's result that was meant to prevent that > race but doesn't.
How do we avoid leaving stale information on the tunnel list for use-cases which don't delete tunnels using netlink? For example the L2TPv2 ppp/socket API depends on sk_destruct to clean up the kernel context on socket destruction. Similarly, userspace may just close the tunnel socket without first making netlink calls to delete the tunnel. By moving the tunnel list removal from l2tp_tunnel_destruct to l2tp_tunnel_delete I can't see how codepaths which don't involve l2tp_tunnel_delete don't end up with a corrupted tunnel list. Tom -- Tom Parkin Katalix Systems Ltd http://www.katalix.com Catalysts for your Embedded Linux software development
signature.asc
Description: Digital signature
