public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH]: Revert 2.6.36 chroot ttyname regression
@ 2010-12-05  5:11 Eric W. Biederman
  2010-12-05 18:06 ` Linus Torvalds
  0 siblings, 1 reply; 5+ messages in thread
From: Eric W. Biederman @ 2010-12-05  5:11 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, Miklos Szeredi, Al Viro, stable


As of 2.6.36 ttyname does not work in a chroot.  It has already
been reported that screen breaks, and for me this breaks an automated
distribution testsuite, that I need to preserve the ability to run
the existing binaries on for several more years.  glibc 2.11.3 which
has a fix for this is not an option.

The root cause of this breakage is:
commit 8df9d1a4142311c084ffeeacb67cd34d190eff74
Author: Miklos Szeredi <mszeredi@suse.cz>
Date:   Tue Aug 10 11:41:41 2010 +0200

    vfs: show unreachable paths in getcwd and proc
    
    Prepend "(unreachable)" to path strings if the path is not reachable
    from the current root.
    
    Two places updated are
     - the return string from getcwd()
     - and symlinks under /proc/$PID.
    
    Other uses of d_path() are left unchanged (we know that some old
    software crashes if /proc/mounts is changed).
    
    Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
    Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>


So remove the nice sounding, but ultimately ill advised change to how
/proc/fd symlinks work.

Index: linux-2.6.37-rc4.x86_64/fs/proc/base.c
===================================================================
--- linux-2.6.37-rc4.x86_64.orig/fs/proc/base.c
+++ linux-2.6.37-rc4.x86_64/fs/proc/base.c
@@ -1574,7 +1574,7 @@ static int do_proc_readlink(struct path 
 	if (!tmp)
 		return -ENOMEM;
 
-	pathname = d_path_with_unreachable(path, tmp, PAGE_SIZE);
+	pathname = d_path(path, tmp, PAGE_SIZE);
 	len = PTR_ERR(pathname);
 	if (IS_ERR(pathname))
 		goto out;

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH]: Revert 2.6.36 chroot ttyname regression
  2010-12-05  5:11 Eric W. Biederman
@ 2010-12-05 18:06 ` Linus Torvalds
  2010-12-06  0:01   ` Eric W. Biederman
  0 siblings, 1 reply; 5+ messages in thread
From: Linus Torvalds @ 2010-12-05 18:06 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: linux-kernel, Miklos Szeredi, Al Viro, stable

On Sat, Dec 4, 2010 at 9:11 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
>
> As of 2.6.36 ttyname does not work in a chroot.

I'll happily revert, but even for trivial stuff like this I'd like to
get the sign-off, just so that we always have it.

                    Linus

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH]: Revert 2.6.36 chroot ttyname regression
@ 2010-12-05 23:51 Eric W. Biederman
  2010-12-06 10:00 ` Miklos Szeredi
  0 siblings, 1 reply; 5+ messages in thread
From: Eric W. Biederman @ 2010-12-05 23:51 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, Miklos Szeredi, Al Viro, stable


As of 2.6.36 ttyname does not work in a chroot.  It has already
been reported that screen breaks, and for me this breaks an automated
distribution testsuite, that I need to preserve the ability to run
the existing binaries on for several more years.  glibc 2.11.3 which
has a fix for this is not an option.

The root cause of this breakage is:
commit 8df9d1a4142311c084ffeeacb67cd34d190eff74
Author: Miklos Szeredi <mszeredi@suse.cz>
Date:   Tue Aug 10 11:41:41 2010 +0200

    vfs: show unreachable paths in getcwd and proc
    
    Prepend "(unreachable)" to path strings if the path is not reachable
    from the current root.
    
    Two places updated are
     - the return string from getcwd()
     - and symlinks under /proc/$PID.
    
    Other uses of d_path() are left unchanged (we know that some old
    software crashes if /proc/mounts is changed).
    
    Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
    Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>


So remove the nice sounding, but ultimately ill advised change to how
/proc/fd symlinks work.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

---

Index: linux-2.6.37-rc4.x86_64/fs/proc/base.c
===================================================================
--- linux-2.6.37-rc4.x86_64.orig/fs/proc/base.c
+++ linux-2.6.37-rc4.x86_64/fs/proc/base.c
@@ -1574,7 +1574,7 @@ static int do_proc_readlink(struct path 
 	if (!tmp)
 		return -ENOMEM;
 
-	pathname = d_path_with_unreachable(path, tmp, PAGE_SIZE);
+	pathname = d_path(path, tmp, PAGE_SIZE);
 	len = PTR_ERR(pathname);
 	if (IS_ERR(pathname))
 		goto out;

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH]: Revert 2.6.36 chroot ttyname regression
  2010-12-05 18:06 ` Linus Torvalds
@ 2010-12-06  0:01   ` Eric W. Biederman
  0 siblings, 0 replies; 5+ messages in thread
From: Eric W. Biederman @ 2010-12-06  0:01 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, Miklos Szeredi, Al Viro, stable

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Sat, Dec 4, 2010 at 9:11 PM, Eric W. Biederman <ebiederm@xmission.com> wrote:
>>
>> As of 2.6.36 ttyname does not work in a chroot.
>
> I'll happily revert, but even for trivial stuff like this I'd like to
> get the sign-off, just so that we always have it.

No problem, I just forgot.  The patch is resent.

Eric

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH]: Revert 2.6.36 chroot ttyname regression
  2010-12-05 23:51 [PATCH]: Revert 2.6.36 chroot ttyname regression Eric W. Biederman
@ 2010-12-06 10:00 ` Miklos Szeredi
  0 siblings, 0 replies; 5+ messages in thread
From: Miklos Szeredi @ 2010-12-06 10:00 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Linus Torvalds, linux-kernel, Al Viro, stable

On Sun, 2010-12-05 at 15:51 -0800, Eric W. Biederman wrote:
> As of 2.6.36 ttyname does not work in a chroot.  It has already
> been reported that screen breaks, and for me this breaks an automated
> distribution testsuite, that I need to preserve the ability to run
> the existing binaries on for several more years.  glibc 2.11.3 which
> has a fix for this is not an option.
> 
> The root cause of this breakage is:
> commit 8df9d1a4142311c084ffeeacb67cd34d190eff74
> Author: Miklos Szeredi <mszeredi@suse.cz>
> Date:   Tue Aug 10 11:41:41 2010 +0200
> 
>     vfs: show unreachable paths in getcwd and proc
>     
>     Prepend "(unreachable)" to path strings if the path is not reachable
>     from the current root.
>     
>     Two places updated are
>      - the return string from getcwd()
>      - and symlinks under /proc/$PID.
>     
>     Other uses of d_path() are left unchanged (we know that some old
>     software crashes if /proc/mounts is changed).
>     
>     Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
>     Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> 
> 
> So remove the nice sounding, but ultimately ill advised change to how
> /proc/fd symlinks work.

I didn't anticipate this problem, and reverting is probably the right
thing to do here.  But the fact remains: proc symlinks remain a badly
defined and, as a consequence, badly used interface.

Userspace assumes that these symlinks, when doing readlink on them, will
yield a valid absolute path that points to the same file (as did ttyname
in previous glibc's).  This is a false assumption because the file may
not be reachable due to it being unlinked, under a chroot, in a
different mount namespace, or on a detached mount, etc...

If the file is unlinked, we'll have "/path/to/old/name (deleted)" which
is an especially bad since it cannot be distinguished from an existing
file called "name (deleted)".

Do we want to do anything with this or should we just leave it broken?

One way to fix the "(unreachable)" thing without breaking ttyname() is
to do a forward pass on unreachable paths, checking whether the exact
same file is indeed reachable under the current root.  Not prepending
"(unreachable)" is defensible in this case because, even though the
dentry/vfsmount pair for the open file is unreachable from the current
root, the file itself *is* reachable under the same name.

Thoughts?

Thanks,
Miklos

> 
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> 
> ---
> 
> Index: linux-2.6.37-rc4.x86_64/fs/proc/base.c
> ===================================================================
> --- linux-2.6.37-rc4.x86_64.orig/fs/proc/base.c
> +++ linux-2.6.37-rc4.x86_64/fs/proc/base.c
> @@ -1574,7 +1574,7 @@ static int do_proc_readlink(struct path 
>  	if (!tmp)
>  		return -ENOMEM;
>  
> -	pathname = d_path_with_unreachable(path, tmp, PAGE_SIZE);
> +	pathname = d_path(path, tmp, PAGE_SIZE);
>  	len = PTR_ERR(pathname);
>  	if (IS_ERR(pathname))
>  		goto out;



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-12-06 10:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-05 23:51 [PATCH]: Revert 2.6.36 chroot ttyname regression Eric W. Biederman
2010-12-06 10:00 ` Miklos Szeredi
  -- strict thread matches above, loose matches on Subject: below --
2010-12-05  5:11 Eric W. Biederman
2010-12-05 18:06 ` Linus Torvalds
2010-12-06  0:01   ` Eric W. Biederman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox