Message ID | 20190610063821.27007-1-jian.w.wen@oracle.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | [net] net_sched: sch_mqprio: handle return value of mqprio_queue_get | expand |
From: Jacob Wen <jian.w.wen@oracle.com> Date: Mon, 10 Jun 2019 14:38:21 +0800 > It may return NULL thus we can't ignore it. Please repost with a proper signoff.
From: David Miller <davem@davemloft.net> Date: Mon, 10 Jun 2019 09:05:26 -0700 (PDT) > From: Jacob Wen <jian.w.wen@oracle.com> > Date: Mon, 10 Jun 2019 14:38:21 +0800 > >> It may return NULL thus we can't ignore it. > > Please repost with a proper signoff. Also, you are breaking the reverse christmas tree ordering of the local variables with your change. Please do not do that.
On Sun, Jun 9, 2019 at 11:41 PM Jacob Wen <jian.w.wen@oracle.com> wrote: > > It may return NULL thus we can't ignore it. How is this possible? All of the callers should have validated the 'cl' before calling this, for example by calling ->find(). I don't see it is possible to be NULL at this point. Did you see a real crash? If so, please put the full stack trace in your changelog. Thanks!
Hi Cong, This was detected by a tool. Thanks for the review :) I will abandon the patch. On 6/11/19 2:19 AM, Cong Wang wrote: > On Sun, Jun 9, 2019 at 11:41 PM Jacob Wen <jian.w.wen@oracle.com> wrote: >> It may return NULL thus we can't ignore it. > How is this possible? All of the callers should have validated > the 'cl' before calling this, for example by calling ->find(). > > I don't see it is possible to be NULL at this point. > > Did you see a real crash? If so, please put the full stack trace > in your changelog. > > Thanks!
diff --git a/net/sched/sch_mqprio.c b/net/sched/sch_mqprio.c index d05086dc3866..d926056f72ac 100644 --- a/net/sched/sch_mqprio.c +++ b/net/sched/sch_mqprio.c @@ -491,9 +491,12 @@ static int mqprio_dump_class(struct Qdisc *sch, unsigned long cl, struct sk_buff *skb, struct tcmsg *tcm) { if (cl < TC_H_MIN_PRIORITY) { - struct netdev_queue *dev_queue = mqprio_queue_get(sch, cl); struct net_device *dev = qdisc_dev(sch); int tc = netdev_txq_to_tc(dev, cl - 1); + struct netdev_queue *dev_queue = mqprio_queue_get(sch, cl); + + if (!dev_queue) + return -EINVAL; tcm->tcm_parent = (tc < 0) ? 0 : TC_H_MAKE(TC_H_MAJ(sch->handle), @@ -558,6 +561,8 @@ static int mqprio_dump_class_stats(struct Qdisc *sch, unsigned long cl, return -1; } else { struct netdev_queue *dev_queue = mqprio_queue_get(sch, cl); + if (!dev_queue) + return -1; sch = dev_queue->qdisc_sleeping; if (gnet_stats_copy_basic(qdisc_root_sleeping_running(sch),