Re: [PATCH] proc: Fix the threaded /proc/self.

[Ingo Molnar]

* Eric W. Biederman <[EMAIL PROTECTED]> wrote:

> We may be stuck with the current broken behavior for backwards 
> compatibility reasons but lets try fixing our ancient bug for the 
> 2.6.25 time frame and see if anyone screams.

to make sure i got you right - do you agree that this is a regression 
and that we need the patch below included in 2.6.24?

> Signed-off-by: Eric W. Biederman <[EMAIL PROTECTED]>

Acked-by: Ingo Molnar <[EMAIL PROTECTED]>

> ---
>  fs/proc/base.c |   12 ++--
>  1 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 34a1821..8502436 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2050,22 +2050,22 @@ static int proc_self_readlink(struct dentry *dentry, 
> char __user *buffer,
> int buflen)
>  {
>   struct pid_namespace *ns = dentry->d_sb->s_fs_info;
> - pid_t tgid = task_tgid_nr_ns(current, ns);
> + pid_t pid = task_pid_nr_ns(current, ns);
>   char tmp[PROC_NUMBUF];
> - if (!tgid)
> + if (!pid)
>   return -ENOENT;
> - sprintf(tmp, "%d", tgid);
> + sprintf(tmp, "%d", pid);
>   return vfs_readlink(dentry,buffer,buflen,tmp);
>  }
>  
>  static void *proc_self_follow_link(struct dentry *dentry, struct nameidata 
> *nd)
>  {
>   struct pid_namespace *ns = dentry->d_sb->s_fs_info;
> - pid_t tgid = task_tgid_nr_ns(current, ns);
> + pid_t pid = task_pid_nr_ns(current, ns);
>   char tmp[PROC_NUMBUF];
> - if (!tgid)
> + if (!pid)
>   return ERR_PTR(-ENOENT);
> - sprintf(tmp, "%d", task_tgid_nr_ns(current, ns));
> + sprintf(tmp, "%d", pid);
>   return ERR_PTR(vfs_follow_link(nd,tmp));
>  }
>  
> -- 
> 1.5.3.rc6.17.g1911

Ingo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] proc: Fix the threaded /proc/self.

[Guillaume Chazarain]

Hello Eric,

This fills a need I had to get the current TID in a Java program,
so I'm very interested in this change. OTOH, how will someone
not reading LKML discover that the current TID is now in
/proc/self and that it was not always the case?

I would put my 2 cents in /proc/self/task/self, this way TGID are
always in /proc and TID in /proc/TGID/task.

-- 
Guillaume
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] proc: Fix the threaded /proc/self.

[Eric W. Biederman]

Long ago when the CLONE_THREAD support first went it someone
thought it would be wise to point /proc/self at /proc/
instead of /proc/.

Given that /proc/ can return information about a very different
task (if enough things have been unshared) then our current process
/proc/ seems blatantly wrong.  So far I have yet to think up
an example where the current behavior would be advantageous, and
I can see several places where it is seriously non-intuitive.

We may be stuck with the current broken behavior for backwards
compatibility reasons but lets try fixing our ancient bug for the
2.6.25 time frame and see if anyone screams.

Signed-off-by: Eric W. Biederman <[EMAIL PROTECTED]>
---
 fs/proc/base.c |   12 ++--
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 34a1821..8502436 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2050,22 +2050,22 @@ static int proc_self_readlink(struct dentry *dentry, 
char __user *buffer,
  int buflen)
 {
struct pid_namespace *ns = dentry->d_sb->s_fs_info;
-   pid_t tgid = task_tgid_nr_ns(current, ns);
+   pid_t pid = task_pid_nr_ns(current, ns);
char tmp[PROC_NUMBUF];
-   if (!tgid)
+   if (!pid)
return -ENOENT;
-   sprintf(tmp, "%d", tgid);
+   sprintf(tmp, "%d", pid);
return vfs_readlink(dentry,buffer,buflen,tmp);
 }
 
 static void *proc_self_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
struct pid_namespace *ns = dentry->d_sb->s_fs_info;
-   pid_t tgid = task_tgid_nr_ns(current, ns);
+   pid_t pid = task_pid_nr_ns(current, ns);
char tmp[PROC_NUMBUF];
-   if (!tgid)
+   if (!pid)
return ERR_PTR(-ENOENT);
-   sprintf(tmp, "%d", task_tgid_nr_ns(current, ns));
+   sprintf(tmp, "%d", pid);
return ERR_PTR(vfs_follow_link(nd,tmp));
 }
 
-- 
1.5.3.rc6.17.g1911

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] proc: Fix the threaded /proc/self.

[Eric W. Biederman]

Long ago when the CLONE_THREAD support first went it someone
thought it would be wise to point /proc/self at /proc/tgid
instead of /proc/pid.

Given that /proc/tgid can return information about a very different
task (if enough things have been unshared) then our current process
/proc/tgid seems blatantly wrong.  So far I have yet to think up
an example where the current behavior would be advantageous, and
I can see several places where it is seriously non-intuitive.

We may be stuck with the current broken behavior for backwards
compatibility reasons but lets try fixing our ancient bug for the
2.6.25 time frame and see if anyone screams.

Signed-off-by: Eric W. Biederman [EMAIL PROTECTED]
---
 fs/proc/base.c |   12 ++--
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 34a1821..8502436 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2050,22 +2050,22 @@ static int proc_self_readlink(struct dentry *dentry, 
char __user *buffer,
  int buflen)
 {
struct pid_namespace *ns = dentry-d_sb-s_fs_info;
-   pid_t tgid = task_tgid_nr_ns(current, ns);
+   pid_t pid = task_pid_nr_ns(current, ns);
char tmp[PROC_NUMBUF];
-   if (!tgid)
+   if (!pid)
return -ENOENT;
-   sprintf(tmp, %d, tgid);
+   sprintf(tmp, %d, pid);
return vfs_readlink(dentry,buffer,buflen,tmp);
 }
 
 static void *proc_self_follow_link(struct dentry *dentry, struct nameidata *nd)
 {
struct pid_namespace *ns = dentry-d_sb-s_fs_info;
-   pid_t tgid = task_tgid_nr_ns(current, ns);
+   pid_t pid = task_pid_nr_ns(current, ns);
char tmp[PROC_NUMBUF];
-   if (!tgid)
+   if (!pid)
return ERR_PTR(-ENOENT);
-   sprintf(tmp, %d, task_tgid_nr_ns(current, ns));
+   sprintf(tmp, %d, pid);
return ERR_PTR(vfs_follow_link(nd,tmp));
 }
 
-- 
1.5.3.rc6.17.g1911

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] proc: Fix the threaded /proc/self.

[Guillaume Chazarain]

Hello Eric,

This fills a need I had to get the current TID in a Java program,
so I'm very interested in this change. OTOH, how will someone
not reading LKML discover that the current TID is now in
/proc/self and that it was not always the case?

I would put my 2 cents in /proc/self/task/self, this way TGID are
always in /proc and TID in /proc/TGID/task.

-- 
Guillaume
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] proc: Fix the threaded /proc/self.

[Ingo Molnar]

* Eric W. Biederman [EMAIL PROTECTED] wrote:

 We may be stuck with the current broken behavior for backwards 
 compatibility reasons but lets try fixing our ancient bug for the 
 2.6.25 time frame and see if anyone screams.

to make sure i got you right - do you agree that this is a regression 
and that we need the patch below included in 2.6.24?

 Signed-off-by: Eric W. Biederman [EMAIL PROTECTED]

Acked-by: Ingo Molnar [EMAIL PROTECTED]

 ---
  fs/proc/base.c |   12 ++--
  1 files changed, 6 insertions(+), 6 deletions(-)
 
 diff --git a/fs/proc/base.c b/fs/proc/base.c
 index 34a1821..8502436 100644
 --- a/fs/proc/base.c
 +++ b/fs/proc/base.c
 @@ -2050,22 +2050,22 @@ static int proc_self_readlink(struct dentry *dentry, 
 char __user *buffer,
 int buflen)
  {
   struct pid_namespace *ns = dentry-d_sb-s_fs_info;
 - pid_t tgid = task_tgid_nr_ns(current, ns);
 + pid_t pid = task_pid_nr_ns(current, ns);
   char tmp[PROC_NUMBUF];
 - if (!tgid)
 + if (!pid)
   return -ENOENT;
 - sprintf(tmp, %d, tgid);
 + sprintf(tmp, %d, pid);
   return vfs_readlink(dentry,buffer,buflen,tmp);
  }
  
  static void *proc_self_follow_link(struct dentry *dentry, struct nameidata 
 *nd)
  {
   struct pid_namespace *ns = dentry-d_sb-s_fs_info;
 - pid_t tgid = task_tgid_nr_ns(current, ns);
 + pid_t pid = task_pid_nr_ns(current, ns);
   char tmp[PROC_NUMBUF];
 - if (!tgid)
 + if (!pid)
   return ERR_PTR(-ENOENT);
 - sprintf(tmp, %d, task_tgid_nr_ns(current, ns));
 + sprintf(tmp, %d, pid);
   return ERR_PTR(vfs_follow_link(nd,tmp));
  }
  
 -- 
 1.5.3.rc6.17.g1911

Ingo
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/