Patchwork [REVIEW,11/15] userns: Teach trace to use from_kuid

login
register
mail settings
Submitter Eric W. Biederman
Date Aug. 26, 2012, 12:04 a.m.
Message ID <87wr0mecxz.fsf@xmission.com>
Download mbox | patch
Permalink /patch/180004/
State RFC
Delegated to: David Miller
Headers show

Comments

Eric W. Biederman - Aug. 26, 2012, 12:04 a.m.
- When tracing capture the kuid.
- When displaying the data to user space convert the kuid into the
  user namespace of the process that opened the report file.

Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 init/Kconfig         |    1 -
 kernel/trace/trace.c |    3 ++-
 kernel/trace/trace.h |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
Steven Rostedt - Aug. 26, 2012, 12:18 a.m.
On Sat, 2012-08-25 at 17:04 -0700, Eric W. Biederman wrote:
> - When tracing capture the kuid.
> - When displaying the data to user space convert the kuid into the
>   user namespace of the process that opened the report file.
> 

> index 5c38c81..c9ace83 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -2060,7 +2060,8 @@ print_trace_header(struct seq_file *m, struct trace_iterator *iter)
>  	seq_puts(m, "#    -----------------\n");
>  	seq_printf(m, "#    | task: %.16s-%d "
>  		   "(uid:%d nice:%ld policy:%ld rt_prio:%ld)\n",
> -		   data->comm, data->pid, data->uid, data->nice,
> +		   data->comm, data->pid,
> +		   from_kuid_munged(seq_user_ns(m), data->uid), data->nice,

This is a global id. That is, it stored whatever process triggered the
report, not the one reading it. Thus, two different readers could get a
different uid for the same task that triggered the latency?

-- Steve

>  		   data->policy, data->rt_priority);
>  	seq_puts(m, "#    -----------------\n");
>  
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 55e1f7f..40a6f30 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -147,7 +147,7 @@ struct trace_array_cpu {
>  	unsigned long		skipped_entries;
>  	cycle_t			preempt_timestamp;
>  	pid_t			pid;
> -	uid_t			uid;
> +	kuid_t			uid;
>  	char			comm[TASK_COMM_LEN];
>  };
>  


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric W. Biederman - Aug. 26, 2012, 12:28 a.m.
Steven Rostedt <rostedt@goodmis.org> writes:

> On Sat, 2012-08-25 at 17:04 -0700, Eric W. Biederman wrote:
>> - When tracing capture the kuid.
>> - When displaying the data to user space convert the kuid into the
>>   user namespace of the process that opened the report file.
>> 
>
>> index 5c38c81..c9ace83 100644
>> --- a/kernel/trace/trace.c
>> +++ b/kernel/trace/trace.c
>> @@ -2060,7 +2060,8 @@ print_trace_header(struct seq_file *m, struct trace_iterator *iter)
>>  	seq_puts(m, "#    -----------------\n");
>>  	seq_printf(m, "#    | task: %.16s-%d "
>>  		   "(uid:%d nice:%ld policy:%ld rt_prio:%ld)\n",
>> -		   data->comm, data->pid, data->uid, data->nice,
>> +		   data->comm, data->pid,
>> +		   from_kuid_munged(seq_user_ns(m), data->uid), data->nice,
>
> This is a global id. That is, it stored whatever process triggered the
> report, not the one reading it. Thus, two different readers could get a
> different uid for the same task that triggered the latency?

Yes the stored value is a kuid_t the global kernel internal form.

We report the value as a uid_t in the user namespace of the reader.  So
if two different processes in different user namespaces read the file
they can see different values.

Now I don't expect in practice we will allow anyone who isn't
the global root user to even think of looking at debugfs, but in
case we do we might as well handle this as best as we can.

Eric
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/init/Kconfig b/init/Kconfig
index af41fa7..d849ba2 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -927,7 +927,6 @@  config UIDGID_CONVERTED
 	# Features
 	depends on IMA = n
 	depends on EVM = n
-	depends on TRACING = n
 	depends on FS_POSIX_ACL = n
 	depends on QUOTA = n
 	depends on QUOTACTL = n
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 5c38c81..c9ace83 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2060,7 +2060,8 @@  print_trace_header(struct seq_file *m, struct trace_iterator *iter)
 	seq_puts(m, "#    -----------------\n");
 	seq_printf(m, "#    | task: %.16s-%d "
 		   "(uid:%d nice:%ld policy:%ld rt_prio:%ld)\n",
-		   data->comm, data->pid, data->uid, data->nice,
+		   data->comm, data->pid,
+		   from_kuid_munged(seq_user_ns(m), data->uid), data->nice,
 		   data->policy, data->rt_priority);
 	seq_puts(m, "#    -----------------\n");
 
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 55e1f7f..40a6f30 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -147,7 +147,7 @@  struct trace_array_cpu {
 	unsigned long		skipped_entries;
 	cycle_t			preempt_timestamp;
 	pid_t			pid;
-	uid_t			uid;
+	kuid_t			uid;
 	char			comm[TASK_COMM_LEN];
 };