lkml.org 
[lkml]   [2021]   [Jun]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.12 058/161] sch_htb: fix refcount leak in htb_parent_to_leaf_offload
    Date
    From: Yunjian Wang <wangyunjian@huawei.com>

    [ Upstream commit 944d671d5faa0d78980a3da5c0f04960ef1ad893 ]

    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@huawei.com>
    Suggested-by: Maxim Mikityanskiy <maximmi@nvidia.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    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 081c11d5717c..8827987ba903 100644
    --- a/net/sched/sch_htb.c
    +++ b/net/sched/sch_htb.c
    @@ -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);
    }
    }

    --
    2.30.2


    \
     
     \ /
      Last update: 2021-06-08 21:17    [W:2.165 / U:0.148 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site