Re: cwd unintentionally changed

[Yannic Schröder <schroeder@xxxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxxx