cwd unintentionally changed

cwd unintentionally changed

[Yannic Schröder]

Hi *,

I got myself into a situation where zsh changed my working directory to
"/" without further notice. Unfortunately, the next command I issued was
"sudo rm -rf *", which did not end well with cwd being "/" :-(

My colleagues and me started to track down the bug. Our efforts are
documented here:
https://github.com/grml/grml-etc-core/issues/76

(We initially though it was a bug with grml zsh config.)

A minimal example that triggers the bug looks like this:

$ mkdir /tmp/tmp.0HnyRZ1iXv/
$ cd /tmp/tmp.0HnyRZ1iXv/
$ mkdir a
$ sudo zsh -f -c 'mount -t tmpfs tmpfs a; cd a; touch x; ls; umount
--lazy ../a; ls; foo=.; echo ${foo:a}; ls; realpath .; echo $PWD;'

It will output the following:
x                      # content of directory a (expected)
x                      # content of directory a (expected)
/                      # NOT expected
bin dev ...            # content of / (NOT expected)
/                      # NOT expected
/tmp/tmp.0HnyRZ1iXv/a  # expected

cwd silently changed to "/" while pwd does not know about this.

This looks like a very specific use case, but a change [0] about how
vcs_info gets the base directory for the repository also triggers this
bug. In my case when using Gnome which automatically mounts and unmounts
external drives (with the --lazy option).

After ending up in the source code for zgetcwd() and zgetdir() we
decided to hand the issue over to you :-)

Regards,
Yannic

[0]
https://github.com/zsh-users/zsh/commit/faa07d064bb2bd9cd9892a255a4b63811242b9fb
-- 
Yannic Schröder, M.Sc.

Technische Universität Braunschweig
Institut für Betriebssysteme und Rechnerverbund
Mühlenpfordtstr. 23
38106 Braunschweig

Fon: +49 (531) 391 - 3249
Fax: +49 (531) 391 - 5936
E-Mail: schroeder@ibr.cs.tu-bs.de

Re: cwd unintentionally changed

[Peter Stephenson]

On Thu, 2019-04-25 at 14:03 +0200, Yannic Schröder wrote:
> Hi *,
> 
> I got myself into a situation where zsh changed my working directory to
> "/" without further notice. Unfortunately, the next command I issued was
> "sudo rm -rf *", which did not end well with cwd being "/" :-(
> 
> My colleagues and me started to track down the bug. Our efforts are
> documented here:
> https://github.com/grml/grml-etc-core/issues/76
> 
> (We initially though it was a bug with grml zsh config.)
> 
> A minimal example that triggers the bug looks like this:
> 
> $ mkdir /tmp/tmp.0HnyRZ1iXv/
> $ cd /tmp/tmp.0HnyRZ1iXv/
> $ mkdir a
> $ sudo zsh -f -c 'mount -t tmpfs tmpfs a; cd a; touch x; ls; umount
> --lazy ../a; ls; foo=.; echo ${foo:a}; ls; realpath .; echo $PWD;'

Thanks, easy to follow.

That's certainly not pleasant as it's silent.

The source of the problem is in zgetdir().  We attempt to climb the
directory hierarchy until we get to /.  In this case we appear to get
there immediately.  Then, fatally, we call zchdir() to the place where we
think we are, but actually we aren't.

The patch below fixes up this case so we make quite sure we're in /.
This looks safe, but I'm not sure it's necessarily the limit of
what we can do to make things as safe as possible...

I'm not quite sure why we need to zchdir().  It seems to be a side
effect rather than a basic part of the call.  In particular I don't see
why it's a good idea in this case --- maybe we just add an argument
saying "not actually changing directory"?

Or should we be using getcwd() nowadays?  Presumably this is fine on most
modern systems?

pws

diff --git a/Src/compat.c b/Src/compat.c
index 7b5c4411c..7131d91a4 100644
--- a/Src/compat.c
+++ b/Src/compat.c
@@ -361,8 +361,18 @@ zgetdir(struct dirsav *d)
 	pino = sbuf.st_ino;
 	pdev = sbuf.st_dev;
 
-	/* If they're the same, we've reached the root directory. */
+	/* If they're the same, we've reached the root directory... */
 	if (ino == pino && dev == pdev) {
+	    /*
+	     * ...well, probably.  If this was an orphaned . after
+	     * an unmount, or something such, we could be in trouble...
+	     */
+	    if (stat("/", &sbuf) < 0 ||
+		sbuf.st_ino != ino ||
+		sbuf.st_dev != dev) {
+		zerr("Failed to get current directory: path invalid");
+		return NULL;
+	    }
 	    if (!buf[pos])
 		buf[--pos] = '/';
 	    if (d) {

Re: cwd unintentionally changed

[Yannic Schröder]

I applied the provided patch and indeed it stops cwd from changing.

I loaded a zshrc that uses vcs_info and it now prints

VCS_INFO_bydir_detect:9: Failed to get current directory: path invalid

with every new prompt after the umount (which might be confusing for
users). However, cwd stays intact.

I could live with this solution (maybe suppress the error in vcs_info).

Regards,
Yannic

On 25.04.19 15:31, Peter Stephenson wrote:
> On Thu, 2019-04-25 at 14:03 +0200, Yannic Schröder wrote:
>> Hi *,
>>  
>> I got myself into a situation where zsh changed my working directory to
>> "/" without further notice. Unfortunately, the next command I issued was
>> "sudo rm -rf *", which did not end well with cwd being "/" :-(
>>  
>> My colleagues and me started to track down the bug. Our efforts are
>> documented here:
>> https://github.com/grml/grml-etc-core/issues/76
>>  
>> (We initially though it was a bug with grml zsh config.)
>>  
>> A minimal example that triggers the bug looks like this:
>>  
>> $ mkdir /tmp/tmp.0HnyRZ1iXv/
>> $ cd /tmp/tmp.0HnyRZ1iXv/
>> $ mkdir a
>> $ sudo zsh -f -c 'mount -t tmpfs tmpfs a; cd a; touch x; ls; umount
>> --lazy ../a; ls; foo=.; echo ${foo:a}; ls; realpath .; echo $PWD;'
> 
> Thanks, easy to follow.
> 
> That's certainly not pleasant as it's silent.
> 
> The source of the problem is in zgetdir().  We attempt to climb the
> directory hierarchy until we get to /.  In this case we appear to get
> there immediately.  Then, fatally, we call zchdir() to the place where we
> think we are, but actually we aren't.
> 
> The patch below fixes up this case so we make quite sure we're in /.
> This looks safe, but I'm not sure it's necessarily the limit of
> what we can do to make things as safe as possible...
> 
> I'm not quite sure why we need to zchdir().  It seems to be a side
> effect rather than a basic part of the call.  In particular I don't see
> why it's a good idea in this case --- maybe we just add an argument
> saying "not actually changing directory"?
> 
> Or should we be using getcwd() nowadays?  Presumably this is fine on most
> modern systems?
> 
> pws
> 
> diff --git a/Src/compat.c b/Src/compat.c
> index 7b5c4411c..7131d91a4 100644
> --- a/Src/compat.c
> +++ b/Src/compat.c
> @@ -361,8 +361,18 @@ zgetdir(struct dirsav *d)
>  	pino = sbuf.st_ino;
>  	pdev = sbuf.st_dev;
>  
> -	/* If they're the same, we've reached the root directory. */
> +	/* If they're the same, we've reached the root directory... */
>  	if (ino == pino && dev == pdev) {
> +	    /*
> +	     * ...well, probably.  If this was an orphaned . after
> +	     * an unmount, or something such, we could be in trouble...
> +	     */
> +	    if (stat("/", &sbuf) < 0 ||
> +		sbuf.st_ino != ino ||
> +		sbuf.st_dev != dev) {
> +		zerr("Failed to get current directory: path invalid");
> +		return NULL;
> +	    }
>  	    if (!buf[pos])
>  		buf[--pos] = '/';
>  	    if (d) {
> 
-- 
Yannic Schröder, M.Sc.

Technische Universität Braunschweig
Institut für Betriebssysteme und Rechnerverbund
Mühlenpfordtstr. 23
38106 Braunschweig

Fon: +49 (531) 391 - 3249
Fax: +49 (531) 391 - 5936
E-Mail: schroeder@ibr.cs.tu-bs.de

Re: cwd unintentionally changed

[Peter Stephenson]

On Fri, 2019-04-26 at 08:12 +0200, Yannic Schröder wrote:
> I applied the provided patch and indeed it stops cwd from changing.
> 
> I loaded a zshrc that uses vcs_info and it now prints
> 
> VCS_INFO_bydir_detect:9: Failed to get current directory: path invalid
> 
> with every new prompt after the umount (which might be confusing for
> users). However, cwd stays intact.
> 
> I could live with this solution (maybe suppress the error in vcs_info).

Having an umount as part of the process is something of a special case:
ideally any workaround would be at that point.

pws