Re: [PATCH 2.6.13.1] Patch for invisible threads

[Al Viro <viro@ftp.linux.org.uk>]

On Thu, Sep 15, 2005 at 07:54:56PM -0500, Sripathi Kodi wrote:
> proc_root_link and proc_task_root_link still have some duplicated code. I 
> could have split these functions further to avoid duplication completely, 
> but that would move incrementing and decrementing fs->lock to two different 
> functions, which I think will be confusing.
> 
> The other way of implementing this that I could think of was to have a flag 
> to indicate that the call is from ->permission path and pass it all along. 
> This will avoid having to change many existing functions, but it will 
> defeat the purpose of limiting this kludge code to ->permission path.
> 
> Please let me know how it is looking now.

Ugh...  Considering that all of that is _only_ for /proc/<pid>/task and
that proc_permission() is a couple of function calls, why bother with
proc_task_check_root() instead of just adding proc_task_permission() with

{
	struct dentry *root;
	struct vfsmount *vfsmnt;

	if (generic_permission(inode, mask, NULL) != 0)
		return -EACCES;

	/* or just open-code it here, for that matter */
	if (proc_task_root_link(inode, &root, &vfsmnt))
		return -ENOENT;

	return proc_check_chroot(root, vfsmnt);
}

for a body and leaving proc_permission() without any changes at all?
 
> Further, about actual permission checks that we are doing, can we say: "A 
> process should be able to see /proc/<pid>/task/* of another process only if 
> they both belong to same uid or reader is root"? But any such change will 
> change the behavior of commands like 'ps', right?

Right.  The real question is whether the current behaviour makes any sense.
I've no objections to your patch + modification above, but I really wonder
if we should keep current rules in that area.