linux-kernel - [PATCH] oprofile: Fix uninitialized memory access when writing to oprofilefs

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: <1324302373-30872-2-git-send-email-robert.richter@amd.com>
Date:	Mon, 19 Dec 2011 14:46:13 +0100
From:	Robert Richter <robert.richter@....com>
To:	Ingo Molnar <mingo@...e.hu>
CC:	LKML <linux-kernel@...r.kernel.org>,
	oprofile-list <oprofile-list@...ts.sourceforge.net>,
	Robert Richter <robert.richter@....com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	<stable@...r.kernel.org>
Subject: [PATCH] oprofile: Fix uninitialized memory access when writing to oprofilefs

If oprofilefs_ulong_from_user() is called with count equals zero, *val
must be initialized. Otherwise *val is later used uninitialized as no
error is returned. Alternatively oprofilefs_ulong_from_user() may not
be called if !count. This patch fixes usage of oprofilefs_ulong_from_
user().

This follows write syscall implementation when count is zero: "If
count is zero ... [and if] no errors are detected, 0 will be returned
without causing any other effect." (man 2 write)

Reported-By: Mike Waychison <mikew@...gle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: stable@...r.kernel.org
Signed-off-by: Robert Richter <robert.richter@....com>
---
 arch/s390/oprofile/init.c         |    3 +++
 drivers/oprofile/oprofile_files.c |    9 +++++++++
 drivers/oprofile/oprofilefs.c     |   15 ++++++++++++++-
 3 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/arch/s390/oprofile/init.c b/arch/s390/oprofile/init.c
index 6efc18b..5d605f1 100644
--- a/arch/s390/oprofile/init.c
+++ b/arch/s390/oprofile/init.c
@@ -87,6 +87,9 @@ static ssize_t hwsampler_write(struct file *file, char const __user *buf,
 	if (*offset)
 		return -EINVAL;
 
+	if (!count)
+		return 0;
+
 	retval = oprofilefs_ulong_from_user(&val, buf, count);
 	if (retval)
 		return retval;
diff --git a/drivers/oprofile/oprofile_files.c b/drivers/oprofile/oprofile_files.c
index 89f6345..8265b41 100644
--- a/drivers/oprofile/oprofile_files.c
+++ b/drivers/oprofile/oprofile_files.c
@@ -44,6 +44,9 @@ static ssize_t timeout_write(struct file *file, char const __user *buf,
 	if (*offset)
 		return -EINVAL;
 
+	if (!count)
+		return 0;
+
 	retval = oprofilefs_ulong_from_user(&val, buf, count);
 	if (retval)
 		return retval;
@@ -83,6 +86,9 @@ static ssize_t depth_write(struct file *file, char const __user *buf, size_t cou
 	if (!oprofile_ops.backtrace)
 		return -EINVAL;
 
+	if (!count)
+		return 0;
+
 	retval = oprofilefs_ulong_from_user(&val, buf, count);
 	if (retval)
 		return retval;
@@ -140,6 +146,9 @@ static ssize_t enable_write(struct file *file, char const __user *buf, size_t co
 	if (*offset)
 		return -EINVAL;
 
+	if (!count)
+		return 0;
+
 	retval = oprofilefs_ulong_from_user(&val, buf, count);
 	if (retval)
 		return retval;
diff --git a/drivers/oprofile/oprofilefs.c b/drivers/oprofile/oprofilefs.c
index d0de6cc..1caf1b0 100644
--- a/drivers/oprofile/oprofilefs.c
+++ b/drivers/oprofile/oprofilefs.c
@@ -59,7 +59,17 @@ ssize_t oprofilefs_ulong_to_user(unsigned long val, char __user *buf, size_t cou
 	return simple_read_from_buffer(buf, count, offset, tmpbuf, maxlen);
 }
 
-
+/*
+ * Note: oprofilefs_ulong_from_user() must be called with *val
+ * initialized, otherwise *val is used uninitialized if !count. This
+ * follows write syscall implementation when count is zero: "If count
+ * is zero ... [and if] no errors are detected, 0 will be returned
+ * without causing any other effect." (man 2 write)
+ *
+ * In case *val is a temporary variable, oprofilefs_ulong_from_user()
+ * may not be called if !count. This causes race conditions due to
+ * missing locking of *var.
+ */
 int oprofilefs_ulong_from_user(unsigned long *val, char const __user *buf, size_t count)
 {
 	char tmpbuf[TMPBUFSIZE];
@@ -98,6 +108,9 @@ static ssize_t ulong_write_file(struct file *file, char const __user *buf, size_
 	if (*offset)
 		return -EINVAL;
 
+	if (!count)
+		return 0;
+
 	retval = oprofilefs_ulong_from_user(&value, buf, count);
 	if (retval)
 		return retval;
-- 
1.7.7


--
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/