net/sched: sch_qfq: Fix null-deref in agg_dequeue

To prevent a potential crash in agg_dequeue (net/sched/sch_qfq.c)
when cl->qdisc->ops->peek(cl->qdisc) returns NULL, we check the return
value before using it, similar to the existing approach in sch_hfsc.c.

To avoid code duplication, the following changes are made:

1. Changed qdisc_warn_nonwc(include/net/pkt_sched.h) into a static
inline function.

2. Moved qdisc_peek_len from net/sched/sch_hfsc.c to
include/net/pkt_sched.h so that sch_qfq can reuse it.

3. Applied qdisc_peek_len in agg_dequeue to avoid crashing.

Signed-off-by: Xiang Mei <xmei5@asu.edu>
Reviewed-by: Cong Wang <xiyou.wangcong@gmail.com>
Link: https://patch.msgid.link/20250705212143.3982664-1-xmei5@asu.edu
Signed-off-by: Paolo Abeni <pabeni@redhat.com>

diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index d7b7b6cd4aa105786c8ce33525ec7f30c0c25317..8a75c73fc5558798186b3ca4622a2606a94bc40a 100644 (file)
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -114,7 +114,6 @@ struct qdisc_rate_table *qdisc_get_rtab(struct tc_ratespec *r,
                                        struct netlink_ext_ack *extack);
 void qdisc_put_rtab(struct qdisc_rate_table *tab);
 void qdisc_put_stab(struct qdisc_size_table *tab);
-void qdisc_warn_nonwc(const char *txt, struct Qdisc *qdisc);
 bool sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
                     struct net_device *dev, struct netdev_queue *txq,
                     spinlock_t *root_lock, bool validate);
@@ -290,4 +289,28 @@ static inline bool tc_qdisc_stats_dump(struct Qdisc *sch,
        return true;
 }
 
+static inline void qdisc_warn_nonwc(const char *txt, struct Qdisc *qdisc)
+{
+       if (!(qdisc->flags & TCQ_F_WARN_NONWC)) {
+               pr_warn("%s: %s qdisc %X: is non-work-conserving?\n",
+                       txt, qdisc->ops->id, qdisc->handle >> 16);
+               qdisc->flags |= TCQ_F_WARN_NONWC;
+       }
+}
+
+static inline unsigned int qdisc_peek_len(struct Qdisc *sch)
+{
+       struct sk_buff *skb;
+       unsigned int len;
+
+       skb = sch->ops->peek(sch);
+       if (unlikely(skb == NULL)) {
+               qdisc_warn_nonwc("qdisc_peek_len", sch);
+               return 0;
+       }
+       len = qdisc_pkt_len(skb);
+
+       return len;
+}
+
 #endif

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 241e86cec9c50d41c7072b457c502c96212a1f44..d7c767b861a4651caac949430f4b71e79b868d50 100644 (file)
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -601,16 +601,6 @@ out:
        qdisc_skb_cb(skb)->pkt_len = pkt_len;
 }
 
-void qdisc_warn_nonwc(const char *txt, struct Qdisc *qdisc)
-{
-       if (!(qdisc->flags & TCQ_F_WARN_NONWC)) {
-               pr_warn("%s: %s qdisc %X: is non-work-conserving?\n",
-                       txt, qdisc->ops->id, qdisc->handle >> 16);
-               qdisc->flags |= TCQ_F_WARN_NONWC;
-       }
-}
-EXPORT_SYMBOL(qdisc_warn_nonwc);
-
 static enum hrtimer_restart qdisc_watchdog(struct hrtimer *timer)
 {
        struct qdisc_watchdog *wd = container_of(timer, struct qdisc_watchdog,

diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 5a7745170e84b144fe268b28b664d99e1af378e1..d8fd35da32a7c6be36df984c1bf3a6ea0118d990 100644 (file)
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -835,22 +835,6 @@ update_vf(struct hfsc_class *cl, unsigned int len, u64 cur_time)
        }
 }
 
-static unsigned int
-qdisc_peek_len(struct Qdisc *sch)
-{
-       struct sk_buff *skb;
-       unsigned int len;
-
-       skb = sch->ops->peek(sch);
-       if (unlikely(skb == NULL)) {
-               qdisc_warn_nonwc("qdisc_peek_len", sch);
-               return 0;
-       }
-       len = qdisc_pkt_len(skb);
-
-       return len;
-}
-
 static void
 hfsc_adjust_levels(struct hfsc_class *cl)
 {

diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index bf1282cb22ebae3a4a8c6fdd4aa3a5773e5593e2..bcce3660887169b34733228bd186b30092fa50d9 100644 (file)
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -989,7 +989,7 @@ static struct sk_buff *agg_dequeue(struct qfq_aggregate *agg,
 
        if (cl->qdisc->q.qlen == 0) /* no more packets, remove from list */
                list_del_init(&cl->alist);
-       else if (cl->deficit < qdisc_pkt_len(cl->qdisc->ops->peek(cl->qdisc))) {
+       else if (cl->deficit < qdisc_peek_len(cl->qdisc)) {
                cl->deficit += agg->lmax;
                list_move_tail(&cl->alist, &agg->active);
        }