linux-kernel - [PATCH] x86_64: Fix misplaced `continue' in mce.c

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-next>] [day] [month] [year] [list]

Date:	Thu, 21 Jun 2007 17:13:49 -0700 (PDT)
From:	Joshua Wise <jwise@...gle.com>
To:	linux-kernel@...r.kernel.org
cc:	ak@...e.de, thockin@...gle.com, akpm@...gle.com
Subject: [PATCH] x86_64: Fix misplaced `continue' in mce.c

From: Joshua Wise <jwise@...gle.com>

Background:
  When a userspace application wants to know about machine check events, it
  opens /dev/mcelog and does a read(). Usually, we found that this interface
  works well, but in some cases, when the system was taking large numbers of
  machine check exceptions, the read() would hang. The system would output a
  soft-lockup warning, and the daemon reading from /dev/mcelog would suck up
  as much of a single CPU as it could spinning in system space.

Description:
  This patch fixes this bug. In particular, there was a "continue" inside a
  timeout loop that presumably was intended to break out of the outer loop,
  but instead caused the inner loop to continue. This patch also makes the
  condition for the break-out a little more evident by changing a
  !time_before to a time_after_eq.

Result:
  The read() no longer hangs in this test case.

Testing:
  On my system, I could replicate the bug with the following command:
    # for i in `seq 15000`; do ./inject_sbe.sh; done
  where inject_sbe.sh contains commands to inject a single-bit error into the
  next memory write transaction.

Patch:
  This patch is against git f1518a088bde6aea49e7c472ed6ab96178fcba3e.

Signed-off-by: Joshua Wise <jwise@...gle.com>
Signed-off-by: Tim Hockin <thockin@...gle.com>

--

diff --git a/arch/x86_64/kernel/mce.c b/arch/x86_64/kernel/mce.c
index a14375d..aa83023 100644
--- a/arch/x86_64/kernel/mce.c
+++ b/arch/x86_64/kernel/mce.c
@@ -497,15 +497,17 @@ static ssize_t mce_read(struct file *fil
  	for (i = 0; i < next; i++) {
  		unsigned long start = jiffies;
  		while (!mcelog.entry[i].finished) {
-			if (!time_before(jiffies, start + 2)) {
+			if (time_after_eq(jiffies, start + 2)) {
  				memset(mcelog.entry + i,0, sizeof(struct mce));
-				continue;
+				goto timeout;
  			}
  			cpu_relax();
  		}
  		smp_rmb();
  		err |= copy_to_user(buf, mcelog.entry + i, sizeof(struct mce));
  		buf += sizeof(struct mce); 
+	 timeout:
+	 	;
  	}

  	memset(mcelog.entry, 0, next * sizeof(struct mce));
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/