[PATCH] net/sched: sch_qfq: Fix null-deref in agg_dequeue

[PATCH] net/sched: sch_qfq: Fix null-deref in agg_dequeue

[Xiang Mei]

From: n132 <xmei5@asu.edu>

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. Moved qdisc_warn_nonwc to include/net/sch_generic.h and removed
its EXPORT_SYMBOL declaration, since all users include the header.

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

3. Applied qdisc_peek_len in agg_dequeue to avoid crashing.

Signed-off-by: n132 <xmei5@asu.edu>
---
 include/net/sch_generic.h | 24 ++++++++++++++++++++++++
 net/sched/sch_api.c       | 10 ----------
 net/sched/sch_hfsc.c      | 16 ----------------
 net/sched/sch_qfq.c       |  2 +-
 4 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 3287988a6a98..d090aaa59ef2 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -814,11 +814,35 @@ static inline bool qdisc_tx_is_noop(const struct net_device *dev)
 	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_pkt_len(const struct sk_buff *skb)
 {
 	return qdisc_skb_cb(skb)->pkt_len;
 }
 
+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;
+}
+
 /* additional qdisc xmit flags (NET_XMIT_MASK in linux/netdevice.h) */
 enum net_xmit_qdisc_t {
 	__NET_XMIT_STOLEN = 0x00010000,
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index df89790c459a..6518fdc63dc2 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -594,16 +594,6 @@ void __qdisc_calculate_pkt_len(struct sk_buff *skb,
 	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 afcb83d469ff..751b1e2c35b3 100644
--- 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 5e557b960bde..e0cefa21ce21 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -992,7 +992,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);
 	}
-- 
2.43.0

Re: [PATCH] net/sched: sch_qfq: Fix null-deref in agg_dequeue

[Greg KH]

On Tue, Jul 01, 2025 at 09:02:28PM -0700, Xiang Mei wrote:
> From: n132 <xmei5@asu.edu>
> 
> 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. Moved qdisc_warn_nonwc to include/net/sch_generic.h and removed
> its EXPORT_SYMBOL declaration, since all users include the header.
> 
> 2. Moved qdisc_peek_len from net/sched/sch_hfsc.c to
> include/net/sch_generic.h so that sch_qfq can reuse it.
> 
> 3. Applied qdisc_peek_len in agg_dequeue to avoid crashing.
> 
> Signed-off-by: n132 <xmei5@asu.edu>

We need a real name, sorry.  Please read the documentation for why.

thanks,

greg k-h

[PATCH] net/sched: sch_qfq: Fix null-deref in agg_dequeue

[Xiang Mei]

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. Moved qdisc_warn_nonwc to include/net/sch_generic.h and removed
its EXPORT_SYMBOL declaration, since all users include the header.

2. Moved qdisc_peek_len from net/sched/sch_hfsc.c to
include/net/sch_generic.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>
---
 include/net/sch_generic.h | 24 ++++++++++++++++++++++++
 net/sched/sch_api.c       | 10 ----------
 net/sched/sch_hfsc.c      | 16 ----------------
 net/sched/sch_qfq.c       |  2 +-
 4 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 3287988a6a98..d090aaa59ef2 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -814,11 +814,35 @@ static inline bool qdisc_tx_is_noop(const struct net_device *dev)
 	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_pkt_len(const struct sk_buff *skb)
 {
 	return qdisc_skb_cb(skb)->pkt_len;
 }
 
+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;
+}
+
 /* additional qdisc xmit flags (NET_XMIT_MASK in linux/netdevice.h) */
 enum net_xmit_qdisc_t {
 	__NET_XMIT_STOLEN = 0x00010000,
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index df89790c459a..6518fdc63dc2 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -594,16 +594,6 @@ void __qdisc_calculate_pkt_len(struct sk_buff *skb,
 	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 afcb83d469ff..751b1e2c35b3 100644
--- 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 5e557b960bde..e0cefa21ce21 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -992,7 +992,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);
 	}
-- 
2.43.0

Re: [PATCH] net/sched: sch_qfq: Fix null-deref in agg_dequeue

[Xiang Mei]

I am sorry for the inconvenience and I appreciate your patience and
help. The new patch was sent.


On Wed, Jul 2, 2025 at 12:18 AM Xiang Mei <xmei5@asu.edu> wrote:
>
> 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. Moved qdisc_warn_nonwc to include/net/sch_generic.h and removed
> its EXPORT_SYMBOL declaration, since all users include the header.
>
> 2. Moved qdisc_peek_len from net/sched/sch_hfsc.c to
> include/net/sch_generic.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>
> ---
>  include/net/sch_generic.h | 24 ++++++++++++++++++++++++
>  net/sched/sch_api.c       | 10 ----------
>  net/sched/sch_hfsc.c      | 16 ----------------
>  net/sched/sch_qfq.c       |  2 +-
>  4 files changed, 25 insertions(+), 27 deletions(-)
>
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index 3287988a6a98..d090aaa59ef2 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -814,11 +814,35 @@ static inline bool qdisc_tx_is_noop(const struct net_device *dev)
>         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_pkt_len(const struct sk_buff *skb)
>  {
>         return qdisc_skb_cb(skb)->pkt_len;
>  }
>
> +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;
> +}
> +
>  /* additional qdisc xmit flags (NET_XMIT_MASK in linux/netdevice.h) */
>  enum net_xmit_qdisc_t {
>         __NET_XMIT_STOLEN = 0x00010000,
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index df89790c459a..6518fdc63dc2 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -594,16 +594,6 @@ void __qdisc_calculate_pkt_len(struct sk_buff *skb,
>         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 afcb83d469ff..751b1e2c35b3 100644
> --- 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 5e557b960bde..e0cefa21ce21 100644
> --- a/net/sched/sch_qfq.c
> +++ b/net/sched/sch_qfq.c
> @@ -992,7 +992,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);
>         }
> --
> 2.43.0
>

Re: [PATCH] net/sched: sch_qfq: Fix null-deref in agg_dequeue

[Greg KH]

On Wed, Jul 02, 2025 at 12:18:18AM -0700, Xiang Mei wrote:
> 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. Moved qdisc_warn_nonwc to include/net/sch_generic.h and removed
> its EXPORT_SYMBOL declaration, since all users include the header.
> 
> 2. Moved qdisc_peek_len from net/sched/sch_hfsc.c to
> include/net/sch_generic.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>
> ---
>  include/net/sch_generic.h | 24 ++++++++++++++++++++++++
>  net/sched/sch_api.c       | 10 ----------
>  net/sched/sch_hfsc.c      | 16 ----------------
>  net/sched/sch_qfq.c       |  2 +-
>  4 files changed, 25 insertions(+), 27 deletions(-)
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

[PATCH v2] net/sched: sch_qfq: Fix null-deref in agg_dequeue

[Xiang Mei]

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. Moved qdisc_warn_nonwc to include/net/sch_generic.h and removed
its EXPORT_SYMBOL declaration, since all users include the header.

2. Moved qdisc_peek_len from net/sched/sch_hfsc.c to
include/net/sch_generic.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>
---
v2:
- Use real name in signed-off-by
v1:
- Move qdisc_warn_nonwc to include/net/sch_generic.h
- Move qdisc_peek_len from net/sched/sch_hfsc.c to include/net/sch_generic.h
- Replace  cl->qdisc->ops->peek() with qdisc_peek_len() in sch_qfq.c

 include/net/sch_generic.h | 24 ++++++++++++++++++++++++
 net/sched/sch_api.c       | 10 ----------
 net/sched/sch_hfsc.c      | 16 ----------------
 net/sched/sch_qfq.c       |  2 +-
 4 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 3287988a6a98..d090aaa59ef2 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -814,11 +814,35 @@ static inline bool qdisc_tx_is_noop(const struct net_device *dev)
 	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_pkt_len(const struct sk_buff *skb)
 {
 	return qdisc_skb_cb(skb)->pkt_len;
 }
 
+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;
+}
+
 /* additional qdisc xmit flags (NET_XMIT_MASK in linux/netdevice.h) */
 enum net_xmit_qdisc_t {
 	__NET_XMIT_STOLEN = 0x00010000,
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index df89790c459a..6518fdc63dc2 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -594,16 +594,6 @@ void __qdisc_calculate_pkt_len(struct sk_buff *skb,
 	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 afcb83d469ff..751b1e2c35b3 100644
--- 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 5e557b960bde..e0cefa21ce21 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -992,7 +992,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);
 	}
-- 
2.43.0

Re: [PATCH v2] net/sched: sch_qfq: Fix null-deref in agg_dequeue

[Cong Wang]

On Wed, Jul 02, 2025 at 01:30:01AM -0700, Xiang Mei wrote:
>  include/net/sch_generic.h | 24 ++++++++++++++++++++++++
>  net/sched/sch_api.c       | 10 ----------
>  net/sched/sch_hfsc.c      | 16 ----------------
>  net/sched/sch_qfq.c       |  2 +-
>  4 files changed, 25 insertions(+), 27 deletions(-)

Looks like you missed the declaration of qdisc_warn_nonwc()?

$ git grep qdisc_warn_nonwc -- include/
include/net/pkt_sched.h:void qdisc_warn_nonwc(const char *txt, struct Qdisc *qdisc);

I suggest moving both inlined functions to include/net/pkt_sched.h.
Sorry for not noticing this earlier.

Thanks!

[PATCH v3] net/sched: sch_qfq: Fix null-deref in agg_dequeue

[Xiang Mei]

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>
---
v3: 
- Move qdisc_warn_nonwc and qdisc_peek_len to pkt_sched.h

v2:
- Use real name in signed-off-by

v1:
- Move qdisc_warn_nonwc to include/net/sch_generic.h
- Move qdisc_peek_len from net/sched/sch_hfsc.c to include/net/sch_generic.h
- Replace  cl->qdisc->ops->peek() with qdisc_peek_len() in sch_qfq.c

 include/net/pkt_sched.h | 25 ++++++++++++++++++++++++-
 net/sched/sch_api.c     | 10 ----------
 net/sched/sch_hfsc.c    | 16 ----------------
 net/sched/sch_qfq.c     |  2 +-
 4 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index 15960564e0c3..4d72d24b1f33 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -112,7 +112,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);
@@ -306,4 +305,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 df89790c459a..6518fdc63dc2 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -594,16 +594,6 @@ void __qdisc_calculate_pkt_len(struct sk_buff *skb,
 	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 afcb83d469ff..751b1e2c35b3 100644
--- 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 5e557b960bde..e0cefa21ce21 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -992,7 +992,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);
 	}
-- 
2.43.0

Re: [PATCH v2] net/sched: sch_qfq: Fix null-deref in agg_dequeue

[Xiang Mei]

Thanks for your code review. After reviewing the purpose and order of
these header files, I found that your suggestion makes more sense.


On Thu, Jul 3, 2025 at 9:45 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Wed, Jul 02, 2025 at 01:30:01AM -0700, Xiang Mei wrote:
> >  include/net/sch_generic.h | 24 ++++++++++++++++++++++++
> >  net/sched/sch_api.c       | 10 ----------
> >  net/sched/sch_hfsc.c      | 16 ----------------
> >  net/sched/sch_qfq.c       |  2 +-
> >  4 files changed, 25 insertions(+), 27 deletions(-)
>
> Looks like you missed the declaration of qdisc_warn_nonwc()?
>
> $ git grep qdisc_warn_nonwc -- include/
> include/net/pkt_sched.h:void qdisc_warn_nonwc(const char *txt, struct Qdisc *qdisc);
>
> I suggest moving both inlined functions to include/net/pkt_sched.h.
> Sorry for not noticing this earlier.
>
> Thanks!

Re: [PATCH v3] net/sched: sch_qfq: Fix null-deref in agg_dequeue

[Cong Wang]

On Sat, Jul 05, 2025 at 02:21:43PM -0700, Xiang Mei wrote:
> 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>

Fixes: 462dbc9101ac ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost")
Reviewed-by: Cong Wang <xiyou.wangcong@gmail.com>

Thanks!

Re: [PATCH v3] net/sched: sch_qfq: Fix null-deref in agg_dequeue

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Sat,  5 Jul 2025 14:21:43 -0700 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [v3] net/sched: sch_qfq: Fix null-deref in agg_dequeue
    https://git.kernel.org/netdev/net/c/dd831ac8221e

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html