Re: [PATCH 1/3] ftrace: add function tracing to single thread


On Tue, 25 Nov 2008, Dave Hansen wrote:

I think the end result was, if this file can only be changed by root, then
we do not need to worry about namespaces. This file is a privileged file
that can only be modified by root.

If someday we decide to let non admin users touch this file, then we would
need to care about this. This file may actually be modified in the future
by users, so this may become an issue.

This really has very little to do with root vs non-root users. In fact,
we're working towards having cases where we have many "root" users, even
those inside namespaces. It is also quite possible for a normal root
user to fork into a new pid namespace. In that case, root simply won't
be able to use this feature because something like:

echo $$ /debugfs/tracing/set_ftrace_pid

just won't work. Let's look at a bit of the code.

+static void ftrace_pid_func(unsigned long ip, unsigned long parent_ip)
+{
+ if (current->pid != ftrace_pid_trace)
+ return;
+
+ ftrace_pid_function(ip, parent_ip);
+}

One thing this doesn't deal with is pid wraparound. Does that matter?

Should not. This is just a way to trace a particular process. Currently
it traces all processes. If we wrap, then we trace the process with the
new pid. This should not be an issue.


If you want to fix this a bit, instead of saving off the pid_t in
ftrace_pid_trace, you should save a 'struct pid'. You can get the
'struct pid' for a particular task by doing a find_get_pid(pid_t). You
can then compare that pid_t to current by doing a
pid_task(struct_pid_that_i_saved, PIDTYPE_PID). That will also protect
against pid wraparound.

The find_get_pid() is handy because it will do the pid_t lookup in the
context of the current task's pid namespace, which is what you want, I
think.

Nope, we can not call that in this context. ftrace_pid_func is called
directly from mcount, without any protection.

struct pid *find_get_pid(pid_t nr)
{
struct pid *pid;

rcu_read_lock();
pid = get_pid(find_vpid(nr));
rcu_read_unlock();

return pid;
}
EXPORT_SYMBOL_GPL(find_get_pid);

This means find_get_pid will call mcount which will call ftrace_pid_func,
and back again. This can also happen with rcu_read_{un}lock() and
get_pid() and find_vpid().

We can not do anything special here.

-- Steve

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages

  • [PATCH 13/20] Allow cloning of new namespace
    ... When cloneis invoked with CLONE_NEWPID, create a new pid namespace ... The size of 'struct pid' is dependent on the the number of pid namespaces ... as necessary and use the pid cache to allocate 'struct pids' for that depth. ...
    (Linux-Kernel)
  • Re: [patch -mm] update mq_notify to use a struct pid
    ... efforts are less likely to duplicate work, ... Filling in a struct pid through sysctl is extremely ugly at the ... That's what i imagined also but we need a bit more of the pid namespace ...
    (Linux-Kernel)
  • Re: [patch -mm] update mq_notify to use a struct pid
    ... efforts are less likely to duplicate work, ... pid value depending on the pid namespace they are being received? ... to add a struct pid to struct sigqueue. ... That's what i imagined also but we need a bit more of the pid namespace ...
    (Linux-Kernel)
  • Re: [PATCH 1/3] ftrace: add function tracing to single thread
    ... What happens if the same pid exists in two or more pid namespaces? ... I think the end result was, if this file can only be changed by root, then ... 'struct pid' for a particular task by doing a find_get_pid. ... context of the current task's pid namespace, which is what you want, I ...
    (Linux-Kernel)
  • Re: exiting chroot()
    ... > Only root can ptrace another process. ... ## Args: ebx -> status ... # ptrace_attach (pid) failed ...
    (comp.unix.programmer)