Re: [PATCH 2.6.13.1] Patch for invisible threads

[Linus Torvalds]

On Tue, 13 Sep 2005, Sripathi Kodi wrote:
> 
> Thanks and regards,
> Sripathi.
> 
> Signed-off-by: Sripathi Kodi <sripathik@xxxxxxxxxx>
> 
> --- 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
-
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/