net_sched: fix error recovery at qdisc creation
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>
This commit is contained in:
parent
1bf960502e
commit
87b60cfacf
|
@ -1019,6 +1019,8 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
|
||||||
|
|
||||||
return sch;
|
return sch;
|
||||||
}
|
}
|
||||||
|
/* ops->init() failed, we call ->destroy() like qdisc_create_dflt() */
|
||||||
|
ops->destroy(sch);
|
||||||
err_out3:
|
err_out3:
|
||||||
dev_put(dev);
|
dev_put(dev);
|
||||||
kfree((char *) sch - sch->padded);
|
kfree((char *) sch - sch->padded);
|
||||||
|
|
|
@ -627,7 +627,9 @@ static int hhf_init(struct Qdisc *sch, struct nlattr *opt)
|
||||||
q->hhf_arrays[i] = hhf_zalloc(HHF_ARRAYS_LEN *
|
q->hhf_arrays[i] = hhf_zalloc(HHF_ARRAYS_LEN *
|
||||||
sizeof(u32));
|
sizeof(u32));
|
||||||
if (!q->hhf_arrays[i]) {
|
if (!q->hhf_arrays[i]) {
|
||||||
hhf_destroy(sch);
|
/* Note: hhf_destroy() will be called
|
||||||
|
* by our caller.
|
||||||
|
*/
|
||||||
return -ENOMEM;
|
return -ENOMEM;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -638,7 +640,9 @@ static int hhf_init(struct Qdisc *sch, struct nlattr *opt)
|
||||||
q->hhf_valid_bits[i] = hhf_zalloc(HHF_ARRAYS_LEN /
|
q->hhf_valid_bits[i] = hhf_zalloc(HHF_ARRAYS_LEN /
|
||||||
BITS_PER_BYTE);
|
BITS_PER_BYTE);
|
||||||
if (!q->hhf_valid_bits[i]) {
|
if (!q->hhf_valid_bits[i]) {
|
||||||
hhf_destroy(sch);
|
/* Note: hhf_destroy() will be called
|
||||||
|
* by our caller.
|
||||||
|
*/
|
||||||
return -ENOMEM;
|
return -ENOMEM;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -52,7 +52,7 @@ static int mq_init(struct Qdisc *sch, struct nlattr *opt)
|
||||||
/* pre-allocate qdiscs, attachment can't fail */
|
/* pre-allocate qdiscs, attachment can't fail */
|
||||||
priv->qdiscs = kcalloc(dev->num_tx_queues, sizeof(priv->qdiscs[0]),
|
priv->qdiscs = kcalloc(dev->num_tx_queues, sizeof(priv->qdiscs[0]),
|
||||||
GFP_KERNEL);
|
GFP_KERNEL);
|
||||||
if (priv->qdiscs == NULL)
|
if (!priv->qdiscs)
|
||||||
return -ENOMEM;
|
return -ENOMEM;
|
||||||
|
|
||||||
for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
|
for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
|
||||||
|
@ -60,18 +60,14 @@ static int mq_init(struct Qdisc *sch, struct nlattr *opt)
|
||||||
qdisc = qdisc_create_dflt(dev_queue, get_default_qdisc_ops(dev, ntx),
|
qdisc = qdisc_create_dflt(dev_queue, get_default_qdisc_ops(dev, ntx),
|
||||||
TC_H_MAKE(TC_H_MAJ(sch->handle),
|
TC_H_MAKE(TC_H_MAJ(sch->handle),
|
||||||
TC_H_MIN(ntx + 1)));
|
TC_H_MIN(ntx + 1)));
|
||||||
if (qdisc == NULL)
|
if (!qdisc)
|
||||||
goto err;
|
return -ENOMEM;
|
||||||
priv->qdiscs[ntx] = qdisc;
|
priv->qdiscs[ntx] = qdisc;
|
||||||
qdisc->flags |= TCQ_F_ONETXQUEUE | TCQ_F_NOPARENT;
|
qdisc->flags |= TCQ_F_ONETXQUEUE | TCQ_F_NOPARENT;
|
||||||
}
|
}
|
||||||
|
|
||||||
sch->flags |= TCQ_F_MQROOT;
|
sch->flags |= TCQ_F_MQROOT;
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
err:
|
|
||||||
mq_destroy(sch);
|
|
||||||
return -ENOMEM;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static void mq_attach(struct Qdisc *sch)
|
static void mq_attach(struct Qdisc *sch)
|
||||||
|
|
|
@ -118,10 +118,8 @@ static int mqprio_init(struct Qdisc *sch, struct nlattr *opt)
|
||||||
/* pre-allocate qdisc, attachment can't fail */
|
/* pre-allocate qdisc, attachment can't fail */
|
||||||
priv->qdiscs = kcalloc(dev->num_tx_queues, sizeof(priv->qdiscs[0]),
|
priv->qdiscs = kcalloc(dev->num_tx_queues, sizeof(priv->qdiscs[0]),
|
||||||
GFP_KERNEL);
|
GFP_KERNEL);
|
||||||
if (priv->qdiscs == NULL) {
|
if (!priv->qdiscs)
|
||||||
err = -ENOMEM;
|
return -ENOMEM;
|
||||||
goto err;
|
|
||||||
}
|
|
||||||
|
|
||||||
for (i = 0; i < dev->num_tx_queues; i++) {
|
for (i = 0; i < dev->num_tx_queues; i++) {
|
||||||
dev_queue = netdev_get_tx_queue(dev, i);
|
dev_queue = netdev_get_tx_queue(dev, i);
|
||||||
|
@ -129,10 +127,9 @@ static int mqprio_init(struct Qdisc *sch, struct nlattr *opt)
|
||||||
get_default_qdisc_ops(dev, i),
|
get_default_qdisc_ops(dev, i),
|
||||||
TC_H_MAKE(TC_H_MAJ(sch->handle),
|
TC_H_MAKE(TC_H_MAJ(sch->handle),
|
||||||
TC_H_MIN(i + 1)));
|
TC_H_MIN(i + 1)));
|
||||||
if (qdisc == NULL) {
|
if (!qdisc)
|
||||||
err = -ENOMEM;
|
return -ENOMEM;
|
||||||
goto err;
|
|
||||||
}
|
|
||||||
priv->qdiscs[i] = qdisc;
|
priv->qdiscs[i] = qdisc;
|
||||||
qdisc->flags |= TCQ_F_ONETXQUEUE | TCQ_F_NOPARENT;
|
qdisc->flags |= TCQ_F_ONETXQUEUE | TCQ_F_NOPARENT;
|
||||||
}
|
}
|
||||||
|
@ -148,7 +145,7 @@ static int mqprio_init(struct Qdisc *sch, struct nlattr *opt)
|
||||||
priv->hw_owned = 1;
|
priv->hw_owned = 1;
|
||||||
err = dev->netdev_ops->ndo_setup_tc(dev, sch->handle, 0, &tc);
|
err = dev->netdev_ops->ndo_setup_tc(dev, sch->handle, 0, &tc);
|
||||||
if (err)
|
if (err)
|
||||||
goto err;
|
return err;
|
||||||
} else {
|
} else {
|
||||||
netdev_set_num_tc(dev, qopt->num_tc);
|
netdev_set_num_tc(dev, qopt->num_tc);
|
||||||
for (i = 0; i < qopt->num_tc; i++)
|
for (i = 0; i < qopt->num_tc; i++)
|
||||||
|
@ -162,10 +159,6 @@ static int mqprio_init(struct Qdisc *sch, struct nlattr *opt)
|
||||||
|
|
||||||
sch->flags |= TCQ_F_MQROOT;
|
sch->flags |= TCQ_F_MQROOT;
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
err:
|
|
||||||
mqprio_destroy(sch);
|
|
||||||
return err;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static void mqprio_attach(struct Qdisc *sch)
|
static void mqprio_attach(struct Qdisc *sch)
|
||||||
|
|
|
@ -743,9 +743,10 @@ static int sfq_init(struct Qdisc *sch, struct nlattr *opt)
|
||||||
q->ht = sfq_alloc(sizeof(q->ht[0]) * q->divisor);
|
q->ht = sfq_alloc(sizeof(q->ht[0]) * q->divisor);
|
||||||
q->slots = sfq_alloc(sizeof(q->slots[0]) * q->maxflows);
|
q->slots = sfq_alloc(sizeof(q->slots[0]) * q->maxflows);
|
||||||
if (!q->ht || !q->slots) {
|
if (!q->ht || !q->slots) {
|
||||||
sfq_destroy(sch);
|
/* Note: sfq_destroy() will be called by our caller */
|
||||||
return -ENOMEM;
|
return -ENOMEM;
|
||||||
}
|
}
|
||||||
|
|
||||||
for (i = 0; i < q->divisor; i++)
|
for (i = 0; i < q->divisor; i++)
|
||||||
q->ht[i] = SFQ_EMPTY_SLOT;
|
q->ht[i] = SFQ_EMPTY_SLOT;
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue