Re: [PATCH 2.6.13.1] Patch for invisible threads

Linus Torvalds
2005-09-13T14:54:12Z

On Tue, 13 Sep 2005, Sripathi Kodi wrote:
>
> Thanks and regards,
> Sripathi.
>
> Signed-off-by: Sripathi Kodi <sripathik@in.ibm.com>
>
> --- linux-2.6.13.1/kernel/exit.c 2005-09-13 15:39:48.738542872 -0500
> +++ /home/sripathi/17794/patch_2.6.13.1/exit.c 2005-09-13 15:39:27.367791720
> -0500
> @@ -463,9 +463,13 @@ static inline void __exit_fs(struct task
> struct fs_struct * fs = tsk->fs;
>
> if (fs) {
> - task_lock(tsk);
> - tsk->fs = NULL;
> - task_unlock(tsk);
> + /* If tsk is thread group leader and if group still has alive
> + * threads, those threads may use tsk->fs */
> + if (!thread_group_leader(tsk) || !atomic_read(&tsk->signal->live)) {
> + task_lock(tsk);
> + tsk->fs = NULL;
> + task_unlock(tsk);
> + }
> __put_fs_struct(fs);
> }
> }

This really is wrong. You "put" the fs without clearing it in that thread,
which means that now the reference counts no longer match the number of
pointers to it. This will inevitably result in using stale fs pointers in
/proc at some point. Not good. In fact, I almost guarantee that you can do
that by just having a thread group which doesn't share it's file
descriptors (which is possible, even though no _nice_ program does it.
Think DoS/security attack).

The sub-threads have a "->fs" of their own, and they'll happily continue
to use their own versions.

So this patch is _wrong_.

I think the problem is "proc_check_root()", which just refuses to do a lot
of things without a fs. Many of those things are unnecessary, afaik - we
should allow it. But allowing it means that some other paths may need more
checking..

So you can _try_ to just make proc_check_root() return 0 when
proc_root_link() returns an error...

Linus