netdev - Re: [PATCH net] l2tp: fix race condition in l2tp_tunnel

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 15 Sep 2017 10:42:59 +0100
From:   Tom Parkin <tparkin@...alix.com>
To:     Sabrina Dubroca <sd@...asysnail.net>
Cc:     netdev@...r.kernel.org, Guillaume Nault <g.nault@...halink.fr>,
        Xin Long <lucien.xin@...il.com>
Subject: Re: [PATCH net] l2tp: fix race condition in l2tp_tunnel_delete

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

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)