* [PATCH 0/3] [GIT PULL][3.8] tracing: Minor fixes for 3.8 @ 2013-01-02 22:50 Steven Rostedt 2013-01-02 22:50 ` [PATCH 1/3] ftrace: Be first to run code modification on modules Steven Rostedt ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Steven Rostedt @ 2013-01-02 22:50 UTC (permalink / raw) To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton [-- Attachment #1: Type: text/plain, Size: 779 bytes --] Ingo, As it's still an early -rc, it would be good to get these fixes into mainline before the next release. Thanks, -- Steve Please pull the latest tip/perf/urgent tree, which can be found at: git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git tip/perf/urgent Head SHA1: f9dd9809c1426ccc7e997201b9b839c96ac810ec Jovi Zhang (1): tracing: Verify target file before registering a uprobe event Steven Rostedt (2): ftrace: Be first to run code modification on modules tracing: Fix sparse warning with is_signed_type() macro ---- include/linux/ftrace_event.h | 2 +- kernel/trace/ftrace.c | 2 +- kernel/trace/trace_uprobe.c | 6 +++++- 3 files changed, 7 insertions(+), 3 deletions(-) [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] ftrace: Be first to run code modification on modules 2013-01-02 22:50 [PATCH 0/3] [GIT PULL][3.8] tracing: Minor fixes for 3.8 Steven Rostedt @ 2013-01-02 22:50 ` Steven Rostedt 2013-01-02 22:50 ` [PATCH 2/3] tracing: Fix sparse warning with is_signed_type() macro Steven Rostedt 2013-01-02 22:50 ` [PATCH 3/3] tracing: Verify target file before registering a uprobe event Steven Rostedt 2 siblings, 0 replies; 8+ messages in thread From: Steven Rostedt @ 2013-01-02 22:50 UTC (permalink / raw) To: linux-kernel Cc: Ingo Molnar, Andrew Morton, Masami Hiramatsu, Frank Ch. Eigler [-- Attachment #1: Type: text/plain, Size: 3185 bytes --] From: Steven Rostedt <srostedt@redhat.com> If some other kernel subsystem has a module notifier, and adds a kprobe to a ftrace mcount point (now that kprobes work on ftrace points), when the ftrace notifier runs it will fail and disable ftrace, as well as kprobes that are attached to ftrace points. Here's the error: WARNING: at kernel/trace/ftrace.c:1618 ftrace_bug+0x239/0x280() Hardware name: Bochs Modules linked in: fat(+) stap_56d28a51b3fe546293ca0700b10bcb29__8059(F) nfsv4 auth_rpcgss nfs dns_resolver fscache xt_nat iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack lockd sunrpc ppdev parport_pc parport microcode virtio_net i2c_piix4 drm_kms_helper ttm drm i2c_core [last unloaded: bid_shared] Pid: 8068, comm: modprobe Tainted: GF 3.7.0-0.rc8.git0.1.fc19.x86_64 #1 Call Trace: [<ffffffff8105e70f>] warn_slowpath_common+0x7f/0xc0 [<ffffffff81134106>] ? __probe_kernel_read+0x46/0x70 [<ffffffffa0180000>] ? 0xffffffffa017ffff [<ffffffffa0180000>] ? 0xffffffffa017ffff [<ffffffff8105e76a>] warn_slowpath_null+0x1a/0x20 [<ffffffff810fd189>] ftrace_bug+0x239/0x280 [<ffffffff810fd626>] ftrace_process_locs+0x376/0x520 [<ffffffff810fefb7>] ftrace_module_notify+0x47/0x50 [<ffffffff8163912d>] notifier_call_chain+0x4d/0x70 [<ffffffff810882f8>] __blocking_notifier_call_chain+0x58/0x80 [<ffffffff81088336>] blocking_notifier_call_chain+0x16/0x20 [<ffffffff810c2a23>] sys_init_module+0x73/0x220 [<ffffffff8163d719>] system_call_fastpath+0x16/0x1b ---[ end trace 9ef46351e53bbf80 ]--- ftrace failed to modify [<ffffffffa0180000>] init_once+0x0/0x20 [fat] actual: cc:bb:d2:4b:e1 A kprobe was added to the init_once() function in the fat module on load. But this happened before ftrace could have touched the code. As ftrace didn't run yet, the kprobe system had no idea it was a ftrace point and simply added a breakpoint to the code (0xcc in the cc:bb:d2:4b:e1). Then when ftrace went to modify the location from a call to mcount/fentry into a nop, it didn't see a call op, but instead it saw the breakpoint op and not knowing what to do with it, ftrace shut itself down. The solution is to simply give the ftrace module notifier the max priority. This should have been done regardless, as the core code ftrace modification also happens very early on in boot up. This makes the module modification closer to core modification. Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> Reported-by: Frank Ch. Eigler <fche@redhat.com> Signed-off-by: Steven Rostedt <rostedt@goodmis.org> --- kernel/trace/ftrace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 51b7159..356bc2f 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -3998,7 +3998,7 @@ static int ftrace_module_notify(struct notifier_block *self, struct notifier_block ftrace_module_nb = { .notifier_call = ftrace_module_notify, - .priority = 0, + .priority = INT_MAX, /* Run before anything that can use kprobes */ }; extern unsigned long __start_mcount_loc[]; -- 1.7.10.4 [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] tracing: Fix sparse warning with is_signed_type() macro 2013-01-02 22:50 [PATCH 0/3] [GIT PULL][3.8] tracing: Minor fixes for 3.8 Steven Rostedt 2013-01-02 22:50 ` [PATCH 1/3] ftrace: Be first to run code modification on modules Steven Rostedt @ 2013-01-02 22:50 ` Steven Rostedt 2013-01-02 22:50 ` [PATCH 3/3] tracing: Verify target file before registering a uprobe event Steven Rostedt 2 siblings, 0 replies; 8+ messages in thread From: Steven Rostedt @ 2013-01-02 22:50 UTC (permalink / raw) To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Robert Jarzmik [-- Attachment #1: Type: text/plain, Size: 1446 bytes --] From: Steven Rostedt <srostedt@redhat.com> Sparse complains when is_signed_type() is used on a pointer. This macro is needed for the format output used for ftrace and perf, to know if a binary field is a signed type or not. The is_signed_type() macro is used against all fields that are recorded by events to automate the operation. The problem sparse has is with the current way is_signed_type() works: ((type)-1 < 0) If "type" is a poiner, than sparse does not like it being compared to an integer (zero). The simple fix is to just give zero the same type. The runtime result stays the same. Reported-by: Robert Jarzmik <robert.jarzmik@free.fr> Signed-off-by: Steven Rostedt <rostedt@goodmis.org> --- include/linux/ftrace_event.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h index 642928c..cc87955 100644 --- a/include/linux/ftrace_event.h +++ b/include/linux/ftrace_event.h @@ -266,7 +266,7 @@ extern int trace_define_field(struct ftrace_event_call *call, const char *type, extern int trace_add_event_call(struct ftrace_event_call *call); extern void trace_remove_event_call(struct ftrace_event_call *call); -#define is_signed_type(type) (((type)(-1)) < 0) +#define is_signed_type(type) (((type)(-1)) < (type)0) int trace_set_clr_event(const char *system, const char *event, int set); -- 1.7.10.4 [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] tracing: Verify target file before registering a uprobe event 2013-01-02 22:50 [PATCH 0/3] [GIT PULL][3.8] tracing: Minor fixes for 3.8 Steven Rostedt 2013-01-02 22:50 ` [PATCH 1/3] ftrace: Be first to run code modification on modules Steven Rostedt 2013-01-02 22:50 ` [PATCH 2/3] tracing: Fix sparse warning with is_signed_type() macro Steven Rostedt @ 2013-01-02 22:50 ` Steven Rostedt 2013-01-04 4:30 ` Namhyung Kim ` (2 more replies) 2 siblings, 3 replies; 8+ messages in thread From: Steven Rostedt @ 2013-01-02 22:50 UTC (permalink / raw) To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Jovi Zhang, Srikar Dronamraju [-- Attachment #1: Type: text/plain, Size: 1310 bytes --] From: Jovi Zhang <bookjovi@gmail.com> Without this patch, we can register a uprobe event for a directory. Enabling such a uprobe event would fail anyway . Example: $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events However dirctories cannot be valid targets for uprobe. Hence verify if the target is a regular file during the probe registration. Signed-off-by: Jovi Zhang <bookjovi@gmail.com> Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> Signed-off-by: Steven Rostedt <rostedt@goodmis.org> --- kernel/trace/trace_uprobe.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index 03003cd..0815f25 100644 --- a/kernel/trace/trace_uprobe.c +++ b/kernel/trace/trace_uprobe.c @@ -257,6 +257,10 @@ static int create_trace_uprobe(int argc, char **argv) goto fail_address_parse; inode = igrab(path.dentry->d_inode); + if (!S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)) { + ret = -EINVAL; + goto fail_address_parse; + } argc -= 2; argv += 2; @@ -356,7 +360,7 @@ fail_address_parse: if (inode) iput(inode); - pr_info("Failed to parse address.\n"); + pr_info("Failed to parse address or file.\n"); return ret; } -- 1.7.10.4 [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 490 bytes --] ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] tracing: Verify target file before registering a uprobe event 2013-01-02 22:50 ` [PATCH 3/3] tracing: Verify target file before registering a uprobe event Steven Rostedt @ 2013-01-04 4:30 ` Namhyung Kim 2013-01-07 13:59 ` Steven Rostedt 2013-01-07 15:57 ` Steven Rostedt 2013-01-24 19:37 ` [tip:perf/core] " tip-bot for Jovi Zhang 2 siblings, 1 reply; 8+ messages in thread From: Namhyung Kim @ 2013-01-04 4:30 UTC (permalink / raw) To: Steven Rostedt Cc: linux-kernel, Ingo Molnar, Andrew Morton, Jovi Zhang, Srikar Dronamraju Hi, Steve. On Wed, 02 Jan 2013 17:50:38 -0500, Steven Rostedt wrote: > From: Jovi Zhang <bookjovi@gmail.com> > > Without this patch, we can register a uprobe event for a directory. > Enabling such a uprobe event would fail anyway . > > Example: > $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events > > However dirctories cannot be valid targets for uprobe. > Hence verify if the target is a regular file during the probe > registration. > > Signed-off-by: Jovi Zhang <bookjovi@gmail.com> > Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > --- > kernel/trace/trace_uprobe.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c > index 03003cd..0815f25 100644 > --- a/kernel/trace/trace_uprobe.c > +++ b/kernel/trace/trace_uprobe.c > @@ -257,6 +257,10 @@ static int create_trace_uprobe(int argc, char **argv) > goto fail_address_parse; > > inode = igrab(path.dentry->d_inode); > + if (!S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)) { Doesn't !S_ISREG() include S_ISDIR() case too? Anyway I can see an additional whitespace in front of the "if". Thanks, Namhyung > + ret = -EINVAL; > + goto fail_address_parse; > + } > > argc -= 2; > argv += 2; > @@ -356,7 +360,7 @@ fail_address_parse: > if (inode) > iput(inode); > > - pr_info("Failed to parse address.\n"); > + pr_info("Failed to parse address or file.\n"); > > return ret; > } ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] tracing: Verify target file before registering a uprobe event 2013-01-04 4:30 ` Namhyung Kim @ 2013-01-07 13:59 ` Steven Rostedt 0 siblings, 0 replies; 8+ messages in thread From: Steven Rostedt @ 2013-01-07 13:59 UTC (permalink / raw) To: Namhyung Kim Cc: linux-kernel, Ingo Molnar, Andrew Morton, Jovi Zhang, Srikar Dronamraju On Fri, 2013-01-04 at 13:30 +0900, Namhyung Kim wrote: > Hi, Steve. > > On Wed, 02 Jan 2013 17:50:38 -0500, Steven Rostedt wrote: > > From: Jovi Zhang <bookjovi@gmail.com> > > > > Without this patch, we can register a uprobe event for a directory. > > Enabling such a uprobe event would fail anyway . > > > > Example: > > $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events > > > > However dirctories cannot be valid targets for uprobe. > > Hence verify if the target is a regular file during the probe > > registration. > > > > Signed-off-by: Jovi Zhang <bookjovi@gmail.com> > > Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > > --- > > kernel/trace/trace_uprobe.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c > > index 03003cd..0815f25 100644 > > --- a/kernel/trace/trace_uprobe.c > > +++ b/kernel/trace/trace_uprobe.c > > @@ -257,6 +257,10 @@ static int create_trace_uprobe(int argc, char **argv) > > goto fail_address_parse; > > > > inode = igrab(path.dentry->d_inode); > > + if (!S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)) { > > Doesn't !S_ISREG() include S_ISDIR() case too? You're correct. Although I'm sure gcc should be smart enough to optimize that out. > > Anyway I can see an additional whitespace in front of the "if". Yep, I forgot to run checkpatch on this. OK, this bug isn't that big of a deal, so I'll take this patch out and put it into the 3.9 queue. Thanks, -- Steve > > Thanks, > Namhyung > > > > + ret = -EINVAL; > > + goto fail_address_parse; > > + } > > > > argc -= 2; > > argv += 2; > > @@ -356,7 +360,7 @@ fail_address_parse: > > if (inode) > > iput(inode); > > > > - pr_info("Failed to parse address.\n"); > > + pr_info("Failed to parse address or file.\n"); > > > > return ret; > > } ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] tracing: Verify target file before registering a uprobe event 2013-01-02 22:50 ` [PATCH 3/3] tracing: Verify target file before registering a uprobe event Steven Rostedt 2013-01-04 4:30 ` Namhyung Kim @ 2013-01-07 15:57 ` Steven Rostedt 2013-01-24 19:37 ` [tip:perf/core] " tip-bot for Jovi Zhang 2 siblings, 0 replies; 8+ messages in thread From: Steven Rostedt @ 2013-01-07 15:57 UTC (permalink / raw) To: linux-kernel; +Cc: Ingo Molnar, Andrew Morton, Jovi Zhang, Srikar Dronamraju Jovi, As Namhyung pointed out. Can you fix the below and resend. On Wed, 2013-01-02 at 17:50 -0500, Steven Rostedt wrote: > From: Jovi Zhang <bookjovi@gmail.com> > > Without this patch, we can register a uprobe event for a directory. > Enabling such a uprobe event would fail anyway . > > Example: > $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events > > However dirctories cannot be valid targets for uprobe. > Hence verify if the target is a regular file during the probe > registration. > > Signed-off-by: Jovi Zhang <bookjovi@gmail.com> > Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > --- > kernel/trace/trace_uprobe.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c > index 03003cd..0815f25 100644 > --- a/kernel/trace/trace_uprobe.c > +++ b/kernel/trace/trace_uprobe.c > @@ -257,6 +257,10 @@ static int create_trace_uprobe(int argc, char **argv) > goto fail_address_parse; > > inode = igrab(path.dentry->d_inode); > + if (!S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode)) { Bad whitespace before 'if'. Also, S_ISDIR() implies S_ISREG(), you can remove the "S_ISDIR()" check. Thanks, -- Steve > + ret = -EINVAL; > + goto fail_address_parse; > + } > > argc -= 2; > argv += 2; > @@ -356,7 +360,7 @@ fail_address_parse: > if (inode) > iput(inode); > > - pr_info("Failed to parse address.\n"); > + pr_info("Failed to parse address or file.\n"); > > return ret; > } ^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip:perf/core] tracing: Verify target file before registering a uprobe event 2013-01-02 22:50 ` [PATCH 3/3] tracing: Verify target file before registering a uprobe event Steven Rostedt 2013-01-04 4:30 ` Namhyung Kim 2013-01-07 15:57 ` Steven Rostedt @ 2013-01-24 19:37 ` tip-bot for Jovi Zhang 2 siblings, 0 replies; 8+ messages in thread From: tip-bot for Jovi Zhang @ 2013-01-24 19:37 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, mingo, rostedt, srikar, tglx, namhyung, bookjovi Commit-ID: d24d7dbf3cc49b00a152e55e24f0eeb173c7a971 Gitweb: http://git.kernel.org/tip/d24d7dbf3cc49b00a152e55e24f0eeb173c7a971 Author: Jovi Zhang <bookjovi@gmail.com> AuthorDate: Wed, 18 Jul 2012 18:16:44 +0800 Committer: Steven Rostedt <rostedt@goodmis.org> CommitDate: Mon, 21 Jan 2013 13:22:31 -0500 tracing: Verify target file before registering a uprobe event Without this patch, we can register a uprobe event for a directory. Enabling such a uprobe event would anyway fail. Example: $ echo 'p /bin:0x4245c0' > /sys/kernel/debug/tracing/uprobe_events However dirctories cannot be valid targets for uprobe. Hence verify if the target is a regular file during the probe registration. Link: http://lkml.kernel.org/r/20130103004212.690763002@goodmis.org Cc: Namhyung Kim <namhyung@kernel.org> Signed-off-by: Jovi Zhang <bookjovi@gmail.com> Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com> [ cleaned up whitespace and removed redundant IS_DIR() check ] Signed-off-by: Steven Rostedt <rostedt@goodmis.org> --- kernel/trace/trace_uprobe.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index c86e6d4..87b6db4 100644 --- a/kernel/trace/trace_uprobe.c +++ b/kernel/trace/trace_uprobe.c @@ -258,6 +258,10 @@ static int create_trace_uprobe(int argc, char **argv) goto fail_address_parse; inode = igrab(path.dentry->d_inode); + if (!S_ISREG(inode->i_mode)) { + ret = -EINVAL; + goto fail_address_parse; + } argc -= 2; argv += 2; @@ -356,7 +360,7 @@ fail_address_parse: if (inode) iput(inode); - pr_info("Failed to parse address.\n"); + pr_info("Failed to parse address or file.\n"); return ret; } ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-01-24 19:37 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-01-02 22:50 [PATCH 0/3] [GIT PULL][3.8] tracing: Minor fixes for 3.8 Steven Rostedt 2013-01-02 22:50 ` [PATCH 1/3] ftrace: Be first to run code modification on modules Steven Rostedt 2013-01-02 22:50 ` [PATCH 2/3] tracing: Fix sparse warning with is_signed_type() macro Steven Rostedt 2013-01-02 22:50 ` [PATCH 3/3] tracing: Verify target file before registering a uprobe event Steven Rostedt 2013-01-04 4:30 ` Namhyung Kim 2013-01-07 13:59 ` Steven Rostedt 2013-01-07 15:57 ` Steven Rostedt 2013-01-24 19:37 ` [tip:perf/core] " tip-bot for Jovi Zhang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).