linux-kernel - [tip:x86/urgent] nvram: Fix write beyond end condition; prove to gcc copy is safe

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

Date:	Sat, 12 Dec 2009 00:16:24 GMT
From:	"tip-bot for H. Peter Anvin" <hpa@...or.com>
To:	linux-tip-commits@...r.kernel.org
Cc:	linux-kernel@...r.kernel.org, hpa@...or.com, mingo@...hat.com,
	wim@...ana.be, arjan@...radead.org, fweisbec@...il.com,
	akpm@...ux-foundation.org, tglx@...utronix.de
Subject: [tip:x86/urgent] nvram: Fix write beyond end condition; prove to gcc copy is safe

Commit-ID:  a01c7800420d2c294ca403988488a635d4087a6d
Gitweb:     http://git.kernel.org/tip/a01c7800420d2c294ca403988488a635d4087a6d
Author:     H. Peter Anvin <hpa@...or.com>
AuthorDate: Fri, 11 Dec 2009 15:48:23 -0800
Committer:  H. Peter Anvin <hpa@...or.com>
CommitDate: Fri, 11 Dec 2009 15:48:23 -0800

nvram: Fix write beyond end condition; prove to gcc copy is safe

In nvram_write, first of all, correctly handle the case where the file
pointer is already beyond the end; we should return EOF in that case.

Second, make the logic a bit more explicit so that gcc can statically
prove that the copy_from_user() is safe.  Once the condition of the
beyond-end filepointer is eliminated, the copy is safe but gcc can't
prove it, causing build failures for i386 allyesconfig.

Third, eliminate the entirely superfluous variable "len", and just use
the passed-in variable "count" instead.

Signed-off-by: H. Peter Anvin <hpa@...or.com>
Cc: Arjan van de Ven <arjan@...radead.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Wim Van Sebroeck <wim@...ana.be>
Cc: Frederic Weisbecker <fweisbec@...il.com>
LKML-Reference: <tip-*@....kernel.org>
---
 drivers/char/nvram.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/char/nvram.c b/drivers/char/nvram.c
index 4008e2c..fdbcc9f 100644
--- a/drivers/char/nvram.c
+++ b/drivers/char/nvram.c
@@ -264,10 +264,16 @@ static ssize_t nvram_write(struct file *file, const char __user *buf,
 	unsigned char contents[NVRAM_BYTES];
 	unsigned i = *ppos;
 	unsigned char *tmp;
-	int len;
 
-	len = (NVRAM_BYTES - i) < count ? (NVRAM_BYTES - i) : count;
-	if (copy_from_user(contents, buf, len))
+	if (i >= NVRAM_BYTES)
+		return 0;	/* Past EOF */
+
+	if (count > NVRAM_BYTES - i)
+		count = NVRAM_BYTES - i;
+	if (count > NVRAM_BYTES)
+		return -EFAULT;	/* Can't happen, but prove it to gcc */
+
+	if (copy_from_user(contents, buf, count))
 		return -EFAULT;
 
 	spin_lock_irq(&rtc_lock);
@@ -275,7 +281,7 @@ static ssize_t nvram_write(struct file *file, const char __user *buf,
 	if (!__nvram_check_checksum())
 		goto checksum_err;
 
-	for (tmp = contents; count-- > 0 && i < NVRAM_BYTES; ++i, ++tmp)
+	for (tmp = contents; count--; ++i, ++tmp)
 		__nvram_write_byte(*tmp, i);
 
 	__nvram_set_checksum();
--
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/