netdev - Re: [Patch net-next] net_sched: initialize timer earlier in red

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <871rkwizq6.fsf@mellanox.com>
Date:   Mon, 27 Jul 2020 20:23:29 +0200
From:   Petr Machata <petrm@...lanox.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     netdev@...r.kernel.org,
        syzbot+6e95a4fabf88dc217145@...kaller.appspotmail.com,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Jiri Pirko <jiri@...nulli.us>
Subject: Re: [Patch net-next] net_sched: initialize timer earlier in red_init()


Cong Wang <xiyou.wangcong@...il.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+6e95a4fabf88dc217145@...kaller.appspotmail.com
> Fixes: aee9caa03fc3 ("net: sched: sch_red: Add qevents "early_drop" and "mark"")
> Cc: Petr Machata <petrm@...lanox.com>
> Cc: Jamal Hadi Salim <jhs@...atatu.com>
> Cc: Jiri Pirko <jiri@...nulli.us>
> Signed-off-by: Cong Wang <xiyou.wangcong@...il.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 <petrm@...lanox.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;