netdev - Re: [Patch net 1/2] net_sched: hfsc: Fix a UAF vulnerability in class handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALYGNiOV2sJY5gQwMX+U6ot9fFURHLWW+F87pBtH3T-RLDL+5Q@mail.gmail.com>
Date: Mon, 14 Apr 2025 11:39:49 +0200
From: Konstantin Khlebnikov <koct9i@...il.com>
To: Cong Wang <xiyou.wangcong@...il.com>
Cc: netdev@...r.kernel.org, jhs@...atatu.com, jiri@...nulli.us, 
	gerrard.tai@...rlabs.sg
Subject: Re: [Patch net 1/2] net_sched: hfsc: Fix a UAF vulnerability in class handling

On Mon, 14 Apr 2025 at 03:09, Cong Wang <xiyou.wangcong@...il.com> wrote:
>
> This patch fixes a Use-After-Free vulnerability in the HFSC qdisc class
> handling. The issue occurs due to a time-of-check/time-of-use condition
> in hfsc_change_class() when working with certain child qdiscs like netem
> or codel.
>
> The vulnerability works as follows:
> 1. hfsc_change_class() checks if a class has packets (q.qlen != 0)
> 2. It then calls qdisc_peek_len(), which for certain qdiscs (e.g.,
>    codel, netem) might drop packets and empty the queue
> 3. The code continues assuming the queue is still non-empty, adding
>    the class to vttree
> 4. This breaks HFSC scheduler assumptions that only non-empty classes
>    are in vttree
> 5. Later, when the class is destroyed, this can lead to a Use-After-Free
>
> The fix adds a second queue length check after qdisc_peek_len() to verify
> the queue wasn't emptied.
>
> Fixes: 21f4d5cc25ec ("net_sched/hfsc: fix curve activation in hfsc_change_class()")
> Reported-by: Gerrard Tai <gerrard.tai@...rlabs.sg>
> Cc: Konstantin Khlebnikov <koct9i@...il.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@...il.com>
> ---
>  net/sched/sch_hfsc.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
> index ce5045eea065..b368ac0595d5 100644
> --- a/net/sched/sch_hfsc.c
> +++ b/net/sched/sch_hfsc.c
> @@ -961,6 +961,7 @@ hfsc_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
>
>         if (cl != NULL) {
>                 int old_flags;
> +               int len = 0;
>
>                 if (parentid) {
>                         if (cl->cl_parent &&
> @@ -991,9 +992,13 @@ hfsc_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
>                 if (usc != NULL)
>                         hfsc_change_usc(cl, usc, cur_time);
>
> +               if (cl->qdisc->q.qlen != 0)
> +                       len = qdisc_peek_len(cl->qdisc);
> +               /* Check queue length again since some qdisc implementations
> +                * (e.g., netem/codel) might empty the queue during the peek
> +                * operation.
> +                */
>                 if (cl->qdisc->q.qlen != 0) {
> -                       int len = qdisc_peek_len(cl->qdisc);
> -

I don't see any functional changes in the code.

>                         if (cl->cl_flags & HFSC_RSC) {
>                                 if (old_flags & HFSC_RSC)
>                                         update_ed(cl, len);
> --
> 2.34.1
>