netdev - Re: INFO: rcu detected stall in br_handle

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20191230230216.GK795@breakpoint.cc>
Date:   Tue, 31 Dec 2019 00:02:16 +0100
From:   Florian Westphal <fw@...len.de>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     Florian Westphal <fw@...len.de>,
        syzbot <syzbot+dc9071cc5a85950bdfce@...kaller.appspotmail.com>,
        davem@...emloft.net, jhs@...atatu.com, jiri@...nulli.us,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        syzkaller-bugs@...glegroups.com, xiyou.wangcong@...il.com
Subject: Re: INFO: rcu detected stall in br_handle_frame (2)

Eric Dumazet <eric.dumazet@...il.com> wrote:
> On 12/28/19 3:15 AM, Florian Westphal wrote:
> > If you don't have a better idea/suggestion for an upperlimit INT_MAX
> > would be enough to prevent perpetual <= 0 condition.
> 
> Thanks Florian for the analysis.
> 
> I guess we could use a conservative upper bound value of (1 << 20)
> ( about 16 64KB packets )
>
> diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
> index ff4c5e9d0d7778d86f20f4bd67cc627eed0713d9..12f1d1c6044fac9db987f7ce3a50a7e2c711358b 100644
> --- a/net/sched/sch_fq.c
> +++ b/net/sched/sch_fq.c
> @@ -786,15 +786,20 @@ static int fq_change(struct Qdisc *sch, struct nlattr *opt,
>         if (tb[TCA_FQ_QUANTUM]) {
>                 u32 quantum = nla_get_u32(tb[TCA_FQ_QUANTUM]);
>  
> -               if (quantum > 0)
> +               if (quantum > 0 && quantum <= (1 << 20))
>                         q->quantum = quantum;
>                 else
>                         err = -EINVAL;
>         }
>  
> -       if (tb[TCA_FQ_INITIAL_QUANTUM])
> -               q->initial_quantum = nla_get_u32(tb[TCA_FQ_INITIAL_QUANTUM]);
> +       if (tb[TCA_FQ_INITIAL_QUANTUM]) {
> +               u32 quantum = nla_get_u32(tb[TCA_FQ_INITIAL_QUANTUM]);
>  
> +               if (quantum > 0 && quantum <= (1 << 20))
> +                       q->initial_quantum = quantum;
> +               else
> +                       err = -EINVAL;
> +       }
>         if (tb[TCA_FQ_FLOW_DEFAULT_RATE])
>                 pr_warn_ratelimited("sch_fq: defrate %u ignored.\n",
>                                     nla_get_u32(tb[TCA_FQ_FLOW_DEFAULT_RATE]));
> 

Perhaps it would make sense to add an #ifdef for the 1 << 20 and
a small comment as to what this is / where this comes from.

But other than that nit, this looks good to me, thanks Eric!