On Sat, Jan 29, 2005 at 12:49:09PM -0800, Matthew Dillon wrote:
> :> 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[0] 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[0] 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.
> :
> :-Maxim
>
> That looks pretty straightforward. The code has changed very little
> from FreeBSD. I'll do the first part of the cleanup but for the moment
> I think we should leave the argv[0] NULL check in the common code rather
> then just having it in the script code.
I can't agree. It's much easier to handle it in the
imgact_shell - it only adds 2 new lines of the code.
Additional benefit will be that the code is the same
in FreeBSD and DF after that (you are probably don't
care, but anyway ;), since it's what I am going to
commit in few minutes.
-Maxim
Index: imgact_shell.c
===================================================================
RCS file: /home/dcvs/src/sys/kern/imgact_shell.c,v
retrieving revision 1.4
diff -d -u -r1.4 imgact_shell.c
--- imgact_shell.c 16 Nov 2003 02:37:39 -0000 1.4
+++ imgact_shell.c 29 Jan 2005 22:03:42 -0000
@@ -109,7 +109,8 @@
* added and 'length' is the number of bytes being removed.
*/
offset += strlen(imgp->args->fname) + 1; /* add fname */
- length = strlen(imgp->args->begin_argv) + 1; /* bytes to delete */
+ length = (imgp->args->argc == 0) ? 0 :
+ strlen(imgp->args->begin_argv) + 1; /* bytes to delete */
if (offset - length > imgp->args->space)
return (E2BIG);
@@ -121,7 +122,13 @@
imgp->args->begin_envv += offset;
imgp->args->endp += offset;
imgp->args->space -= offset;
- /* decr argc remove old argv[0], incr argc for fname add, net 0 */
+
+ /*
+ * If there were no arguments then we've added one, otherwise
+ * decr argc remove old argv[0], incr argc for fname add, net 0
+ */
+ if (imgp->args->argc == 0)
+ imgp->args->argc = 1;
/*
* Loop through the interpreter name yet again, copying as