2007-01-31 23:40:33

by Guillaume Chazarain

[permalink] [raw]
Subject: [PATCH] procfs: Fix listing of /proc/NOT_A_TGID/task

Hi,

I think this is 2.6.20 material, if it gets appropriately reviewed ;-)

Thanks.

--
Guillaume


Attachments:
procfs-fix-task-on-non-leader.patch (1.26 kB)

2007-02-01 00:49:34

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH] procfs: Fix listing of /proc/NOT_A_TGID/task

Guillaume Chazarain <[email protected]> writes:

> Hi,
>
> I think this is 2.6.20 material, if it gets appropriately reviewed ;-)

Look good to me.

Lookup doesn't have this problem because it only tests for tgid.
I'm not certain at the moment if we need the rcu_read_lock,
and pid_alive checks, but at worst they are overkill.

> Thanks.
>
> --
> Guillaume
>
> Listing /proc/PID/task were PID is not a TGID
> should not result in duplicated entries.
>
> [g ~]$ pidof thunderbird-bin
> 2751
> [g ~]$ ls /proc/2751/task
> 2751 2770 2771 2824 2826 2834 2835 2851 2853
> [g ~]$ ls /proc/2770/task
> 2751 2770 2771 2824 2826 2834 2835 2851 2853
> 2770 2771 2824 2826 2834 2835 2851 2853
> [g ~]$
>
> Signed-off-by: Guillaume Chazarain <[email protected]>
> ---
>
> fs/proc/base.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff -r f99a8b402753 fs/proc/base.c
> --- a/fs/proc/base.c Wed Jan 31 05:00:31 2007 +0000
> +++ b/fs/proc/base.c Thu Feb 01 00:25:33 2007 +0100
> @@ -2328,13 +2328,23 @@ static int proc_task_readdir(struct file
> {
> struct dentry *dentry = filp->f_path.dentry;
> struct inode *inode = dentry->d_inode;
> - struct task_struct *leader = get_proc_task(inode);
> + struct task_struct *leader = NULL;
> struct task_struct *task;
> int retval = -ENOENT;
> ino_t ino;
> int tid;
> unsigned long pos = filp->f_pos; /* avoiding "long long" filp->f_pos */
>
> + task = get_proc_task(inode);
> + if (!task)
> + goto out_no_task;
> + rcu_read_lock();
> + if (pid_alive(task)) {
> + leader = task->group_leader;
> + get_task_struct(leader);
> + }
> + rcu_read_unlock();
> + put_task_struct(task);
> if (!leader)
> goto out_no_task;
> retval = 0;