Skip to contentSkip to conversation

tracing: Fix possible NULL pointer dereferences

PATCH Applied Li Zefan 2 participants 6 messages Jun 7, 2013 – Jun 13, 2013

Subsystems (3)
lore
Share & tools
lore

Li Zefan

author
#0 lore raw

PATCH 1/2 tracing: Fix possible NULL pointer dereferences

commit 6a76f8c0ab19f215af2a3442870eeb5f0e81998d upstream.

Currently set_ftrace_pid and set_graph_function files use seq_lseek for their fops. However seq_open() is called only for FMODE_READ in the fops->open() so that if an user tries to seek one of those file when she open it for writing, it sees NULL seq_file and then panic.

It can be easily reproduced with following command:

  $ cd /sys/kernel/debug/tracing
  $ echo 1234 | sudo tee -a set_ftrace_pid

In this example, GNU coreutils' tee opens the file with fopen(, "a") and then the fopen() internally calls lseek().

Link:
http://lkml.kernel.org/r/[email protected]

Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
[ lizf: adjust context ]
---
 kernel/trace/ftrace.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
kernel/trace/ftrace.c +5 -5
index 8e4361f..b17a3f6 100644
@@ -2300,7 +2300,7 @@ ftrace_notrace_open(struct inode *inode, struct file *file)
 }
 
 static loff_t
-ftrace_regex_lseek(struct file *file, loff_t offset, int origin)
+ftrace_filter_lseek(struct file *file, loff_t offset, int origin)
 {
 	loff_t ret;
 
@@ -3118,7 +3118,7 @@ static const struct file_operations ftrace_filter_fops = {
 	.open = ftrace_filter_open,
 	.read = seq_read,
 	.write = ftrace_filter_write,
-	.llseek = ftrace_regex_lseek,
+	.llseek = ftrace_filter_lseek,
 	.release = ftrace_regex_release,
 };
 
@@ -3126,7 +3126,7 @@ static const struct file_operations ftrace_notrace_fops = {
 	.open = ftrace_notrace_open,
 	.read = seq_read,
 	.write = ftrace_notrace_write,
-	.llseek = ftrace_regex_lseek,
+	.llseek = ftrace_filter_lseek,
 	.release = ftrace_regex_release,
 };
 
@@ -3335,8 +3335,8 @@ static const struct file_operations ftrace_graph_fops = {
 	.open		= ftrace_graph_open,
 	.read		= seq_read,
 	.write		= ftrace_graph_write,
+	.llseek		= ftrace_filter_lseek,
 	.release	= ftrace_graph_release,
-	.llseek		= seq_lseek,
 };
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
 
@@ -3822,7 +3822,7 @@ static const struct file_operations ftrace_pid_fops = {
 	.open		= ftrace_pid_open,
 	.write		= ftrace_pid_write,
 	.read		= seq_read,
-	.llseek		= seq_lseek,
+	.llseek		= ftrace_filter_lseek,
 	.release	= ftrace_pid_release,
 };
 
Signature · 1 line
1.8.0.2
author
#1 lore raw

PATCH 2/2 ftrace: Move ftrace_filter_lseek out of CONFIG_DYNAMIC_FTRACE section

commit 7f49ef69db6bbf756c0abca7e9b65b32e999eec8 upstream.

As ftrace_filter_lseek is now used with ftrace_pid_fops, it needs to be moved out of the #ifdef CONFIG_DYNAMIC_FTRACE section as the ftrace_pid_fops is defined when DYNAMIC_FTRACE is not.

Cc Namhyung Kim <[email protected]>
[ lizf: adjust context ]
---
 kernel/trace/ftrace.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)
kernel/trace/ftrace.c +13 -13
index b17a3f6..0d704b0 100644
@@ -933,6 +933,19 @@ static __init void ftrace_profile_debugfs(struct dentry *d_tracer)
 
 static struct pid * const ftrace_swapper_pid = &init_struct_pid;
 
+static loff_t
+ftrace_filter_lseek(struct file *file, loff_t offset, int whence)
+{
+	loff_t ret;
+
+	if (file->f_mode & FMODE_READ)
+		ret = seq_lseek(file, offset, whence);
+	else
+		file->f_pos = ret = 1;
+
+	return ret;
+}
+
 #ifdef CONFIG_DYNAMIC_FTRACE
 
 #ifndef CONFIG_FTRACE_MCOUNT_RECORD
@@ -2299,19 +2312,6 @@ ftrace_notrace_open(struct inode *inode, struct file *file)
 				 inode, file);
 }
 
-static loff_t
-ftrace_filter_lseek(struct file *file, loff_t offset, int origin)
-{
-	loff_t ret;
-
-	if (file->f_mode & FMODE_READ)
-		ret = seq_lseek(file, offset, origin);
-	else
-		file->f_pos = ret = 1;
-
-	return ret;
-}
-
 static int ftrace_match(char *str, char *regex, int len, int type)
 {
 	int matched = 0;
Signature · 1 line
1.8.0.2
maintainer Applied
#2 lore raw

Patch "ftrace: Move ftrace_filter_lseek out of CONFIG_DYNAMIC_FTRACE section" has been added to the 3.0-stable tree

This is a note to let you know that I've just added the patch titled

ftrace: Move ftrace_filter_lseek out of CONFIG_DYNAMIC_FTRACE section

to the 3.0-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
ftrace-move-ftrace_filter_lseek-out-of-config_dynamic_ftrace-section.patch
and it can be found in the queue-3.0 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From [email protected] Mon Jun 10 14:16:48 2013

From: Li Zefan <[email protected]>
Date: Fri, 7 Jun 2013 17:01:04 +0800
Subject: ftrace: Move ftrace_filter_lseek out of CONFIG_DYNAMIC_FTRACE section
To: Greg Kroah-Hartman <[email protected]>
Cc: Namhyung Kim <[email protected]>, Frederic Weisbecker <[email protected]>, Steven Rostedt <[email protected]>, Ingo Molnar <[email protected]>, LKML <[email protected]>, stable <[email protected]>
Message-ID: <[email protected]>

From: Steven Rostedt <[email protected]>

commit 7f49ef69db6bbf756c0abca7e9b65b32e999eec8 upstream.

As ftrace_filter_lseek is now used with ftrace_pid_fops, it needs to be moved out of the #ifdef CONFIG_DYNAMIC_FTRACE section as the ftrace_pid_fops is defined when DYNAMIC_FTRACE is not.

Cc Namhyung Kim <[email protected]>
[ lizf: adjust context ]
---
 kernel/trace/ftrace.c |   26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)
kernel/trace/ftrace.c +13 -13
@@ -933,6 +933,19 @@ static __init void ftrace_profile_debugf
 
 static struct pid * const ftrace_swapper_pid = &init_struct_pid;
 
+static loff_t
+ftrace_filter_lseek(struct file *file, loff_t offset, int whence)
+{
+	loff_t ret;
+
+	if (file->f_mode & FMODE_READ)
+		ret = seq_lseek(file, offset, whence);
+	else
+		file->f_pos = ret = 1;
+
+	return ret;
+}
+
 #ifdef CONFIG_DYNAMIC_FTRACE
 
 #ifndef CONFIG_FTRACE_MCOUNT_RECORD
@@ -2299,19 +2312,6 @@ ftrace_notrace_open(struct inode *inode,
 				 inode, file);
 }
 
-static loff_t
-ftrace_filter_lseek(struct file *file, loff_t offset, int origin)
-{
-	loff_t ret;
-
-	if (file->f_mode & FMODE_READ)
-		ret = seq_lseek(file, offset, origin);
-	else
-		file->f_pos = ret = 1;
-
-	return ret;
-}
-
 static int ftrace_match(char *str, char *regex, int len, int type)
 {
 	int matched = 0;

Patches currently in stable-queue which might be from [email protected] are

queue-3.0/tracing-fix-possible-null-pointer-dereferences.patch queue-3.0/ftrace-move-ftrace_filter_lseek-out-of-config_dynamic_ftrace-section.patch

#3 lore raw
In reply to Li Zefan
Quoted 6 lines · Li Zefan

commit 6a76f8c0ab19f215af2a3442870eeb5f0e81998d upstream.

Currently set_ftrace_pid and set_graph_function files use seq_lseek for their fops. However seq_open() is called only for FMODE_READ in the fops->open() so that if an user tries to seek one of those file when she open it for writing, it sees NULL seq_file and then panic.

You lost the original authorship of this patch, which is a very bad thing to do.

Ugh, time to edit emails by hand, not my favorite pasttime, someone owes me a drink...

greg k-h

author
#4 lore raw
Quoted 11 lines · Greg Kroah-Hartman
Quoted 6 lines · Li Zefan

commit 6a76f8c0ab19f215af2a3442870eeb5f0e81998d upstream.

Currently set_ftrace_pid and set_graph_function files use seq_lseek for their fops. However seq_open() is called only for FMODE_READ in the fops->open() so that if an user tries to seek one of those file when she open it for writing, it sees NULL seq_file and then panic.

You lost the original authorship of this patch, which is a very bad thing to do.

Oops, my bad.

Quoted 3 lines

Ugh, time to edit emails by hand, not my favorite pasttime, someone owes me a drink...

Sure, maybe at Plumber. ;)

maintainer Applied
#5 lore raw

Patch "tracing: Fix possible NULL pointer dereferences" has been added to the 3.0-stable tree

In reply to Li Zefan

This is a note to let you know that I've just added the patch titled

tracing: Fix possible NULL pointer dereferences

to the 3.0-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
tracing-fix-possible-null-pointer-dereferences.patch
and it can be found in the queue-3.0 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From [email protected] Mon Jun 10 14:16:36 2013

From: Li Zefan <[email protected]>
Date: Fri, 7 Jun 2013 17:00:33 +0800
Subject: tracing: Fix possible NULL pointer dereferences
To: Greg Kroah-Hartman <[email protected]>
Cc: Namhyung Kim <[email protected]>, Frederic Weisbecker <[email protected]>, Steven Rostedt <[email protected]>, Ingo Molnar <[email protected]>, LKML <[email protected]>, stable <[email protected]>
Message-ID: <[email protected]>

From: Namhyung Kim <[email protected]>

commit 6a76f8c0ab19f215af2a3442870eeb5f0e81998d upstream.

Currently set_ftrace_pid and set_graph_function files use seq_lseek for their fops. However seq_open() is called only for FMODE_READ in the fops->open() so that if an user tries to seek one of those file when she open it for writing, it sees NULL seq_file and then panic.

It can be easily reproduced with following command:

  $ cd /sys/kernel/debug/tracing
  $ echo 1234 | sudo tee -a set_ftrace_pid

In this example, GNU coreutils' tee opens the file with fopen(, "a") and then the fopen() internally calls lseek().

Link:
http://lkml.kernel.org/r/[email protected]

Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
[ lizf: adjust context ]
---
 kernel/trace/ftrace.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
kernel/trace/ftrace.c +5 -5
@@ -2300,7 +2300,7 @@ ftrace_notrace_open(struct inode *inode,
 }
 
 static loff_t
-ftrace_regex_lseek(struct file *file, loff_t offset, int origin)
+ftrace_filter_lseek(struct file *file, loff_t offset, int origin)
 {
 	loff_t ret;
 
@@ -3118,7 +3118,7 @@ static const struct file_operations ftra
 	.open = ftrace_filter_open,
 	.read = seq_read,
 	.write = ftrace_filter_write,
-	.llseek = ftrace_regex_lseek,
+	.llseek = ftrace_filter_lseek,
 	.release = ftrace_regex_release,
 };
 
@@ -3126,7 +3126,7 @@ static const struct file_operations ftra
 	.open = ftrace_notrace_open,
 	.read = seq_read,
 	.write = ftrace_notrace_write,
-	.llseek = ftrace_regex_lseek,
+	.llseek = ftrace_filter_lseek,
 	.release = ftrace_regex_release,
 };
 
@@ -3335,8 +3335,8 @@ static const struct file_operations ftra
 	.open		= ftrace_graph_open,
 	.read		= seq_read,
 	.write		= ftrace_graph_write,
+	.llseek		= ftrace_filter_lseek,
 	.release	= ftrace_graph_release,
-	.llseek		= seq_lseek,
 };
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
 
@@ -3822,7 +3822,7 @@ static const struct file_operations ftra
 	.open		= ftrace_pid_open,
 	.write		= ftrace_pid_write,
 	.read		= seq_read,
-	.llseek		= seq_lseek,
+	.llseek		= ftrace_filter_lseek,
 	.release	= ftrace_pid_release,
 };
 

Patches currently in stable-queue which might be from [email protected] are

queue-3.0/tracing-fix-possible-null-pointer-dereferences.patch queue-3.0/ftrace-move-ftrace_filter_lseek-out-of-config_dynamic_ftrace-section.patch