netconsole: fix NULL pointer dereference
authorNikolay Aleksandrov <nikolay@redhat.com>
Wed, 23 Oct 2013 13:04:49 +0000 (15:04 +0200)
committerDavid S. Miller <davem@davemloft.net>
Fri, 25 Oct 2013 23:26:58 +0000 (19:26 -0400)
We need to disable the netconsole (enabled = 0) before setting nt->np.dev
to NULL because otherwise we might still have users after the
netpoll_cleanup() since nt->enabled is set afterwards and we can
have a message which will result in a NULL pointer dereference.
It is very easy to hit dereferences all over the netpoll_send_udp function
by running the following two loops in parallel:
while [ 1 ]; do echo 1 > enabled; echo 0 > enabled; done;
while [ 1 ]; do echo 00:11:22:33:44:55 > remote_mac; done;
(the second loop is to generate messages, it can be done by anything)

We're safe to set nt->np.dev = NULL and nt->enabled = 0 with the spinlock
since it's required in the write_msg() function.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
Reviewed-by: Veacelsav Falico <vfalico@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/netconsole.c

index adeee615dd19f4e0d7c6288466798781a1f8bda7..1505dcb950fa57b44c97dcfe583ffe3fb3d1559d 100644 (file)
@@ -310,6 +310,7 @@ static ssize_t store_enabled(struct netconsole_target *nt,
                             const char *buf,
                             size_t count)
 {
+       unsigned long flags;
        int enabled;
        int err;
 
@@ -342,6 +343,13 @@ static ssize_t store_enabled(struct netconsole_target *nt,
                printk(KERN_INFO "netconsole: network logging started\n");
 
        } else {        /* 0 */
+               /* We need to disable the netconsole before cleaning it up
+                * otherwise we might end up in write_msg() with
+                * nt->np.dev == NULL and nt->enabled == 1
+                */
+               spin_lock_irqsave(&target_list_lock, flags);
+               nt->enabled = 0;
+               spin_unlock_irqrestore(&target_list_lock, flags);
                netpoll_cleanup(&nt->np);
        }