linux-kernel - [PATCH] APEI / ERST: Fix missing error handling in erst

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]

Message-Id: <20171214123116.929-1-tiwai@suse.de>
Date:   Thu, 14 Dec 2017 13:31:16 +0100
From:   Takashi Iwai <tiwai@...e.de>
To:     Kees Cook <keescook@...omium.org>
Cc:     Anton Vorontsov <anton@...msg.org>,
        Colin Cross <ccross@...roid.com>,
        Tony Luck <tony.luck@...el.com>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Jerry Tang <jtang@...e.com>, linux-acpi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: [PATCH] APEI / ERST: Fix missing error handling in erst_reader()

The commit f6f828513290 ("pstore: pass allocated memory region back to
caller") changed the check of the return value from erst_read() in
erst_reader() in the following way:

        if (len == -ENOENT)
                goto skip;
-       else if (len < 0) {
-               rc = -1;
+       else if (len < sizeof(*rcd)) {
+               rc = -EIO;
                goto out;

This introduced another bug: since the comparison with sizeof() is
cast to unsigned, a negative len value doesn't hit any longer.
As a result, when an error is returned from erst_read(), the code
falls through, and it may eventually lead to some weird thing like
memory corruption.

This patch adds the negative error value check more explicitly for
addressing the issue.

Fixes: f6f828513290 ("pstore: pass allocated memory region back to caller")
Cc: <stable@...r.kernel.org>
Tested-by: Jerry Tang <jtang@...e.com>
Signed-off-by: Takashi Iwai <tiwai@...e.de>
---
 drivers/acpi/apei/erst.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c
index 6742f6c68034..9bff853e85f3 100644
--- a/drivers/acpi/apei/erst.c
+++ b/drivers/acpi/apei/erst.c
@@ -1007,7 +1007,7 @@ static ssize_t erst_reader(struct pstore_record *record)
 	/* The record may be cleared by others, try read next record */
 	if (len == -ENOENT)
 		goto skip;
-	else if (len < sizeof(*rcd)) {
+	else if (len < 0 || len < sizeof(*rcd)) {
 		rc = -EIO;
 		goto out;
 	}
-- 
2.15.1