All of lore.kernel.org
 help / color / mirror / Atom feed
From: Robert Richter <robert.richter@amd.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: LKML <linux-kernel@vger.kernel.org>,
	oprofile-list <oprofile-list@lists.sourceforge.net>,
	Robert Richter <robert.richter@amd.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	<stable@vger.kernel.org>
Subject: [PATCH] oprofile: Fix uninitialized memory access when writing to oprofilefs
Date: Mon, 19 Dec 2011 14:46:13 +0100	[thread overview]
Message-ID: <1324302373-30872-2-git-send-email-robert.richter@amd.com> (raw)
In-Reply-To: <1324302373-30872-1-git-send-email-robert.richter@amd.com>

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@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: stable@vger.kernel.org
Signed-off-by: Robert Richter <robert.richter@amd.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



  reply	other threads:[~2011-12-19 13:46 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-12-19 13:46 [GIT PULL] oprofile fixes for 3.2 Robert Richter
2011-12-19 13:46 ` Robert Richter [this message]
2011-12-19 14:00 ` Ingo Molnar
2011-12-19 14:17   ` Robert Richter
2011-12-19 15:38     ` [PATCH v2] oprofile: Fix uninitialized memory access when writing to Robert Richter
2011-12-19 16:19       ` Ingo Molnar
2011-12-20  8:57       ` [tip:perf/urgent] oprofile: Fix uninitialized memory access when writing to writing to oprofilefs tip-bot for Robert Richter
2011-12-22 15:15 ` [PATCH] oprofile, arm/sh: Fix oprofile_arch_exit() linkage issue Robert Richter
2011-12-23 13:19   ` [tip:perf/urgent] " tip-bot for Vladimir Zapolskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1324302373-30872-2-git-send-email-robert.richter@amd.com \
    --to=robert.richter@amd.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=oprofile-list@lists.sourceforge.net \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.