Re: [PATCH 2.6.13.1] Patch for invisible threads

Al Viro
2005-09-16T07:46:18Z

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.