DragonFly bugs List (threaded) for 2005-01
Re: Looks like split of execve(2) syscall created bugs
Matthew Dillon wrote:
:I am working on eliminating stackgap in FreeBSD's linuxlator following
:DF's footsteps and fond that there is potential bug has been introduced
:into execve(). The problem is that DF now first checks if argv is
:NULL, then replaces it with pathname and then proceeds with scanning
:other arguments instead of stopping there. According to the comment in
:the code, such behaviour has been introduced to make shell scripts
:working. However there are two problems with this approach:
:1. According to the POSIX, execve() should pass arguments list
:unmodified to the newly created process. This means that if I invoke
:execve with argv being NULL, the new image should see argc == 0 and
:argv = NULL. DF in this case will copy the new image path as argv
:and new image will see see it as argv/argc == 1.
:2. In some cases, the new logic will result in bogus arguments passed to
:the new image or EFAULT when first argument is NULL. This will happen
:due to the bug in routine which copies arguments from the user space
:into the kernel space. It assumes that both argv and argv are
:NULL, while only former is required to be to stop processing.
:The proper fix is to move special handling of argv == NULL case into
:imgact_shell.c where it belongs.
It's unclear whether passing a NULL argv for any case is legal.
I think to be strictly conforming, argv must always be non-NULL.
Latest POSIX explicitly mentions that NULL argv is permitted,
although discouraged for compatibility reasons:
Early proposals required that the value of argc passed to main() be "one
or greater". This was driven by the same requirement in drafts of the
ISO C standard. In fact, historical implementations have passed a value
of zero when no arguments are supplied to the caller of the exec
functions. This requirement was removed from the ISO C standard and
subsequently removed from this volume of IEEE Std 1003.1-2001 as well.
The wording, in particular the use of the word should, requires a
Strictly Conforming POSIX Application to pass at least one argument to
the exec function, thus guaranteeing that argc be one or greater when
invoked by such an application. In fact, this is good practice, since
many existing applications reference argv without first checking the
value of argc.
You'll have to be more specific about case (2). What in the codebase
are you refering to, file and line ?
Trunk as of several hours ago, sys/kern/kern_exec.c function
exec_copyin_args() around line 700. The code there fetches pointer to
argv from userspace, checks if it's NULL and puts first argument
instead of it. Then it increases userspace pointer by one and fetches
the next pointer *unconditionally*, so that in the case when argv is
NULL you may get some invalid (e.g. junk but non-NULL pointer) and get
EFAULT for no reason. The same code ignores argv being NULL - see my
follow-up. FreeBSD code in this case explicitly returns EFAULT.