* cwd unintentionally changed @ 2019-04-25 12:03 ` Yannic Schröder 2019-04-25 13:31 ` Peter Stephenson 0 siblings, 1 reply; 4+ messages in thread From: Yannic Schröder @ 2019-04-25 12:03 UTC (permalink / raw) To: zsh-workers; +Cc: Vincent Breitmoser, weichbrodt 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: cwd unintentionally changed 2019-04-25 12:03 ` cwd unintentionally changed Yannic Schröder @ 2019-04-25 13:31 ` Peter Stephenson 2019-04-26 6:12 ` Yannic Schröder 0 siblings, 1 reply; 4+ messages in thread From: Peter Stephenson @ 2019-04-25 13:31 UTC (permalink / raw) To: zsh-workers 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) { ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: cwd unintentionally changed 2019-04-25 13:31 ` Peter Stephenson @ 2019-04-26 6:12 ` Yannic Schröder 2019-04-26 8:30 ` Peter Stephenson 0 siblings, 1 reply; 4+ messages in thread From: Yannic Schröder @ 2019-04-26 6:12 UTC (permalink / raw) To: zsh-workers 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: cwd unintentionally changed 2019-04-26 6:12 ` Yannic Schröder @ 2019-04-26 8:30 ` Peter Stephenson 0 siblings, 0 replies; 4+ messages in thread From: Peter Stephenson @ 2019-04-26 8:30 UTC (permalink / raw) To: zsh-workers 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-04-26 8:31 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20190425120502epcas5p21b83b94136e0b2fae1002220c7d52656@epcas5p2.samsung.com> 2019-04-25 12:03 ` cwd unintentionally changed Yannic Schröder 2019-04-25 13:31 ` Peter Stephenson 2019-04-26 6:12 ` Yannic Schröder 2019-04-26 8:30 ` Peter Stephenson
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).