From: Daniel Lezcano <dlezcano@fr•ibm.com>
To: Pavel Emelyanov <xemul@openvz•org>
Cc: David Miller <davem@davemloft•net>,
Linux Netdev List <netdev@vger•kernel.org>
Subject: Re: [PATCH][NETNS]: Fix arbitrary net_device-s corruptions on net_ns stop.
Date: Wed, 07 May 2008 17:33:57 +0200 [thread overview]
Message-ID: <4821CBE5.5090801@fr.ibm.com> (raw)
In-Reply-To: <48216398.9050509@openvz.org>
Pavel Emelyanov wrote:
> When a net namespace is destroyed, some devices (those, not killed
> on ns stop explicitly) are moved back to init_net.
>
> The problem, is that this net_ns change has one point of failure -
> the __dev_alloc_name() may be called if a name collision occurs (and
> this is easy to trigger). This allocator performs a likely-to-fail
> GFP_ATOMIC allocation to find a suitable number. Other possible
> conditions that may cause error (for device being ns local or not
> registered) are always false in this case.
>
> So, when this call fails, the device is unregistered. But this is
> *not* the right thing to do, since after this the device may be
> released (and kfree-ed) improperly. E. g. bridges require more
> actions (sysfs update, timer disarming, etc.), some other devices
> want to remove their private areas from lists, etc.
>
> I. e. arbitrary use-after-free cases may occur.
>
> The proposed fix is the following: since the only reason for the
> dev_change_net_namespace to fail is the name generation, we may
> give it a unique fall-back name w/o %d-s in it - the dev<ifindex>
> one, since ifindexes are still unique.
>
> So make this change, raise the failure-case printk loglevel to
> EMERG and replace the unregister_netdevice call with BUG().
>
> Signed-off-by: Pavel Emelyanov <xemul@openvz•org>
>
> ---
Good catch :)
> diff --git a/net/core/dev.c b/net/core/dev.c
> index d334446..86da6aa 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4480,17 +4480,19 @@ static void __net_exit default_device_exit(struct net *net)
> rtnl_lock();
> for_each_netdev_safe(net, dev, next) {
> int err;
> + char fb_name[IFNAMSIZ];
>
> /* Ignore unmoveable devices (i.e. loopback) */
> if (dev->features & NETIF_F_NETNS_LOCAL)
> continue;
>
> /* Push remaing network devices to init_net */
> - err = dev_change_net_namespace(dev, &init_net, "dev%d");
> + sprintf(fb_name, "dev%d", dev->ifindex);
The computed interface name can not exceed IFNAMSIZ, 3 ('dev') + 10 (max
int) + 1 ('\0'). In this case there is no risk to corrupt the stack but
may be it is more secure to change that to snprintf(fb_name, IFNAMSIZ,
"dev%d", dev->ifindex), just in case, no ?
> + err = dev_change_net_namespace(dev, &init_net, fb_name);
> if (err) {
> - printk(KERN_WARNING "%s: failed to move %s to init_net: %d\n",
> + printk(KERN_EMERG "%s: failed to move %s to init_net: %d\n",
> __func__, dev->name, err);
> - unregister_netdevice(dev);
> + BUG();
> }
> }
> rtnl_unlock();
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger•kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
next prev parent reply other threads:[~2008-05-07 15:36 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-05-07 8:08 [PATCH][NETNS]: Fix arbitrary net_device-s corruptions on net_ns stop Pavel Emelyanov
2008-05-07 15:33 ` Daniel Lezcano [this message]
2008-05-07 15:37 ` Pavel Emelyanov
2008-05-07 16:12 ` Daniel Lezcano
2008-05-08 8:25 ` David Miller
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=4821CBE5.5090801@fr.ibm.com \
--to=dlezcano@fr$(echo .)ibm.com \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=xemul@openvz$(echo .)org \
/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