Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Johannes Berg <johannes@sipsolutions.net>,
        linux-wireless@vger.kernel.org
Cc:     Johannes Berg <johannes.berg@intel.com>,
        Dmitry Vyukov <dvyukov@google.com>,
        syzbot+0e964fad69a9c462bc1e@syzkaller.appspotmail.com
Subject: Re: [PATCH v2] mac80211-hwsim: fix late beacon hrtimer handling
In-Reply-To: <20210915112936.544f383472eb.I3f9712009027aa09244b65399bf18bf482a8c4f1@changeid>
References: <20210915112936.544f383472eb.I3f9712009027aa09244b65399bf18bf482a8c4f1@changeid>
Date:   Wed, 15 Sep 2021 11:34:12 +0200
Message-ID: <87h7emb2sr.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Wed, Sep 15 2021 at 11:29, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Thomas explained in https://lore.kernel.org/r/87mtoeb4hb.ffs@tglx
> that our handling of the hrtimer here is wrong: If the timer fires
> late (e.g. due to vCPU scheduling, as reported by Dmitry/syzbot)
> then it tries to actually rearm the timer at the next deadline,
> which might be in the past already:
>
>  1          2          3          N          N+1
>  |          |          |   ...    |          |
>
>  ^ intended to fire here (1)
>             ^ next deadline here (2)
>                                       ^ actually fired here
>
> The next time it fires, it's later, but will still try to schedule
> for the next deadline (now 3), etc. until it catches up with N,
> but that might take a long time, causing stalls etc.
>
> Now, all of this is simulation, so we just have to fix it, but
> note that the behaviour is wrong even per spec, since there's no
> value then in sending all those beacons unaligned - they should be
> aligned to the TBTT (1, 2, 3, ... in the picture), and if we're a
> bit (or a lot) late, then just resume at that point.

Well done changelog!

> Therefore, change the code to use hrtimer_forward_now() which will
> ensure that the next firing of the timer would be at N+1 (in the
> picture), i.e. the next interval point after the current time.
>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Reported-by: syzbot+0e964fad69a9c462bc1e@syzkaller.appspotmail.com
> Fixes: 01e59e467ecf ("mac80211_hwsim: hrtimer beacon")
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>