[PATCH 0/5] [GIT PULL] tracing: A few last minute clean up and fixes

[PATCH 0/5] [GIT PULL] tracing: A few last minute clean up and fixes

[Steven Rostedt]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 977 bytes --]


Linus,

A few clean ups and bug fixes

 - Replace open coded "ARRAY_SIZE()" with macro

 - Updates to uprobes

 - Bug fix for perf event filter on error path

Please pull the latest ktest-v4.17-2 tree, which can be found at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-ktest.git
ktest-v4.17-2

Tag SHA1: f4a56400c3b9f949cf5740153aab48a8240f88db
Head SHA1: 0b3dec05dbbce023f4f25aba975b5d253c313ebb


Howard McLauchlan (1):
      tracing/uprobe: Add support for overlayfs

Jérémy Lefaure (1):
      tracing: Use ARRAY_SIZE() macro instead of open coding it

Ravi Bangoria (2):
      trace_uprobe: Use %lx to display offset
      trace_uprobe: Simplify probes_seq_show()

Steven Rostedt (VMware) (1):
      tracing: Enforce passing in filter=NULL to create_filter()

----
 kernel/trace/trace_events_filter.c | 26 +++++++++++---------------
 kernel/trace/trace_uprobe.c        | 23 ++++-------------------
 2 files changed, 15 insertions(+), 34 deletions(-)

[PATCH 2/5] tracing/uprobe: Add support for overlayfs

[Steven Rostedt]

[-- Attachment #1: 0002-tracing-uprobe-Add-support-for-overlayfs.patch --]
[-- Type: text/plain, Size: 2037 bytes --]

From: Howard McLauchlan <hmclauchlan@fb.com>

uprobes cannot successfully attach to binaries located in a directory
mounted with overlayfs.

To verify, create directories for mounting overlayfs
(upper,lower,work,merge), move some binary into merge/ and use readelf
to obtain some known instruction of the binary. I used /bin/true and the
entry instruction(0x13b0):

	$ mount -t overlay overlay -o lowerdir=lower,upperdir=upper,workdir=work merge
	$ cd /sys/kernel/debug/tracing
	$ echo 'p:true_entry PATH_TO_MERGE/merge/true:0x13b0' > uprobe_events
	$ echo 1 > events/uprobes/true_entry/enable

This returns 'bash: echo: write error: Input/output error' and dmesg
tells us 'event trace: Could not enable event true_entry'

This change makes create_trace_uprobe() look for the real inode of a
dentry. In the case of normal filesystems, this simplifies to just
returning the inode. In the case of overlayfs(and similar fs) we will
obtain the underlying dentry and corresponding inode, upon which uprobes
can successfully register.

Running the example above with the patch applied, we can see that the
uprobe is enabled and will output to trace as expected.

Link: http://lkml.kernel.org/r/20180410231030.2720-1-hmclauchlan@fb.com

Reviewed-by: Josef Bacik <jbacik@fb.com>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Signed-off-by: Howard McLauchlan <hmclauchlan@fb.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_uprobe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 268029ae1be6..8b86d76c55ee 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -446,7 +446,7 @@ static int create_trace_uprobe(int argc, char **argv)
 	if (ret)
 		goto fail_address_parse;
 
-	inode = igrab(d_inode(path.dentry));
+	inode = igrab(d_real_inode(path.dentry));
 	path_put(&path);
 
 	if (!inode || !S_ISREG(inode->i_mode)) {
-- 
2.16.3

[PATCH 3/5] trace_uprobe: Use %lx to display offset

[Steven Rostedt]

[-- Attachment #1: 0003-trace_uprobe-Use-lx-to-display-offset.patch --]
[-- Type: text/plain, Size: 1157 bytes --]

From: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>

tu->offset is unsigned long, not a pointer, thus %lx should
be used to print it, not the %px.

Link: http://lkml.kernel.org/r/20180315082756.9050-1-ravi.bangoria@linux.vnet.ibm.com

Cc: stable@vger.kernel.org
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Fixes: 0e4d819d0893 ("trace_uprobe: Display correct offset in uprobe_events")
Suggested-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_uprobe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 8b86d76c55ee..d7d3c9237f64 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -608,7 +608,7 @@ static int probes_seq_show(struct seq_file *m, void *v)
 
 	/* Don't print "0x  (null)" when offset is 0 */
 	if (tu->offset) {
-		seq_printf(m, "0x%px", (void *)tu->offset);
+		seq_printf(m, "0x%0*lx", (int)(sizeof(void *) * 2), tu->offset);
 	} else {
 		switch (sizeof(void *)) {
 		case 4:
-- 
2.16.3

[PATCH 4/5] trace_uprobe: Simplify probes_seq_show()

[Steven Rostedt]

[-- Attachment #1: 0004-trace_uprobe-Simplify-probes_seq_show.patch --]
[-- Type: text/plain, Size: 1556 bytes --]

From: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>

Simplify probes_seq_show() function. No change in output
before and after patch.

Link: http://lkml.kernel.org/r/20180315082756.9050-2-ravi.bangoria@linux.vnet.ibm.com

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_uprobe.c | 21 +++------------------
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index d7d3c9237f64..21604754bb79 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -602,24 +602,9 @@ static int probes_seq_show(struct seq_file *m, void *v)
 	char c = is_ret_probe(tu) ? 'r' : 'p';
 	int i;
 
-	seq_printf(m, "%c:%s/%s", c, tu->tp.call.class->system,
-			trace_event_name(&tu->tp.call));
-	seq_printf(m, " %s:", tu->filename);
-
-	/* Don't print "0x  (null)" when offset is 0 */
-	if (tu->offset) {
-		seq_printf(m, "0x%0*lx", (int)(sizeof(void *) * 2), tu->offset);
-	} else {
-		switch (sizeof(void *)) {
-		case 4:
-			seq_printf(m, "0x00000000");
-			break;
-		case 8:
-		default:
-			seq_printf(m, "0x0000000000000000");
-			break;
-		}
-	}
+	seq_printf(m, "%c:%s/%s %s:0x%0*lx", c, tu->tp.call.class->system,
+			trace_event_name(&tu->tp.call), tu->filename,
+			(int)(sizeof(void *) * 2), tu->offset);
 
 	for (i = 0; i < tu->tp.nr_args; i++)
 		seq_printf(m, " %s=%s", tu->tp.args[i].name, tu->tp.args[i].comm);
-- 
2.16.3

[PATCH 5/5] tracing: Enforce passing in filter=NULL to create_filter()

[Steven Rostedt]

[-- Attachment #1: 0005-tracing-Enforce-passing-in-filter-NULL-to-create_fil.patch --]
[-- Type: text/plain, Size: 3807 bytes --]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

There's some inconsistency with what to set the output parameter filterp
when passing to create_filter(..., struct event_filter **filterp).

Whatever filterp points to, should be NULL when calling this function. The
create_filter() calls create_filter_start() with a pointer to a local
"filter" variable that is set to NULL. The create_filter_start() has a
WARN_ON() if the passed in pointer isn't pointing to a value set to NULL.

Ideally, create_filter() should pass the filterp variable it received to
create_filter_start() and not hide it as with a local variable, this allowed
create_filter() to fail, and not update the passed in filter, and the caller
of create_filter() then tried to free filter, which was never initialized to
anything, causing memory corruption.

Link: http://lkml.kernel.org/r/00000000000032a0c30569916870@google.com

Fixes: 80765597bc587 ("tracing: Rewrite filter logic to be simpler and faster")
Reported-by: syzbot+dadcc936587643d7f568@syzkaller.appspotmail.com
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 kernel/trace/trace_events_filter.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 5eba1cec945c..9b4716bb8bb0 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -1704,18 +1704,16 @@ static int create_filter(struct trace_event_call *call,
 			 struct event_filter **filterp)
 {
 	struct filter_parse_error *pe = NULL;
-	struct event_filter *filter = NULL;
 	int err;
 
-	err = create_filter_start(filter_string, set_str, &pe, &filter);
+	err = create_filter_start(filter_string, set_str, &pe, filterp);
 	if (err)
 		return err;
 
-	err = process_preds(call, filter_string, filter, pe);
+	err = process_preds(call, filter_string, *filterp, pe);
 	if (err && set_str)
-		append_filter_err(pe, filter);
+		append_filter_err(pe, *filterp);
 
-	*filterp = filter;
 	return err;
 }
 
@@ -1739,24 +1737,22 @@ static int create_system_filter(struct trace_subsystem_dir *dir,
 				struct trace_array *tr,
 				char *filter_str, struct event_filter **filterp)
 {
-	struct event_filter *filter = NULL;
 	struct filter_parse_error *pe = NULL;
 	int err;
 
-	err = create_filter_start(filter_str, true, &pe, &filter);
+	err = create_filter_start(filter_str, true, &pe, filterp);
 	if (!err) {
 		err = process_system_preds(dir, tr, pe, filter_str);
 		if (!err) {
 			/* System filters just show a default message */
-			kfree(filter->filter_string);
-			filter->filter_string = NULL;
+			kfree((*filterp)->filter_string);
+			(*filterp)->filter_string = NULL;
 		} else {
-			append_filter_err(pe, filter);
+			append_filter_err(pe, *filterp);
 		}
 	}
 	create_filter_finish(pe);
 
-	*filterp = filter;
 	return err;
 }
 
@@ -1764,7 +1760,7 @@ static int create_system_filter(struct trace_subsystem_dir *dir,
 int apply_event_filter(struct trace_event_file *file, char *filter_string)
 {
 	struct trace_event_call *call = file->event_call;
-	struct event_filter *filter;
+	struct event_filter *filter = NULL;
 	int err;
 
 	if (!strcmp(strstrip(filter_string), "0")) {
@@ -1817,7 +1813,7 @@ int apply_subsystem_event_filter(struct trace_subsystem_dir *dir,
 {
 	struct event_subsystem *system = dir->subsystem;
 	struct trace_array *tr = dir->tr;
-	struct event_filter *filter;
+	struct event_filter *filter = NULL;
 	int err = 0;
 
 	mutex_lock(&event_mutex);
@@ -2024,7 +2020,7 @@ int ftrace_profile_set_filter(struct perf_event *event, int event_id,
 			      char *filter_str)
 {
 	int err;
-	struct event_filter *filter;
+	struct event_filter *filter = NULL;
 	struct trace_event_call *call;
 
 	mutex_lock(&event_mutex);
-- 
2.16.3

Re: [PATCH 0/5] [GIT PULL] tracing: A few last minute clean up and fixes

[Linus Torvalds]

On Wed, Apr 11, 2018 at 6:53 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> Please pull the latest ktest-v4.17-2 tree, which can be found at:

That doesn't exist, and makes no sense. Did you mean tracing, not ktest?

And even if you did, why did your scripts not notice that the branch
you point to doesn't actually exist?

               Linus

Re: [PATCH 0/5] [GIT PULL] tracing: A few last minute clean up and fixes

[Linus Torvalds]

On Thu, Apr 12, 2018 at 10:53 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> That doesn't exist, and makes no sense. Did you mean tracing, not ktest?

Oh, it's more f'ed up than that. That "ktest-v4.17-2" tag does exist -
in the tracing tree.

So the tag is mis-named, but in the right tree, and then your pull
request points at the wrong tree.

Do you want to fix that all up, or should I just pull from that messed-up thing?

                  Linus

Re: [PATCH 0/5] [GIT PULL] tracing: A few last minute clean up and fixes

[Steven Rostedt]

On Thu, 12 Apr 2018 10:58:21 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Thu, Apr 12, 2018 at 10:53 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > That doesn't exist, and makes no sense. Did you mean tracing, not ktest?  
> 
> Oh, it's more f'ed up than that. That "ktest-v4.17-2" tag does exist -
> in the tracing tree.
> 
> So the tag is mis-named, but in the right tree, and then your pull
> request points at the wrong tree.
> 
> Do you want to fix that all up, or should I just pull from that messed-up thing?
> 

Grumble, no don't pull it. Let me fix it.

That's what I get when I up-arrow commands. I'll retag it and make sure
it's fine, and send you another pull request.

Oh, and I just received my nitrokey and I'm playing with it. One thing
I found is that it doesn't like keys sized 3072 (gives me a bad key
error), which is my current subkey. So I created a new subkey:

Primary key fingerprint: 5ED9 A48F C54C 0A22 D1D0  804C EBC2 6CDB 5A56 DE73
     Subkey fingerprint: B5D7 BDD5 67E0 67A3 EE0C  9FBE 3F0B D661 FC59 E3D3

That is 4096. I pushed it to the key servers about a half hour ago,
hopefully, it has made its way around, as I'm going to start using it.

-- Steve

Re: [PATCH 0/5] [GIT PULL] tracing: A few last minute clean up and fixes

[Konstantin Ryabitsev]

[-- Attachment #1.1: Type: text/plain, Size: 847 bytes --]

On 04/12/18 14:28, Steven Rostedt wrote:
> That is 4096. I pushed it to the key servers about a half hour ago,
> hopefully, it has made its way around, as I'm going to start using it.

I semi-heartily recommend using an ECDSA key instead (ED25519), because
it will be much faster and we know they work fine for Linus since jejb
uses them to sign his tags. My understanding is that 4096-bit RSA keys
are pretty slow on Nitrokey Start devices.

If you want to stick to RSA instead of the new-fangled ECC (if you can
call a standard from 1980s new-fangled, that is), there is really no
benefit to having a signing subkey stronger than 2048 bits, especially
for the purposes of signing git objects -- which are only as strong as SHA1.


Regards,
-- 
Konstantin Ryabitsev
Director, IT Infrastructure Security
The Linux Foundation


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 0/5] [GIT PULL] tracing: A few last minute clean up and fixes

[Steven Rostedt]

On Thu, 12 Apr 2018 14:28:45 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> Primary key fingerprint: 5ED9 A48F C54C 0A22 D1D0  804C EBC2 6CDB 5A56 DE73
>      Subkey fingerprint: B5D7 BDD5 67E0 67A3 EE0C  9FBE 3F0B D661 FC59 E3D3

Don't use this key.

Konstantin wanted me to make a ECC key, which I did. Here's the new key:

   Subkey fingerprint: 514B 0EDE 3C38 7F94 4FB3  7993 29E5 7410 9AEB FAAA

-- Steve

Re: [PATCH 0/5] [GIT PULL] tracing: A few last minute clean up and fixes

[Konstantin Ryabitsev]

[-- Attachment #1.1: Type: text/plain, Size: 1009 bytes --]

On 04/12/18 15:46, Steven Rostedt wrote:
> On Thu, 12 Apr 2018 14:28:45 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
>> Primary key fingerprint: 5ED9 A48F C54C 0A22 D1D0  804C EBC2 6CDB 5A56 DE73
>>      Subkey fingerprint: B5D7 BDD5 67E0 67A3 EE0C  9FBE 3F0B D661 FC59 E3D3
> 
> Don't use this key.
> 
> Konstantin wanted me to make a ECC key, which I did. Here's the new key:
> 
>    Subkey fingerprint: 514B 0EDE 3C38 7F94 4FB3  7993 29E5 7410 9AEB FAAA

Nice!

You don't actually have to tell people the fingerprint, as the subkey
will inherit the trust/validity of your master key. For the recipients,
it's sufficient to just refresh your key:

gpg2 --refresh-key rostedt@goodmis.org

To validate ECC tags, you will need to tell git to always use gpg2,
since gpg1 doesn't know what ECC is:

git config --global gpg.program gpg2
git config --global gpgv.program gpgv2

Regards,
-- 
Konstantin Ryabitsev
Director, IT Infrastructure Security
The Linux Foundation


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]