Re: RFC [patch 13/34] PID Virtualization Define new task_pid api

[ebiederm@xmission.com (Eric W. Biederman)]

Herbert Poetzl <herbert@13thfloor.at> writes:

> anyway, if that would be the aim, it could be done
> much simpler by 'just' adding a v/upid field to the
> task struct and use that for everything userspace
> related (i.e. locating tasks, sending signals, etc)
> no need to change the current *pid entries at all

Yes and no.  Changing the current pid entries as opposed
to adding the kpid/upid separation is a slightly different
problem.  

In particular there are 4 uses we need to change.  
 - Printing the pid in debug messages.
 - Comparing pids (because we need to add a context comparison)
 - Sending signals/localing tasks.
 - Entering a code path that wants to do one of the above.

Printing the pid in debug messages seems to be confined to
performing the action with reference to a task_struct, and
is a completeness issue not a correctness issue.

Sending signals and locating tasks by pid is fairly
straight forward to change the interface of all affected
functions.  And thus forcing an audit of all callers,
recursively also works well.

Comparing pids is where I think things get sticky but arguably
that case is rare enough we may be able to catch all of the users
with a code audit.

The only change I would really advocate at the moment beyond
adding the kpid fields to the task struct is to rename
(pid, tgid, pgrp, session) to (upid, utgid, upgrp, usession)
so we catch and break the users.  This would catch flush into
the open all of the users that are doing weird things like comparing
pids and would leave any rare untested and unspotted case broken where
it will not compile.

Arguably that it is overkill to break all of the users to catch the
stragglers that we can't easily spot with a code review.  Likely I be
satisfied with not breaking the code until I found a straggler that
affects correctness that made it through a kernel code review. 

So far I have yet to see a version of the code that does not miss
important stragglers.  Which is why to be correct I suspect we need
to break all users.

Eric