Cong Wang <xiyou.wangc...@gmail.com> writes:
> When red_init() fails, red_destroy() is called to clean up.
> If the timer is not initialized yet, del_timer_sync() will
> complain. So we have to move timer_setup() before any failure.
>
> Reported-and-tested-by: syzbot+6e95a4fabf88dc217...@syzkaller.appspotmail.com
> Fixes: aee9caa03fc3 ("net: sched: sch_red: Add qevents "early_drop" and
> "mark"")
> Cc: Petr Machata <pe...@mellanox.com>
> Cc: Jamal Hadi Salim <j...@mojatatu.com>
> Cc: Jiri Pirko <j...@resnulli.us>
> Signed-off-by: Cong Wang <xiyou.wangc...@gmail.com>
Ah, correct, this used to be done in red_init() not only before calling
red_change(), but actually before doing anything that can fail. Thanks
for fixing this.
Reviewed-by: Petr Machata <pe...@mellanox.com>
> ---
> net/sched/sch_red.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
> index 4cc0ad0b1189..deac82f3ad7b 100644
> --- a/net/sched/sch_red.c
> +++ b/net/sched/sch_red.c
> @@ -333,6 +333,10 @@ static int red_init(struct Qdisc *sch, struct nlattr
> *opt,
> struct nlattr *tb[TCA_RED_MAX + 1];
> int err;
>
> + q->qdisc = &noop_qdisc;
> + q->sch = sch;
> + timer_setup(&q->adapt_timer, red_adaptative_timer, 0);
> +
> if (!opt)
> return -EINVAL;
>
> @@ -341,10 +345,6 @@ static int red_init(struct Qdisc *sch, struct nlattr
> *opt,
> if (err < 0)
> return err;
>
> - q->qdisc = &noop_qdisc;
> - q->sch = sch;
> - timer_setup(&q->adapt_timer, red_adaptative_timer, 0);
> -
> err = __red_change(sch, tb, extack);
> if (err)
> return err;
