linux-kernel - Re: [PATCH] watchdog: revert cleanup handling of false positives

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: <YKPfDQoN5hToB9nk@alley>
Date:   Tue, 18 May 2021 17:36:45 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Sergey Senozhatsky <senozhatsky@...omium.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] watchdog: revert cleanup handling of false positives

On Mon 2021-05-17 23:06:12, Sergey Senozhatsky wrote:
> This reverts commit 9bf3bc949f8aeefeacea4b1198db833b722a8e27.
> 
> I can reproduce the case when resumed VCPU starts to execute
> is_softlockup() with PVCLOCK_GUEST_STOPPED set on this VCPU:
> 
> 	watchdog_timer_fn()
> 	{
> 		...
> 
> 		kvm_check_and_clear_guest_paused();
> 
> 		...
> 
> 		duration = is_softlockup(touch_ts, period_ts);
> 		if (unlikely(duration)) {
> 			....
> 		}
> 	}
> 
> Which means that guest VCPU has been suspended between
> kvm_check_and_clear_guest_paused() and is_softlockup(),
> and jiffies (clock) thus shifted forward.

Are jiffies really updated here?
watchdog_timer_fn() should be called with interrupts disabled.

kvm_check_and_clear_guest_paused() calls
touch_softlockup_watchdog_sync(). It sets softlockup_touch_sync
when jiffies have to be updated explicitely.

Well, I am not 100% sure.

Anyway, the code does not guarantee in which order and how
many times are touch_ts and current jiffies read. And touch_ts
might be updated also from NMI.

I have a patch that mfixes the ordering and makes sure that
the same value is used in all checks. But I still need to double
check some things and write proper commit message.

I would prefer to fix it properly. The original code was
not good either.

Best Regards,
Petr