[pseudo][PATCH] pseudo_util: Fix resolving relative paths from "/"

[pseudo][PATCH] pseudo_util: Fix resolving relative paths from "/"

[Tomi Belan]

pseudo_fix_path() incorrectly assumed that 'base' never ends with a slash
because it's a canonical path. However, base can be "/" - a path which is
canonical and yet starts with a slash. This happens when pseudo_cwd is "/" or
when we're starting from a dirfd pointing to "/". The wrong result from
pseudo_fix_path() caused the database lookup to fail and made pseudo abort.

Signed-off-by: Tomi Belan <tomi.belan@gmail.com>
---
Hi. Here is my third patch for 'pseudo'. I included a test that demonstrates
the fix. Try "make test" with old and new pseudo_util.c to see the difference.
Run it with PSEUDO_DEBUG to see what happens and how it leads to an abort.

JFYI: The real fix is to check "baselen == 1". The "rootlen == 1" condition is
just for symmetry. Unlike pseudo_cwd and fd_path(), which can be "/", I think
pseudo_chroot will never be "/" because pseudo_client_chroot sets it to NULL
instead. So rootlen should never be 1. It's just to be safe.

 pseudo_util.c                   | 12 +++++++++---
 test/test-relative-from-root.sh | 14 ++++++++++++++
 2 files changed, 23 insertions(+), 3 deletions(-)
 create mode 100755 test/test-relative-from-root.sh

diff --git a/pseudo_util.c b/pseudo_util.c
index e8e9803..64636b7 100644
--- a/pseudo_util.c
+++ b/pseudo_util.c
@@ -792,9 +792,9 @@ static char *pathbufs[PATHBUFS] = { 0 };
 static int pathbuf = 0;
 
 /* Canonicalize path.  "base", if present, is an already-canonicalized
- * path of baselen characters, presumed not to end in a /.  path is
- * the new path to be canonicalized.  The tricky part is that path may
- * contain symlinks, which must be resolved.
+ * path of baselen characters, presumed not to end in a / unless it is
+ * just "/".  path is the new path to be canonicalized.  The tricky part
+ * is that path may contain symlinks, which must be resolved.
  */
 char *
 pseudo_fix_path(const char *base, const char *path, size_t rootlen, size_t baselen, size_t *lenp, int leave_last) {
@@ -808,6 +808,12 @@ pseudo_fix_path(const char *base, const char *path, size_t rootlen, size_t basel
 		pseudo_diag("can't fix empty path.\n");
 		return 0;
 	}
+	if (baselen == 1) {
+		baselen = 0;
+	}
+	if (rootlen == 1) {
+		rootlen = 0;
+	}
 	newpathlen = pseudo_path_max();
 	pathlen = strlen(path);
 	/* Crazy shell code (e.g. libtool) can pass in a command pipeline as a path which exceeds the max path
diff --git a/test/test-relative-from-root.sh b/test/test-relative-from-root.sh
new file mode 100755
index 0000000..e2c230e
--- /dev/null
+++ b/test/test-relative-from-root.sh
@@ -0,0 +1,14 @@
+#!/bin/bash
+# pseudo had a bug that made it abort() when looking up a relative path from
+# base "/", such as by openat(dirfd_of_root, "foo/bar") or when cwd is /. It
+# tried to look up base+"/"+path = "//foo/bar", which is wrong.
+
+set -e
+
+touch f1
+relative_pwd=${PWD#/}
+
+cd /
+cat "$relative_pwd/f1"
+
+rm "$relative_pwd/f1"
-- 
2.25.1

Re: [OE-core] [pseudo][PATCH] pseudo_util: Fix resolving relative paths from "/"

[Luca Ceresoli]

On Wed, 24 Aug 2022 16:51:36 +0000
"Tomi Belan" <tomi.belan@gmail.com> wrote:

> pseudo_fix_path() incorrectly assumed that 'base' never ends with a slash
> because it's a canonical path. However, base can be "/" - a path which is
> canonical and yet starts with a slash. This happens when pseudo_cwd is "/" or
                    ^^^^^^
starts -> ends?

> when we're starting from a dirfd pointing to "/". The wrong result from
> pseudo_fix_path() caused the database lookup to fail and made pseudo abort.
> 
> Signed-off-by: Tomi Belan <tomi.belan@gmail.com>

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [OE-core] [pseudo][PATCH] pseudo_util: Fix resolving relative paths from "/"

[Tomi Belan]

On Wed, Aug 24, 2022 at 10:40 PM Luca Ceresoli
<luca.ceresoli@bootlin.com> wrote:
> > canonical and yet starts with a slash. This happens when pseudo_cwd is "/" or
>                     ^^^^^^
> starts -> ends?

Oops, yes, of course. Good catch.

Should I send a v2 patch? :)
The corrected commit message line should be:
---
canonical and yet ends with a slash. This happens when pseudo_cwd is "/" or
---

Tomi

Re: [OE-core] [pseudo][PATCH] pseudo_util: Fix resolving relative paths from "/"

[Luca Ceresoli]

Hi Tomi,

On Wed, 24 Aug 2022 23:09:51 +0200
"Tomi Belan" <tomi.belan@gmail.com> wrote:

> On Wed, Aug 24, 2022 at 10:40 PM Luca Ceresoli
> <luca.ceresoli@bootlin.com> wrote:
> > > canonical and yet starts with a slash. This happens when pseudo_cwd is "/" or  
> >                     ^^^^^^
> > starts -> ends?  
> 
> Oops, yes, of course. Good catch.
> 
> Should I send a v2 patch? :)

No need. Richard already took your patch and fixed the message. It's
currently in the oe-core-next branch:
https://git.yoctoproject.org/pseudo/log/?h=oe-core-next

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com