diff options
authorPeter Seebach <peter.seebach@windriver.com>2012-06-27 22:26:41 -0500
committerPeter Seebach <peter.seebach@windriver.com>2012-06-28 17:29:03 -0500
commit3b18bb84473e13d295cc421358a83bed41601b70 (patch)
parentee7f316398a694d908081d3713b7141a06ff5a55 (diff)
[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.
2 files changed, 10 insertions, 3 deletions
diff --git a/ChangeLog.txt b/ChangeLog.txt
index 7174ab1..702ae9e 100644
--- a/ChangeLog.txt
+++ b/ChangeLog.txt
@@ -1,3 +1,6 @@
+ * (seebs) Fix chroot coredump with long root path.
* (seebs) Update README about new upstream.
diff --git a/pseudo_util.c b/pseudo_util.c
index fd3236d..95844fa 100644
--- a/pseudo_util.c
+++ b/pseudo_util.c
@@ -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;