Re: [PATCH 2/4] autofs4 - track uid and gid of last mount requester

Quoting Serge E. Hallyn (serue@xxxxxxxxxx):
Quoting Ian Kent (raven@xxxxxxxxxx):

On Fri, 2008-08-08 at 09:58 -0500, Serge E. Hallyn wrote:
Quoting Ian Kent (raven@xxxxxxxxxx):

On Fri, 2008-08-08 at 11:48 +0800, Ian Kent wrote:

Please remind me again why autofs's use of current->uid and
current->gid is not busted in the presence of PID namespaces, where
these things are no longer system-wide unique?

I actually don't see what the autofs4_waitq->pid is used for. It's
copied from current into wq->pid at autofs4_wait, and into a packet to
send to userspace (I assume) at autofs4_notify_daemon.

So as long as a daemon can serve multiple pid namespaces (which
doubtless it can), the pid could be confusing (or erroneous) for the
daemon.

Your point is well taken.

The pid is used purely for logging purposes to aid in debugging in user
space. I'm not sure it is worth worrying about it too much as the daemon
has no business interfering with user space processes it is not the
owner of.


If I'm remotely right about how the pid is being used, then the thing to
do would be to
1. store the daemon's pid namespace (would that belong in
the autofs_sb_info?)

Yep.

2. store the task_pid(current) in the waitqueue
3. retrieve the pid_t for the waiting task in the daemon's
pid namespace, and put that into the packet at
autofs4_notify_daemon.

I realize this patch was about the *uids*, but the pids seem more
urgent.

OK, I get it.
I'll have a go at doing this for completeness.

On second thoughts I'm not sure about this.

The pid that is logged needs to relate to a process in the name space of
the one that caused the mount to be done.

For example, suppose a GUI user finds mounts never expiring, then we get
a debug log to try and identify the culprit. So the pid should
correspond to a process that the user sees (So I guess in the namespace
of that user).

This is the only reason I added the pid to the request packet in the
first place.

Please correct me if my understanding of this is not right.

It's not wrong, but we just have to think through which value is the
most useful.

Any process executing clone(CLONE_NEWPID) (with CAP_SYS_ADMIN) can start
an application in a new pid namespace. So imagine the user at the
desktop clicking some button which runs an application in a new pid
namespace. Now if the user starts an xterm and runs ps -ef, the pid
values he'll see for the tasks in that new namespace will not be the
same as those which the application sees for itself, and not the same as
those which, right now, autofs would report.

For instance, if I start a shell in a new pid namespace, then within the
new pid namespace ps -ef gives me:

sh-3.2# ps -ef
UID PID PPID C STIME TTY TIME CMD
root 1 0 0 10:54 pts/1 00:00:00 /bin/sh
root 5 1 0 10:54 pts/1 00:00:00 /bin/sleep 100
root 6 1 0 10:54 pts/1 00:00:00 ps -ef

but from another shell as the same user, partial output of ps -ef
gives me:

root 2877 2876 0 10:54 pts/1 00:00:00 /bin/sh
root 2881 2877 0 10:54 pts/1 00:00:00 /bin/sleep 100

And so what we're trying to decide is whether autofs should send
pid 5 or pid 2881 for a message about the "/bin/sleep 100" task.

In fact, if the user clicks that button twice, chances are both
instances of the application will have the same pid values for each
process in the application. So now if autofs sends a message to the
user about the application, the user cannot tell which process is at
fault.

Autofs will be sending the user some message about 'process 5'. The
user won't know whether it means "the real" pid 5, [watchdog/0],
pid 5 in the first instance of the application, or pid 5 in the
second instance.

Now it's true that the user's xterm may still be in a different
(descendent) pidns of the autofs daemon. But we can't expect
the autofs daemon to do pid_t translation for the user, so I
think what we have to aim for is making sure that the values
reported are unique within the pidns of the autofs daemon. And
that means sending back either the pid values in the autofs
daemon's pid namespace, or using the top-level pid_ts, that is,
the pid values in the init namespace, which will be unique
on the whole system.

Sorry this turned out long-winded, I hope it makes sense.
And if I'm just showing a misunderstanding of what you're doing,
please do correct me :)

Yes, it's a bit tricky.

In reality this information is only logged when debug logging is enabled
and generally is only used by myself or others that maintain autofs. But
getting sensible logging is important so it's worth sorting this out.

I think it would be best to use the pid of the highest view namespace
which I think is the gist of what you said in the beginning. Then (at
some future time), if there was a user space API, the daemon could
report additional pid information related to subordinate pid namespaces.
I am assuming that, to be useful, an autofs daemon that is able to serve
multiple namespaces would be higher up in the tree. But the forgoing
description sounds like there's not a necessarily a hierarchic structure
to pid namespaces?

It is a simple tree, but of course if you have 3 autofs daemons in
separate containers, and one on the main host, then the pid namespaces for
each of the 3 container autofs daemons will be siblings, so they won't
have meaningful translations for each others' pids, while pids in
each container will have meaningful translations in the host system's
pid namespace (the init_pid_ns).

The other issue that comes up is, after storing (a reference to) the
daemon namespace in the super block info struct a "kill -9" on the
daemon would render the namespace invalid. At the moment, when this
happens the write on the kernel pipe fails causing the autofs mount to
become catatonic, but for the namespace aware case the namespace is now
invalid so we won't get that far. I could make the mount catatonic when

I figured you would grab a reference to the pid namespace, so it
wouldn't go away until you released the superblock or registered a new
daemon for it.

I detect that the namespace has become invalid but I'm not sure how to
check it. Is there a way I can to do this? Would there be any issues
with namespace (pid) reuse for such a check?

I'm not sure what you mean - but struct pid is never reused so long
as you have a reference to one. So you could store
get_pid(task_pid(autofs_daemon)) and check whether the autofs daemon's
pid has changed that way, I suppose. But grabbing a struct pid reference
does not pin the pid_ns iirc.

Maybe you're right about just getting the top-level pid_nr.

After doing a quick test with your git tree, I find that I was wrong,
and task->pid is the global pid of the process, not the pid in its own
pid namespace.

So currently autofs sends a process id in the init_pid_ns. This may
be meaningless in the autofs daemon's pid namespace, but since the
purpose is just for logging/debugging, having a global pid, which
uniquely identifies any task on the system, seems correct.

So in terms of pids no change is needed IMO.

thanks,
-serge
--
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

  • Re: [PATCH 2/2] Replace pid_t in autofs with struct pid reference
    ... Yes this is data being sent to a userspace daemon (Ian pls ... Otherwise we could get the wrong pid when we have ... At the moment autofs only uses the packet->pid for logging purposes. ... This solves an age old problem of not knowing who is causing mount ...
    (Linux-Kernel)
  • Re: [PATCH 2/4] autofs4 - track uid and gid of last mount requester
    ... So as long as a daemon can serve multiple pid namespaces (which ... pid namespace, and put that into the packet at ... And so what we're trying to decide is whether autofs should send ...
    (Linux-Kernel)
  • Re: [PATCH] proc connector: add event for process becoming session leader
    ... While a daemon will normally do this as part of the process of becoming ... the special PIDTYPE_SID pid is set. ... in the initial pid namespace -- not a good solution given Scott's example ... Document the process events connector user/kernel interface. ...
    (Linux-Kernel)
  • Re: [PATCH 2/4] autofs4 - track uid and gid of last mount requester
    ... So as long as a daemon can serve multiple pid namespaces (which ... pid namespace, and put that into the packet at ... And so what we're trying to decide is whether autofs should send ...
    (Linux-Kernel)
  • Re: [RFC][v8][PATCH 0/10] Implement clone3() system call
    ... I can argue exactly the same thing, the main motivation for your new syscall is to make it possible to restart a process tree for a checkpoint / restart and this is orthogonal with adding extended clone flags:) ... These 2 features allows to do what *we* need, that is recreate a process tree and the bonus with this approach is the ability to inject a process into a namespace, something asked by several people, eg. debug with gdb an application running into another pid namespace. ... system container because the container is killed by killing the first ...
    (Linux-Kernel)