public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Nicolas Dichtel <nicolas.dichtel@6wind•com>
To: Willem de Bruijn <willemb@google•com>,
	davem@davemloft•net, edumazet@google•com, netdev@vger•kernel.org
Subject: Re: [PATCH net] sit: fix use after free of fb_tunnel_dev
Date: Thu, 14 Nov 2013 11:31:49 +0100	[thread overview]
Message-ID: <5284A695.2090501@6wind.com> (raw)
In-Reply-To: <1384396058-26850-1-git-send-email-willemb@google.com>

Le 14/11/2013 03:27, Willem de Bruijn a écrit :
> Bug: The fallback device is created in sit_init_net and assumed to be
> freed in sit_exit_net. First, it is dereferenced in that function, in
> sit_destroy_tunnels:
>
>          struct net *net = dev_net(sitn->fb_tunnel_dev);
>
> Prior to this, rtnl_unlink_register has removed all devices that match
> rtnl_link_ops == sit_link_ops.
>
> Commit 205983c43700 added the line
>
> +       sitn->fb_tunnel_dev->rtnl_link_ops = &sit_link_ops;
>
> which cases the fallback device to match here and be freed before it
> is last dereferenced.
>
> Fix: This commit adds an explicit .delllink callback to sit_link_ops
> that skips deallocation at rtnl_unlink_register for the fallback
> device. This mechanism is comparable to the one in ip_tunnel.
>
> It also modifies sit_destroy_tunnels and its only caller sit_exit_net
> to avoid the offending dereference in the first place. That double
> lookup is more complicated than required.
>
> Test: The bug is only triggered when CONFIG_NET_NS is enabled. It
> causes a GPF only when CONFIG_DEBUG_SLAB is enabled. Verified that
> this bug exists at the mentioned commit, at davem-net HEAD and at
> 3.11.y HEAD. Verified that it went away after applying this patch.
>
> Fixes: 205983c43700 ("sit: allow to use rtnl ops on fb tunnel")
>
> Signed-off-by: Willem de Bruijn <willemb@google•com>
With your patch, it's no more possible to remove manually the fallback device:
'ip link del sit0', but it's better so ;-)

Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind•com>

  parent reply	other threads:[~2013-11-14 10:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-14  2:27 [PATCH net] sit: fix use after free of fb_tunnel_dev Willem de Bruijn
2013-11-14  2:58 ` Willem de Bruijn
2013-11-14 10:31 ` Nicolas Dichtel [this message]
2013-11-14 14:47 ` Eric Dumazet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5284A695.2090501@6wind.com \
    --to=nicolas.dichtel@6wind$(echo .)com \
    --cc=davem@davemloft$(echo .)net \
    --cc=edumazet@google$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=willemb@google$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox