net/hsr: Fix NULL pointer dereference and refcnt bugs when deleting a HSR interface.
Subsystems (3)
PATCH v2 1/1 net/hsr: Fix NULL pointer dereference and refcnt bugs when deleting a HSR interface.
To repeat:
$ sudo ip link del hsr0 BUG: unable to handle kernel NULL pointer dereference at 0000000000000018 IP: [<ffffffff8187f495>] hsr_del_port+0x15/0xa0 etc...
Bug description:
As part of the hsr master device destruction, hsr_del_port() is called for each of the hsr ports. At each such call, the master device is updated regarding features and mtu. When the master device is freed before the slave interfaces, master will be NULL in hsr_del_port(), which led to a NULL pointer dereference.
Additionally, dev_put() was called on the master device itself in hsr_del_port(), causing a refcnt error.
A third bug in the same code path was that the rtnl lock was not taken before hsr_del_port() was called as part of hsr_dev_destroy().
The reporter (Nicolas Dichtel) also said: "hsr_netdev_notify() supposes that the port will always be available when the notification is for an hsr interface. It's wrong. For example, netdev_wait_allrefs() may resend NETDEV_UNREGISTER.". As a precaution against this, a check for port == NULL was added in hsr_dev_notify().
--- net/hsr/hsr_device.c | 3 +++ net/hsr/hsr_main.c | 4 ++++ net/hsr/hsr_slave.c | 10 +++++++--- 3 files changed, 14 insertions(+), 3 deletions(-)
net/hsr/hsr_device.c +3 -0
@@ -359,8 +359,11 @@ static void hsr_dev_destroy(struct net_device *hsr_dev)struct hsr_port *port; hsr =netdev_priv (hsr_dev); + +rtnl_lock ();hsr_for_each_port (hsr, port)hsr_del_port (port); +rtnl_unlock ();del_timer_sync (&hsr->prune_timer);del_timer_sync (&hsr->announce_timer);
net/hsr/hsr_main.c +4 -0
@@ -36,6 +36,10 @@ static int hsr_netdev_notify(struct notifier_block *nb, unsigned long event,return NOTIFY_DONE ;/* Not an HSR device */ hsr =netdev_priv (dev); port =hsr_port_get_hsr (hsr,HSR_PT_MASTER ); +if (port ==NULL ) { +/* Resend of notification concerning removed device? */ +return NOTIFY_DONE ; + } }else { hsr = port->hsr; }
net/hsr/hsr_slave.c +7 -3
@@ -181,8 +181,10 @@ void hsr_del_port(struct hsr_port *port)list_del_rcu (&port->port_list);if (port != master) { -netdev_update_features (master->dev); -dev_set_mtu (master->dev,hsr_get_max_mtu (hsr)); +if (master !=NULL ) { +netdev_update_features (master->dev); +dev_set_mtu (master->dev,hsr_get_max_mtu (hsr)); + }netdev_rx_handler_unregister (port->dev);dev_set_promiscuity (port->dev,-1 ); }
@@ -192,5 +194,7 @@ void hsr_del_port(struct hsr_port *port)*/ synchronize_rcu (); -dev_put (port->dev); + +if (port != master) +dev_put (port->dev); }
Signature · 5 lines
2.0.5 Arvid Brodin | Consultant (Linux) ALTEN | Knarrarnäsgatan 7 | SE-164 40 Kista | Sweden [email protected] | www.alten.se/en/
From: Arvid Brodin <[email protected]>
Date: Fri, 27 Feb 2015 21:26:03 +0100
Quoted 20 lines
To repeat:
$ sudo ip link del hsr0 BUG: unable to handle kernel NULL pointer dereference at 0000000000000018 IP: [<ffffffff8187f495>] hsr_del_port+0x15/0xa0 etc...Bug description:
As part of the hsr master device destruction,
hsr_del_port()is called for each of the hsr ports. At each such call, the master device is updated regarding features and mtu. When the master device is freed before the slave interfaces, master will be NULL inhsr_del_port(), which led to a NULL pointer dereference.Additionally,
dev_put()was called on the master device itself inhsr_del_port(), causing a refcnt error.A third bug in the same code path was that the rtnl lock was not taken before
hsr_del_port()was called as part ofhsr_dev_destroy().
Quoted 8 lines
The reporter (Nicolas Dichtel) also said: "
hsr_netdev_notify()supposes that the port will always be available when the notification is for an hsr interface. It's wrong. For example,netdev_wait_allrefs()may resendNETDEV_UNREGISTER.". As a precaution against this, a check for port == NULL was added inhsr_dev_notify().Reported-by Nicolas Dichtel <[email protected]>Fixes 51f3c605318b056a ("net/hsr: Move slave init to hsr_slave.c.")Signed-off-by Arvid Brodin <[email protected]>
Applied.