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

[Robert Richter]

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@xxxxxxxxxx>
Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Robert Richter <robert.richter@xxxxxxx>
---
 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/