netdev - Re: [PATCH net] l2tp: fix l2tp

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20170928151410.jmsvdcdr6h3xlffg@alphalink.fr>
Date:   Thu, 28 Sep 2017 17:14:10 +0200
From:   Guillaume Nault <g.nault@...halink.fr>
To:     Tom Parkin <tparkin@...alix.com>
Cc:     netdev@...r.kernel.org, James Chapman <jchapman@...alix.com>
Subject: Re: [PATCH net] l2tp: fix l2tp_eth module loading

On Thu, Sep 28, 2017 at 03:17:28PM +0100, Tom Parkin wrote:
> On Thu, Sep 28, 2017 at 03:44:38PM +0200, Guillaume Nault wrote:
> > The l2tp_eth module crashes if its netlink callbacks are run when the
> > pernet data aren't initialised.
> > 
> > We should normally register_pernet_device() before the genl callbacks.
> > However, the pernet data only maintain a list of l2tpeth interfaces,
> > and this list is never used. So let's just drop pernet handling
> > instead.
> > 
> > Fixes: d9e31d17ceba ("l2tp: Add L2TP ethernet pseudowire support")
> > Signed-off-by: Guillaume Nault <g.nault@...halink.fr>
> 
> Whoops.  I think this was intended to clear up the devices in the net
> namespace,
Yes, that's what I thought too. That's what virtual devices are
supposed to do and think I'll eventually implement this at a later
time.

> but since l2tp_core.c already deletes tunnels on namespace
> exit I don't think it's necessary for l2tp_eth.c to do anything more.
> 
Well, removing l2tpeth devices is just a side effect of closing the
tunnel. But the tunnel may be in a different namespace than the device,
in case the later was moved after creation. In this case, the l2tpeth
interface isn't deleted when its namespace is destroyed. It's moved
to the initial namespace instead (because, for now, l2tpeth devices
don't implement ->rtnl_link_ops).

These are shortcomings I'd like to fix, but there are more important
issues to tackle first.