[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <1622804598-11164-1-git-send-email-wangyunjian@huawei.com>
Date: Fri, 4 Jun 2021 19:03:18 +0800
From: wangyunjian <wangyunjian@...wei.com>
To: <netdev@...r.kernel.org>
CC: <kuba@...nel.org>, <davem@...emloft.net>, <maximmi@...dia.com>,
<tariqt@...dia.com>, <xiyou.wangcong@...il.com>,
<jhs@...atatu.com>, <dingxiaoxiong@...wei.com>,
Yunjian Wang <wangyunjian@...wei.com>
Subject: [PATCH net] sch_htb: fix refcount leak in htb_parent_to_leaf_offload
From: Yunjian Wang <wangyunjian@...wei.com>
The commit ae81feb7338c ("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: ae81feb7338c ("sch_htb: fix null pointer dereference on a null new_q")
Signed-off-by: Yunjian Wang <wangyunjian@...wei.com>
Suggested-by: Maxim Mikityanskiy <maximmi@...dia.com>
---
net/sched/sch_htb.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 4f9304567dcc..469301a18ff8 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1491,7 +1491,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));
}
@@ -1678,10 +1679,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);
}
}
--
2.23.0
Powered by blists - more mailing lists