Commit 944d671d authored by Yunjian Wang's avatar Yunjian Wang Committed by David S. Miller

sch_htb: fix refcount leak in htb_parent_to_leaf_offload

The commit ae81feb7 ("sch_htb: fix null pointer dereference
on a null new_q") fixes a NULL pointer dereference bug, but it
is not correct.

Because htb_graft_helper properly handles the case when new_q
is NULL, and after the previous patch by skipping this call
which creates an inconsistency : dev_queue->qdisc will still
point to the old qdisc, but cl->parent->leaf.q will point to
the new one (which will be noop_qdisc, because new_q was NULL).
The code is based on an assumption that these two pointers are
the same, so it can lead to refcount leaks.

The correct fix is to add a NULL pointer check to protect
qdisc_refcount_inc inside htb_parent_to_leaf_offload.

Fixes: ae81feb7 ("sch_htb: fix null pointer dereference on a null new_q")
Signed-off-by: default avatarYunjian Wang <wangyunjian@huawei.com>
Suggested-by: default avatarMaxim Mikityanskiy <maximmi@nvidia.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 26821ecd
......@@ -1488,7 +1488,8 @@ static void htb_parent_to_leaf_offload(struct Qdisc *sch,
struct Qdisc *old_q;
/* One ref for cl->leaf.q, the other for dev_queue->qdisc. */
qdisc_refcount_inc(new_q);
if (new_q)
qdisc_refcount_inc(new_q);
old_q = htb_graft_helper(dev_queue, new_q);
WARN_ON(!(old_q->flags & TCQ_F_BUILTIN));
}
......@@ -1675,10 +1676,9 @@ static int htb_delete(struct Qdisc *sch, unsigned long arg,
cl->parent->common.classid,
NULL);
if (q->offload) {
if (new_q) {
if (new_q)
htb_set_lockdep_class_child(new_q);
htb_parent_to_leaf_offload(sch, dev_queue, new_q);
}
htb_parent_to_leaf_offload(sch, dev_queue, new_q);
}
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment