linux-kernel - Re: [PATCH] watchdog: Reliable handling of timestamps

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]

Date:   Thu, 20 May 2021 14:29:31 +0900
From:   Sergey Senozhatsky <senozhatsky@...omium.org>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Sergey Senozhatsky <senozhatsky@...omium.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] watchdog: Reliable handling of timestamps

On (21/05/19 13:43), Petr Mladek wrote:
> 
> The commit 9bf3bc949f8aeefeacea4b ("watchdog: cleanup handling of false
> positives") tried to handle a virtual host stopped by the host a more
> straightforward and cleaner way.
> 
> But it introduced a risk of false softlockup reports. The virtual host
> might be stopped at any time, for example between
> kvm_check_and_clear_guest_paused() and is_softlockup().
> As a result, is_softlockup() might read the updated jiffies
> are detects softlockup.
> 
> A solution might be to put back kvm_check_and_clear_guest_paused()
> after is_softlockup() and detect it. But it would put back
> the cycle that complicates the logic.
> 
> In fact, the handling of all the timestamps is not reliable.
> The code does not guarantee when and how many times the timestamps
> are read. For example, "period_ts" might be touched anytime also
> from NMI and re-read in is_softlockup(). It works just by chance.
> 
> Fix all the problems by making the code even more explicit.
> 
> 1. Make sure that "now" and "period_ts" timestamps are read only
>    once. They might be changed at anytime by NMI or when the virtual
>    guest is stopped by the host. Note that "now" timestamp does
>    this implicitly because "jiffies" is marked volatile.
> 
> 2. "now" time must be read first. The state of "period_ts" will decide
>    whether it will be used or the period will get restarted.
> 
> 3. kvm_check_and_clear_guest_paused() must be called before reading
>    "period_ts". It touches the variable when the guest was
>    stopped.
> 
> As a result, "now" timestamp is used only when the watchdog was
> not touched and the guest not stopped in the meantime. "period_ts"
> is restarted in all other situations.
> 
> Fixes: 9bf3bc949f8aeefeacea4b ("watchdog: cleanup handling of false positives")
> Reported-by: Sergey Senozhatsky <senozhatsky@...omium.org>
> Signed-off-by: Petr Mladek <pmladek@...e.com>

Reviewed-by: Sergey Senozhatsky <senozhatsky@...omium.org>