[PATCH 0/3] [GIT PULL] tracing: Minor fixes for 5.1-rc2

[PATCH 0/3] [GIT PULL] tracing: Minor fixes for 5.1-rc2

[Steven Rostedt]

Linus,

Three small fixes:

 - A fix to a double free in the histogram code
 - Uninitialized variable fix
 - Use NULL instead of zero fix and spelling fixes

Please pull the latest trace-v5.1-rc2 tree, which can be found at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-v5.1-rc2

Tag SHA1: ed41a01c516cdb7f43a2e0fde385f94f06dff5ff
Head SHA1: 9efb85c5cfac7e1f0caae4471446d936ff2163fe


Frank Rowand (1):
      tracing: initialize variable in create_dyn_event()

Hariprasad Kelam (1):
      ftrace: Fix warning using plain integer as NULL & spelling corrections

Tom Zanussi (1):
      tracing: Remove unnecessary var_ref destroy in track_data_destroy()

----
 kernel/trace/ftrace.c            | 12 ++++++------
 kernel/trace/trace_dynevent.c    |  2 +-
 kernel/trace/trace_events_hist.c |  1 -
 3 files changed, 7 insertions(+), 8 deletions(-)

[PATCH 1/3] tracing: Remove unnecessary var_ref destroy in track_data_destroy()

[Steven Rostedt]

From: Tom Zanussi <tom.zanussi@linux.intel.com>

Commit 656fe2ba85e8 (tracing: Use hist trigger's var_ref array to
destroy var_refs) centralized the destruction of all the var_refs
in one place so that other code didn't have to do it.

The track_data_destroy() added later ignored that and also destroyed
the track_data var_ref, causing a double-free error flagged by KASAN.

==================================================================
BUG: KASAN: use-after-free in destroy_hist_field+0x30/0x70
Read of size 8 at addr ffff888086df2210 by task bash/1694

CPU: 6 PID: 1694 Comm: bash Not tainted 5.1.0-rc1-test+ #15
Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v03.03
07/14/2016
Call Trace:
 dump_stack+0x71/0xa0
 ? destroy_hist_field+0x30/0x70
 print_address_description.cold.3+0x9/0x1fb
 ? destroy_hist_field+0x30/0x70
 ? destroy_hist_field+0x30/0x70
 kasan_report.cold.4+0x1a/0x33
 ? __kasan_slab_free+0x100/0x150
 ? destroy_hist_field+0x30/0x70
 destroy_hist_field+0x30/0x70
 track_data_destroy+0x55/0xe0
 destroy_hist_data+0x1f0/0x350
 hist_unreg_all+0x203/0x220
 event_trigger_open+0xbb/0x130
 do_dentry_open+0x296/0x700
 ? stacktrace_count_trigger+0x30/0x30
 ? generic_permission+0x56/0x200
 ? __x64_sys_fchdir+0xd0/0xd0
 ? inode_permission+0x55/0x200
 ? security_inode_permission+0x18/0x60
 path_openat+0x633/0x22b0
 ? path_lookupat.isra.50+0x420/0x420
 ? __kasan_kmalloc.constprop.12+0xc1/0xd0
 ? kmem_cache_alloc+0xe5/0x260
 ? getname_flags+0x6c/0x2a0
 ? do_sys_open+0x149/0x2b0
 ? do_syscall_64+0x73/0x1b0
 ? entry_SYSCALL_64_after_hwframe+0x44/0xa9
 ? _raw_write_lock_bh+0xe0/0xe0
 ? __kernel_text_address+0xe/0x30
 ? unwind_get_return_address+0x2f/0x50
 ? __list_add_valid+0x2d/0x70
 ? deactivate_slab.isra.62+0x1f4/0x5a0
 ? getname_flags+0x6c/0x2a0
 ? set_track+0x76/0x120
 do_filp_open+0x11a/0x1a0
 ? may_open_dev+0x50/0x50
 ? _raw_spin_lock+0x7a/0xd0
 ? _raw_write_lock_bh+0xe0/0xe0
 ? __alloc_fd+0x10f/0x200
 do_sys_open+0x1db/0x2b0
 ? filp_open+0x50/0x50
 do_syscall_64+0x73/0x1b0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7fa7b24a4ca2
Code: 25 00 00 41 00 3d 00 00 41 00 74 4c 48 8d 05 85 7a 0d 00 8b 00 85 c0
75 6d 89 f2 b8 01 01 00 00 48 89 fe bf 9c ff ff ff 0f 05 <48> 3d 00 f0 ff ff
0f 87 a2 00 00 00 48 8b 4c 24 28 64 48 33 0c 25
RSP: 002b:00007fffbafb3af0 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
RAX: ffffffffffffffda RBX: 000055d3648ade30 RCX: 00007fa7b24a4ca2
RDX: 0000000000000241 RSI: 000055d364a55240 RDI: 00000000ffffff9c
RBP: 00007fffbafb3bf0 R08: 0000000000000020 R09: 0000000000000002
R10: 00000000000001b6 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000000003 R14: 0000000000000001 R15: 000055d364a55240
==================================================================

So remove the track_data_destroy() destroy_hist_field() call for that
var_ref.

Link: http://lkml.kernel.org/r/1deffec420f6a16d11dd8647318d34a66d1989a9.camel@linux.intel.com

Fixes: 466f4528fbc69 ("tracing: Generalize hist trigger onmax and save action")
Reported-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Tom Zanussi <tom.zanussi@linux.intel.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_hist.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index ca46339f3009..795aa2038377 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -3713,7 +3713,6 @@ static void track_data_destroy(struct hist_trigger_data *hist_data,
 	struct trace_event_file *file = hist_data->event_file;
 
 	destroy_hist_field(data->track_data.track_var, 0);
-	destroy_hist_field(data->track_data.var_ref, 0);
 
 	if (data->action == ACTION_SNAPSHOT) {
 		struct track_data *track_data;
-- 
2.20.1

[PATCH 2/3] tracing: initialize variable in create_dyn_event()

[Steven Rostedt]

From: Frank Rowand <frank.rowand@sony.com>

Fix compile warning in create_dyn_event(): 'ret' may be used uninitialized
in this function [-Wuninitialized].

Link: http://lkml.kernel.org/r/1553237900-8555-1-git-send-email-frowand.list@gmail.com

Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
Cc: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Cc: stable@vger.kernel.org
Fixes: 5448d44c3855 ("tracing: Add unified dynamic event framework")
Signed-off-by: Frank Rowand <frank.rowand@sony.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_dynevent.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c
index dd1f43588d70..fa100ed3b4de 100644
--- a/kernel/trace/trace_dynevent.c
+++ b/kernel/trace/trace_dynevent.c
@@ -74,7 +74,7 @@ int dyn_event_release(int argc, char **argv, struct dyn_event_operations *type)
 static int create_dyn_event(int argc, char **argv)
 {
 	struct dyn_event_operations *ops;
-	int ret;
+	int ret = -ENODEV;
 
 	if (argv[0][0] == '-' || argv[0][0] == '!')
 		return dyn_event_release(argc, argv, NULL);
-- 
2.20.1

[PATCH 3/3] ftrace: Fix warning using plain integer as NULL & spelling corrections

[Steven Rostedt]

From: Hariprasad Kelam <hariprasad.kelam@gmail.com>

Changed  0 --> NULL to avoid sparse warning
Corrected spelling mistakes reported by checkpatch.pl
Sparse warning below:

sudo make C=2 CF=-D__CHECK_ENDIAN__ M=kernel/trace

CHECK   kernel/trace/ftrace.c
kernel/trace/ftrace.c:3007:24: warning: Using plain integer as NULL pointer
kernel/trace/ftrace.c:4758:37: warning: Using plain integer as NULL pointer

Link: http://lkml.kernel.org/r/20190323183523.GA2244@hari-Inspiron-1545

Signed-off-by: Hariprasad Kelam <hariprasad.kelam@gmail.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/ftrace.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index fa79323331b2..26c8ca9bd06b 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1992,7 +1992,7 @@ static void print_bug_type(void)
  * modifying the code. @failed should be one of either:
  * EFAULT - if the problem happens on reading the @ip address
  * EINVAL - if what is read at @ip is not what was expected
- * EPERM - if the problem happens on writting to the @ip address
+ * EPERM - if the problem happens on writing to the @ip address
  */
 void ftrace_bug(int failed, struct dyn_ftrace *rec)
 {
@@ -2391,7 +2391,7 @@ __ftrace_replace_code(struct dyn_ftrace *rec, int enable)
 		return ftrace_modify_call(rec, ftrace_old_addr, ftrace_addr);
 	}
 
-	return -1; /* unknow ftrace bug */
+	return -1; /* unknown ftrace bug */
 }
 
 void __weak ftrace_replace_code(int mod_flags)
@@ -3004,7 +3004,7 @@ ftrace_allocate_pages(unsigned long num_to_init)
 	int cnt;
 
 	if (!num_to_init)
-		return 0;
+		return NULL;
 
 	start_pg = pg = kzalloc(sizeof(*pg), GFP_KERNEL);
 	if (!pg)
@@ -4755,7 +4755,7 @@ static int
 ftrace_set_addr(struct ftrace_ops *ops, unsigned long ip, int remove,
 		int reset, int enable)
 {
-	return ftrace_set_hash(ops, 0, 0, ip, remove, reset, enable);
+	return ftrace_set_hash(ops, NULL, 0, ip, remove, reset, enable);
 }
 
 /**
@@ -5463,7 +5463,7 @@ void ftrace_create_filter_files(struct ftrace_ops *ops,
 
 /*
  * The name "destroy_filter_files" is really a misnomer. Although
- * in the future, it may actualy delete the files, but this is
+ * in the future, it may actually delete the files, but this is
  * really intended to make sure the ops passed in are disabled
  * and that when this function returns, the caller is free to
  * free the ops.
@@ -5786,7 +5786,7 @@ void ftrace_module_enable(struct module *mod)
 	/*
 	 * If the tracing is enabled, go ahead and enable the record.
 	 *
-	 * The reason not to enable the record immediatelly is the
+	 * The reason not to enable the record immediately is the
 	 * inherent check of ftrace_make_nop/ftrace_make_call for
 	 * correct previous instructions.  Making first the NOP
 	 * conversion puts the module to the correct state, thus
-- 
2.20.1

Re: [PATCH 0/3] [GIT PULL] tracing: Minor fixes for 5.1-rc2

[Linus Torvalds]

On Tue, Mar 26, 2019 at 5:42 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Please pull the latest trace-v5.1-rc2 tree, which can be found at:

Pulled and pushed out, but for some reason the pr-tracker-bot doesn't
seem to have reacted.

Presumably there's something in your git pull message that means it
doesn't trigger.

And I suspect I know what it is: the patch that your subject line
starts with "[PATCH..]" instead of "[GIT PULL..]"

              Linus

Re: [PATCH 0/3] [GIT PULL] tracing: Minor fixes for 5.1-rc2

[Steven Rostedt]

On Tue, 26 Mar 2019 10:48:44 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, Mar 26, 2019 at 5:42 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > Please pull the latest trace-v5.1-rc2 tree, which can be found at:  
> 
> Pulled and pushed out, but for some reason the pr-tracker-bot doesn't
> seem to have reacted.
> 
> Presumably there's something in your git pull message that means it
> doesn't trigger.
> 
> And I suspect I know what it is: the patch that your subject line
> starts with "[PATCH..]" instead of "[GIT PULL..]"
>

Interesting. This is how I've always sent patches to you via quilt.

I usually don't put fixes sent to me through for-next, as I haven't
been convinced that that helps any. I make it a requirement to send any
patch I push into my tree for merging as a separate patch. As I usually
do that when I push to linux-next, if I send directly to you, I will
then send them individually.

The point being, when I send individual patches directly to you (and
not as a single email pull request), I use quilt sendmail. Which will
append the "[PATCH 0/x]" before my cover message subject.

Should I work on changing this?

-- Steve

Re: [PATCH 0/3] [GIT PULL] tracing: Minor fixes for 5.1-rc2

[Linus Torvalds]

On Tue, Mar 26, 2019 at 11:48 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Should I work on changing this?

I don't personally much care, but the pr-tracker-bot clearly does.

If you don't care about the automated "it's been pulled" message, that
doesn't matter, of course.

That said, I'd almost prefer to get just the regular pull request
without the patches. If the complete patch is small, it's often nice
to see that _in_ the pull request (at the bottom), but I don't
generally need or want the individual patches themselves as separate
emails (unless there's some particular reason you want me to comment
on something, or apply them directly as patches).

                   Linus

Re: [PATCH 0/3] [GIT PULL] tracing: Minor fixes for 5.1-rc2

[Steven Rostedt]

On Tue, 26 Mar 2019 14:29:27 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Tue, Mar 26, 2019 at 11:48 AM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > Should I work on changing this?  
> 
> I don't personally much care, but the pr-tracker-bot clearly does.
> 
> If you don't care about the automated "it's been pulled" message, that
> doesn't matter, of course.

I'm subscribed to your git tree and filter out all updates for my
signed off by. An then I get those emails. So I get the "Pulled"
message regardless.

> 
> That said, I'd almost prefer to get just the regular pull request
> without the patches. If the complete patch is small, it's often nice
> to see that _in_ the pull request (at the bottom), but I don't
> generally need or want the individual patches themselves as separate
> emails (unless there's some particular reason you want me to comment
> on something, or apply them directly as patches).

It's not really for you, but more for transparency in general. I have a
rule that I don't push anything to you that I haven't personally sent
to LKML as a separate patch.

It's not a big deal. I could make this a two step process, and send
these as my "for-next" patches so that people (including the authors of
the patch) know that I'm sending them upstream.

-- Steve