linux-kernel - [PATCH 3/4] iopoll: Reorder the timeout handling in poll_timeout

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: <20250702223439.19752-3-ville.syrjala@linux.intel.com>
Date: Thu,  3 Jul 2025 01:34:38 +0300
From: Ville Syrjala <ville.syrjala@...ux.intel.com>
To: linux-kernel@...r.kernel.org
Cc: Jani Nikula <jani.nikula@...el.com>,
	Lucas De Marchi <lucas.demarchi@...el.com>,
	Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@...el.com>,
	Imre Deak <imre.deak@...el.com>,
	David Laight <david.laight.linux@...il.com>,
	Geert Uytterhoeven <geert+renesas@...der.be>,
	Matt Wagantall <mattw@...eaurora.org>,
	Dejin Zheng <zhengdejin5@...il.com>,
	intel-gfx@...ts.freedesktop.org,
	intel-xe@...ts.freedesktop.org,
	Ville Syrjälä <ville.syrjala@...ux.intel.com>
Subject: [PATCH 3/4] iopoll: Reorder the timeout handling in poll_timeout_us()

From: Ville Syrjälä <ville.syrjala@...ux.intel.com>

Currently poll_timeout_us() evaluates 'op' and 'cond' twice
within the loop, once at the start, and a second time after
the timeout check. While it's probably not a big deal to do
it twice almost back to back, it does make the macro a bit messy.

Simplify the implementation to evaluate the timeout at the
very start, then follow up with 'op'/'cond', and finally
check if the timeout did in fact happen or not.

For good measure throw in a compiler barrier between the timeout
and 'op'/'cond' evaluations to make sure the compiler can't reoder
the operations (which could cause false positive timeouts).
The similar i915 __wait_for() macro already has the barrier, though
there it is between the 'op' and 'cond' evaluations, which seems
like it could still allow 'op' and the timeout evaluations to get
reordered incorrectly. I suppose the ktime_get() might itself act
as a sufficient barrier here, but better safe than sorry I guess.

Cc: Jani Nikula <jani.nikula@...el.com>
Cc: Lucas De Marchi <lucas.demarchi@...el.com>
Cc: Dibin Moolakadan Subrahmanian <dibin.moolakadan.subrahmanian@...el.com>
Cc: Imre Deak <imre.deak@...el.com>
Cc: David Laight <david.laight.linux@...il.com>
Cc: Geert Uytterhoeven <geert+renesas@...der.be>
Cc: Matt Wagantall <mattw@...eaurora.org>
Cc: Dejin Zheng <zhengdejin5@...il.com>
Cc: intel-gfx@...ts.freedesktop.org
Cc: intel-xe@...ts.freedesktop.org
Cc: linux-kernel@...r.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@...ux.intel.com>
---
 include/linux/iopoll.h | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
index 69296e6adbf3..0e0940a60fdb 100644
--- a/include/linux/iopoll.h
+++ b/include/linux/iopoll.h
@@ -41,18 +41,17 @@
 	if ((sleep_before_op) && __sleep_us) \
 		usleep_range((__sleep_us >> 2) + 1, __sleep_us); \
 	for (;;) { \
+		bool __expired = __timeout_us && \
+			ktime_compare(ktime_get(), __timeout) > 0; \
+		/* guarantee 'op' and 'cond' are evaluated after timeout expired */ \
+		barrier(); \
 		op; \
 		if (cond) { \
 			___ret = 0; \
 			break; \
 		} \
-		if (__timeout_us && \
-		    ktime_compare(ktime_get(), __timeout) > 0) { \
-			op; \
-			if (cond) \
-				___ret = 0; \
-			else \
-				___ret = -ETIMEDOUT; \
+		if (__expired) { \
+			___ret = -ETIMEDOUT; \
 			break; \
 		} \
 		if (__sleep_us) \
@@ -97,17 +96,16 @@
 			__left_ns -= __delay_ns; \
 	} \
 	for (;;) { \
+		bool __expired = __timeout_us && __left_ns < 0; \
+		/* guarantee 'op' and 'cond' are evaluated after timeout expired */ \
+		barrier(); \
 		op; \
 		if (cond) { \
 			___ret = 0; \
 			break; \
 		} \
-		if (__timeout_us && __left_ns < 0) { \
-			op; \
-			if (cond) \
-				___ret = 0; \
-			else \
-				___ret = -ETIMEDOUT; \
+		if (__expired) { \
+			___ret = -ETIMEDOUT; \
 			break; \
 		} \
 		if (__delay_us) { \
-- 
2.49.0