net_sched: fix error recovery at qdisc creation
authorEric Dumazet <edumazet@google.com>
Fri, 10 Feb 2017 18:31:49 +0000 (10:31 -0800)
committerBen Hutchings <ben@decadent.org.uk>
Sat, 11 Nov 2017 13:34:36 +0000 (13:34 +0000)
commit 87b60cfacf9f17cf71933c6e33b66e68160af71d upstream.

Dmitry reported uses after free in qdisc code [1]

The problem here is that ops->init() can return an error.

qdisc_create_dflt() then call ops->destroy(),
while qdisc_create() does _not_ call it.

Four qdisc chose to call their own ops->destroy(), assuming their caller
would not.

This patch makes sure qdisc_create() calls ops->destroy()
and fixes the four qdisc to avoid double free.

[1]
BUG: KASAN: use-after-free in mq_destroy+0x242/0x290 net/sched/sch_mq.c:33 at addr ffff8801d415d440
Read of size 8 by task syz-executor2/5030
CPU: 0 PID: 5030 Comm: syz-executor2 Not tainted 4.3.5-smp-DEV #119
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
 0000000000000046 ffff8801b435b870 ffffffff81bbbed4 ffff8801db000400
 ffff8801d415d440 ffff8801d415dc40 ffff8801c4988510 ffff8801b435b898
 ffffffff816682b1 ffff8801b435b928 ffff8801d415d440 ffff8801c49880c0
Call Trace:
 [<ffffffff81bbbed4>] __dump_stack lib/dump_stack.c:15 [inline]
 [<ffffffff81bbbed4>] dump_stack+0x6c/0x98 lib/dump_stack.c:51
 [<ffffffff816682b1>] kasan_object_err+0x21/0x70 mm/kasan/report.c:158
 [<ffffffff81668524>] print_address_description mm/kasan/report.c:196 [inline]
 [<ffffffff81668524>] kasan_report_error+0x1b4/0x4b0 mm/kasan/report.c:285
 [<ffffffff81668953>] kasan_report mm/kasan/report.c:305 [inline]
 [<ffffffff81668953>] __asan_report_load8_noabort+0x43/0x50 mm/kasan/report.c:326
 [<ffffffff82527b02>] mq_destroy+0x242/0x290 net/sched/sch_mq.c:33
 [<ffffffff82524bdd>] qdisc_destroy+0x12d/0x290 net/sched/sch_generic.c:953
 [<ffffffff82524e30>] qdisc_create_dflt+0xf0/0x120 net/sched/sch_generic.c:848
 [<ffffffff8252550d>] attach_default_qdiscs net/sched/sch_generic.c:1029 [inline]
 [<ffffffff8252550d>] dev_activate+0x6ad/0x880 net/sched/sch_generic.c:1064
 [<ffffffff824b1db1>] __dev_open+0x221/0x320 net/core/dev.c:1403
 [<ffffffff824b24ce>] __dev_change_flags+0x15e/0x3e0 net/core/dev.c:6858
 [<ffffffff824b27de>] dev_change_flags+0x8e/0x140 net/core/dev.c:6926
 [<ffffffff824f5bf6>] dev_ifsioc+0x446/0x890 net/core/dev_ioctl.c:260
 [<ffffffff824f61fa>] dev_ioctl+0x1ba/0xb80 net/core/dev_ioctl.c:546
 [<ffffffff82430509>] sock_do_ioctl+0x99/0xb0 net/socket.c:879
 [<ffffffff82430d30>] sock_ioctl+0x2a0/0x390 net/socket.c:958
 [<ffffffff816f3b68>] vfs_ioctl fs/ioctl.c:44 [inline]
 [<ffffffff816f3b68>] do_vfs_ioctl+0x8a8/0xe50 fs/ioctl.c:611
 [<ffffffff816f41a4>] SYSC_ioctl fs/ioctl.c:626 [inline]
 [<ffffffff816f41a4>] SyS_ioctl+0x94/0xc0 fs/ioctl.c:617
 [<ffffffff8123e357>] entry_SYSCALL_64_fastpath+0x12/0x17

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
[bwh: Backported to 3.2:
 - Drop changes to sch_hhf (doesn't exist) and sch_sfq (doesn't have this bug)
 - Adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
net/sched/sch_api.c
net/sched/sch_mq.c
net/sched/sch_mqprio.c

index dca6c1a..2751bd8 100644 (file)
@@ -866,6 +866,8 @@ qdisc_create(struct net_device *dev, struct netdev_queue *dev_queue,
 
                return sch;
        }
+       /* ops->init() failed, we call ->destroy() like qdisc_create_dflt() */
+       ops->destroy(sch);
 err_out3:
        dev_put(dev);
        kfree((char *) sch - sch->padded);
index 0a4b2f9..7cf451f 100644 (file)
@@ -52,7 +52,7 @@ static int mq_init(struct Qdisc *sch, struct nlattr *opt)
        /* pre-allocate qdiscs, attachment can't fail */
        priv->qdiscs = kcalloc(dev->num_tx_queues, sizeof(priv->qdiscs[0]),
                               GFP_KERNEL);
-       if (priv->qdiscs == NULL)
+       if (!priv->qdiscs)
                return -ENOMEM;
 
        for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
@@ -60,17 +60,13 @@ static int mq_init(struct Qdisc *sch, struct nlattr *opt)
                qdisc = qdisc_create_dflt(dev_queue, &pfifo_fast_ops,
                                          TC_H_MAKE(TC_H_MAJ(sch->handle),
                                                    TC_H_MIN(ntx + 1)));
-               if (qdisc == NULL)
-                       goto err;
+               if (!qdisc)
+                       return -ENOMEM;
                priv->qdiscs[ntx] = qdisc;
        }
 
        sch->flags |= TCQ_F_MQROOT;
        return 0;
-
-err:
-       mq_destroy(sch);
-       return -ENOMEM;
 }
 
 static void mq_attach(struct Qdisc *sch)
index 28de430..b4a388b 100644 (file)
@@ -117,20 +117,17 @@ static int mqprio_init(struct Qdisc *sch, struct nlattr *opt)
        /* pre-allocate qdisc, attachment can't fail */
        priv->qdiscs = kcalloc(dev->num_tx_queues, sizeof(priv->qdiscs[0]),
                               GFP_KERNEL);
-       if (priv->qdiscs == NULL) {
-               err = -ENOMEM;
-               goto err;
-       }
+       if (!priv->qdiscs)
+               return -ENOMEM;
 
        for (i = 0; i < dev->num_tx_queues; i++) {
                dev_queue = netdev_get_tx_queue(dev, i);
                qdisc = qdisc_create_dflt(dev_queue, &pfifo_fast_ops,
                                          TC_H_MAKE(TC_H_MAJ(sch->handle),
                                                    TC_H_MIN(i + 1)));
-               if (qdisc == NULL) {
-                       err = -ENOMEM;
-                       goto err;
-               }
+               if (!qdisc)
+                       return -ENOMEM;
+
                priv->qdiscs[i] = qdisc;
        }
 
@@ -142,7 +139,7 @@ static int mqprio_init(struct Qdisc *sch, struct nlattr *opt)
                priv->hw_owned = 1;
                err = dev->netdev_ops->ndo_setup_tc(dev, qopt->num_tc);
                if (err)
-                       goto err;
+                       return err;
        } else {
                netdev_set_num_tc(dev, qopt->num_tc);
                for (i = 0; i < qopt->num_tc; i++)
@@ -156,10 +153,6 @@ static int mqprio_init(struct Qdisc *sch, struct nlattr *opt)
 
        sch->flags |= TCQ_F_MQROOT;
        return 0;
-
-err:
-       mqprio_destroy(sch);
-       return err;
 }
 
 static void mqprio_attach(struct Qdisc *sch)