Permalink
Browse files

[Yocto #2639] Don't crash with really long chroot directories

The logic for whether to allocate space for the "base" path
in pseudo_fix_path recognized that you don't need it when the
path you're evaluating starts with a slash.

This is great, except:
1.  It's not actually true, if rootlen isn't 0.
2.  The decision of whether or not to copy over the base
path didn't check for this, so it would happen anyway.

The net result is, if you had a path in excess of 256 characters as
a base (say, a chroot directory), and you tried to evaluate a path
starting with a slash (say, /etc/shadow), pseudo would allocate enough
space for the path, but not for the base path, and then copy the
base path into it anyway. The rounding up to multiples of 256 isn't
enough to save us in this case.

Solution:
1.  Make the logic for the base path copy match the allocation logic.
2.  Use (path[0] != '/' || rootlen) as the second part of the test,
because if there's a non-zero rootlen, we're in a chroot and MUST
preserve at least some of the path.

This could maybe be smarter (what if we only allocated space for
rootlen in that case?) except that in reality, it's very very
often the case that baselen == rootlen, and it's not as though we
want MORE complexity.
  • Loading branch information...
wr-seebs committed Jun 28, 2012
1 parent ee7f316 commit 3b18bb84473e13d295cc421358a83bed41601b70
Showing with 10 additions and 3 deletions.
  1. +3 −0 ChangeLog.txt
  2. +7 −3 pseudo_util.c
View
@@ -1,3 +1,6 @@
+2012-06-27:
+ * (seebs) Fix chroot coredump with long root path.
+
2012-04-30:
* (seebs) Update README about new upstream.
View
@@ -597,9 +597,13 @@ pseudo_fix_path(const char *base, const char *path, size_t rootlen, size_t basel
}
pathlen = strlen(path);
newpathlen = pathlen;
- if (baselen && path[0] != '/') {
+ /* If the path starts with /, we don't care about base, UNLESS
+ * rootlen is non-zero, in which case we're doing a chroot thing
+ * and will actually need to append some components.
+ */
+ if (baselen && (path[0] != '/' || rootlen)) {
newpathlen += baselen + 2;
- }
+ }
/* allow a bit of slush. overallocating a bit won't
* hurt. rounding to 256's in the hopes that it makes life
* easier for the library.
@@ -611,7 +615,7 @@ pseudo_fix_path(const char *base, const char *path, size_t rootlen, size_t basel
return 0;
}
current = newpath;
- if (baselen) {
+ if (baselen && (path[0] != '/' || rootlen)) {
memcpy(current, base, baselen);
current += baselen;
}

0 comments on commit 3b18bb8

Please sign in to comment.