Re: Task server thread and task allocation/deallocation interfaces proposal

[Marcus Brinkmann]

At Wed, 09 Mar 2005 16:03:39 +0000,
Matthieu Lemerre <address@hidden> wrote:
> 
> Marcus Brinkmann <address@hidden> writes:
> >
> > I disagree.  We need task groups, so we can kill off child tasks which
> > did not register themselves with the proc server.  This is not only
> > important for sub-hurds like boot, or similar applications where a
> > process doesn't register child tasks with the proc server, but also to
> > clean up after a partial fork() or exec(), which otherwise could leave
> > behind unregistered tasks in a broken state.
> >
> > The task group ID can just be a protected word that can only be set by
> > a privileged server, for example proc, and which is inherited at task
> > creation.  proc could set this word to something as simple as the PID.
> >
> > Killing a task should kill all tasks with the same task group ID, too.
> > This ensures that no stranded unregistered tasks are left behind.
> 
> Shouldn't we instead pass a flag to tell if we want to destroy all
> tasks with that group ID, or just this one?  If I think of a
> particuliar application which create a new task and then wants to
> destroy it, it must contact the proc server just to change the task id
> of that task and make sure the task server won't destroy itself.

Yes, good point.  Total agreement.
 
> Eventually, tasks within the same group ID could be organized in a
> tree structure, and when upon task destruction the task server would
> kill all of its sons, recursively.  So killing the root of this tree
> would kill all tasks.

I considered this, but uhm, I am unsure.  I don't think it should be
the burden of proc (or anybody else) to figure out what the root of
the tree should be.  Consider you have three tasks in your ps output,
only one is registered with proc, the others have the same task group
id, but the one registered with proc is _not_ the one that is the root
of the tree.  If you want to kill all three you would then need to try
them in turns.

Or you can offer all three options: kill one task, kill whole task id
group, kill hierarchy.  But I question the usefulness of the last
feature: Who wants to kill all tasks and its (unregistered) children?
If you need to do that internally in your task id group, you can
manage the hierarchy yourself just as well according to your own
policies.

I think for our system code, we only need the task id group feature.
If I am missing something, let me know.

? It would also solve the problem below, without
> changing the exec protocol.  On the implementation, I don't think that
> it will make things more complicated, since I think that killing every
> tasks on a group ID already require that the task_t are linked
> together as a linked list.
> 
> I also wonder why is the group ID word useful.  Isn't links between
> task_t nodes sufficient for storing relationships between the tasks?

Certainly, if you never want to export the relationship, then just
keeping it internal is sufficient, and only indirect operations are
necessary (create new task group for this task, etc).  But I think it
is useful to see the relationship clearly in the output of the ps
program.  So, somehow we need to export the information which tasks
belong together.  You are right that this could be done in more
indirect ways than naming them.  It's worth considering.

> task_set_group_id could be replaced by
> 
> task_reparent (l4_thread_id_t task_server,
>                hurd_cap_handle_t parent_or_privileged_capability,
>                hurd_cap_handle_t task_to_reparent)
> 
> and the new parent of TASK_TO_REPARENT would be set to
> PARENT_OR_PRIVILEGED_CAPABILITY in case of it beeing a normal task
> control capability, or there would be no parent if
> PARENT_OR_PRIVILEGED_CAPABILITY is the proc control capability.
> (This may be a bad interface).

Actually, it's similar the same I came up with when writing the reply
to your last paragraph ;) I would phrase it a bit differently, though.
First, this is not about parent and child relationship (my previous
mails may have been misleading), so it should be task_set_group.
Also, I think your arguments are quite confused.  As I said earlier,
RPCs should always go to a task object (in this case,
task_to_reparent) and not to the "task server", which is not an
object.  What may be hitting you here is that I didn't write the
libhurd-cap-client library yet that wraps this up.

/* Set the task group of task TASK to the same as the one of task
   GROUP_SPECIFIER, unless GROUP_SPECIFIER is HURD_CAP_NULL, in which
   case a new group is created for this task.  */
task_set_group (task_t task, task_t group_specifier);

> Killing all tasks in a group ID also introduces the following problem:
> if a tasks create a new task and give it to a filesystem, the latter,
> if malicious, could kill both tasks (the passed one and the calling
> one).  To avoid that, we should complete the exec protocol like this:
> before giving a task control capability to the filesystem for a newly
> created task, we have to contact the proc server to change the group
> id of this task.

Not necessarily, if killing all tasks in the same group is a
privileged operation only allowed for the proc server.  But you have a
good point, it's something we have to consider and address somehow.
If we want to allow anybody to kill whole task groups based on their
task group relationship, we need to make sure that this doesn't
influence fork/exec in the way you describe.  We have to look in more
detail into it.  For now I am slightly in favor of permitting the
kill-all operation to proc only.

> > BTW, one important thing to consider is how suid exec will work.  If
> > we want proper accounting, suid tasks should be donated by the starter
> > to the filesystem, but the filesystem needs total control over it.  So
> > some revoke-and-reparent operation will be needed: revoke to remove
> > access by the original starter, and reparent to associate the donated
> > task to the filesystem doing the suid exec.  So, the task_set_group_id
> > should also be permitted for non-privileged users in the special case
> > that you want to set the group ID to the group ID of your own task
> > (rather than any arbitrary group ID).  Ie, you claim a task you have
> > control over (you have the task capability) as belonging to your own
> > task group (there are no security problems with this I think - there
> > _are_ security issues with donating your own tasks to somebody else's
> > task group).
> >
> 
> I think I found one of them ("a malicious filesystem could kill the
> calling task."), and maybe a way to avoid it (the tree structure), but
> I may miss many others.  Could you please explain a bit more on this?

Actually, you are far ahead of me - you found a potential security
problem where I saw none (mainly because I never really integrated
task groups into my design from ground up).

What I meant is something different: If you allow somebody to regroup
a task to some arbitrary other task group, somebody could always hide
their unregistered tasks into foreign task groups, for example the
task group of a long-living system task.  This is a mild security
problem because it means that your unregistered task is a bit more
hidden, but also because the other task just might not expect that
foreign tasks enter its group in some uncontrolled fashion.

Thanks,
Marcus