Mail Archives: djgpp-workers/1999/03/08/04:37:29
On Thu, 4 Mar 1999, Mark E. wrote:
> Below are my latest chroot patches. I'm hoping they are complete and
> correct.
Thanks!
Unfortunately, I'm afraid I need to be the Bad Guy again ;-)
(Will somebody else please review the code as well, at least to share
the blame, if not for other good reasons? ;-)
> + else if (*path != '/' && *path != '\\' && __djgpp_root_len > 0)
> + {
> + /* Relative paths are relative to the root, not the cwd. */
> + char path_buf[PATH_MAX+1];
> + path_buf[0] = '/';
> + strcpy(path_buf+1, path);
I don't see such interpretation in the Unix man pages. Relative file
names are allowed and are interpreted as usual: relative to cwd.
> + if (unix_emu)
> + {
> + /* If a root directory already exists, the new root directory
> + must be relative to the existing root directory.
> + If not, then reject the path. */
> + if (__djgpp_root_len > 0 && *buf != '/')
> + {
> + errno = EACCES;
> + return -1;
> + }
Nate tried on Linux and said that the argument passed to `chroot' is
always interpreted as relative to the current root. However, your
implementation calls `_fixpath' too early, and thus deviates from this
behavior.
For example, if the current root is "c:/foo" and `chroot' is called
with "/foo/bar/baz", then `chroot' will allow it (I think; I didn't
actually try this) because it generates "c:/foo/bar/baz" and then
removes "c:/foo" from it. But the Unix description says you should
append the argument to the current root, to get "c:/foo/foo/bar/baz",
and *then* see if that directory is valid.
I think the problem is that you call `_fixpath' too early in some
cases. How about the following logic:
- if the file name begins with a slash, append to current root, then
run the result through `_fixpath';
- else run the result through `_fixpath', then append to the current
root;
- now call `access' to see if the result exists and is a directory;
- if it is a valid directory, proceed with the rest of code.
The current implementation also has problems with ANSI name-space
pollution: since `chroot' is called from the startup code, it is
linked into every DJGPP program. Therefore, it cannot call any
non-ANSI functions whose names don't begin with an underscore. The
problems are with `chroot' itself, with `setenv', and with
`strnicmp'. `chroot' must be stubbed (see <libc/stubs.h>). I suggest
to use `stricmp' instead of `strnicmp' and `putenv' instead of
`setenv', since the replacements are already stubbed, and to add the
necessary code for the replacements to work.
Sorry I didn't catch the name-space pollution problems earlier. Did I
said that others should look at this code as well? ;-)
> + /* If the path is absolute and a root path is set,
> + then add the root path to the output. */
> + else if (path[0] == '/' && __djgpp_root_len > 0 )
> + {
> + char *root = __djgpp_root;
> + for ( ; *root; root++)
> + {
> + _farnspokeb (o++, *root);
> + space -= 1;
> + }
I think the "else if" clause above (this is from putpath.c) should
also catch backslashes, not only forward slashes.
Also, what about file names like "c:/foo" when the current root is
"c:/foo/bar"? I think `_put_path2' should fail such arguments when
the Unix emulation flag is set, otherwise ``chdir ("c:/foo")'' and
such likes will defeat the purpose of this feature. The simplest way
to do that is to prepend the current root to the argument, if the
latter starts with a drive letter.
The ``permissive'' mode doesn't need to enforce this, I think.
> + /* Succeeds because 'c:/djgpp/bin' is relative to 'c:/djgpp'.
> + Passing in '/bin' would have also worked. */
> + chroot("c:/djgpp/bin");
The current code correctly fails in this case, so the docs needs to be
changed.
> + @subheading Return Value
> +
> + Always returns zero for success, unless @var{file_handle} is less than
> + zero.
This is no longer true: `fchroot' always ignores the handle.
> + @subheading Portability
> +
> + @portability !ansi, !posix
I suggest to add a @portability-note to the effect that `fchroot' only
allows to return to the original root x:/; you can't save away several
previous values of the root (by saving the handles) and then return to
some of them.
> *** src/libc/crt0/c1root.c.orig Sun Feb 28 14:08:22 1999
> --- src/libc/crt0/c1root.c Thu Mar 4 12:46:52 1999
The new files, such as c1root.c and others, need to be added to the
list of files in the makefile's in their respective directories,
otherwise the library build procedure won't pick them up.
Thanks again for working on this.
- Raw text -