linux-kernel - [PATCH v2 1/3] platform/x86: intel_scu_ipc: Check status after timeout in busy

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: <20230906180944.2197111-2-swboyd@chromium.org>
Date:   Wed,  6 Sep 2023 11:09:41 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Mark Gross <markgross@...nel.org>
Cc:     linux-kernel@...r.kernel.org, patches@...ts.linux.dev,
        platform-driver-x86@...r.kernel.org,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        Prashant Malani <pmalani@...omium.org>
Subject: [PATCH v2 1/3] platform/x86: intel_scu_ipc: Check status after timeout in busy_loop()

It's possible for the polling loop in busy_loop() to get scheduled away
for a long time.

  status = ipc_read_status(scu); // status = IPC_STATUS_BUSY
  <long time scheduled away>
  if (!(status & IPC_STATUS_BUSY))

If this happens, then the status bit could change while the task is
scheduled away and this function would never read the status again after
timing out. Instead, the function will return -ETIMEDOUT when it's
possible that scheduling didn't work out and the status bit was cleared.
Bit polling code should always check the bit being polled one more time
after the timeout in case this happens.

Fix this by reading the status once more after the while loop breaks.

Cc: Prashant Malani <pmalani@...omium.org>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Fixes: e7b7ab3847c9 ("platform/x86: intel_scu_ipc: Sleeping is fine when polling")
Signed-off-by: Stephen Boyd <swboyd@...omium.org>
---

This is sufficiently busy so I didn't add any tags from previous round.

 drivers/platform/x86/intel_scu_ipc.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/intel_scu_ipc.c b/drivers/platform/x86/intel_scu_ipc.c
index 6851d10d6582..b2a2de22b8ff 100644
--- a/drivers/platform/x86/intel_scu_ipc.c
+++ b/drivers/platform/x86/intel_scu_ipc.c
@@ -232,18 +232,21 @@ static inline u32 ipc_data_readl(struct intel_scu_ipc_dev *scu, u32 offset)
 static inline int busy_loop(struct intel_scu_ipc_dev *scu)
 {
 	unsigned long end = jiffies + IPC_TIMEOUT;
+	u32 status;
 
 	do {
-		u32 status;
-
 		status = ipc_read_status(scu);
 		if (!(status & IPC_STATUS_BUSY))
-			return (status & IPC_STATUS_ERR) ? -EIO : 0;
+			goto not_busy;
 
 		usleep_range(50, 100);
 	} while (time_before(jiffies, end));
 
-	return -ETIMEDOUT;
+	status = ipc_read_status(scu);
+	if (status & IPC_STATUS_BUSY)
+		return -ETIMEDOUT;
+not_busy:
+	return (status & IPC_STATUS_ERR) ? -EIO : 0;
 }
 
 /* Wait till ipc ioc interrupt is received or timeout in 10 HZ */
-- 
https://chromeos.dev