Re: [PATCH] binfmt_script: do not leave interp on stack
On Fri, Oct 12, 2012 at 10:50 PM, halfdog wrote: > Kees Cook wrote: >> More importantly, I also wonder if interp handling to just be >> changed to be an allocation that needs to be cleaned up, as done with >> argv? > > You mean like an allocation on the stack of the new process' growing > stack? This would be cleaned automatically if something goes wrong > during exec. Either in userspace like argv, or just a straight kmalloc. Looking at the code, the problem is that binfmt_script and binfmt_misc are "rewrite" hacks (they change the bprm instead of actually starting a process), and the module loading is a hack in that it retries all the loaders a second time. These two hacks together aren't very compatible if the error path of the rewrite hacks expects to see the bprm go away instead of getting retried. I'm concerned that the proposed patch is really just a band-aid on top of a broken design. To make this safe, either the bprm needs to be explicitly copied for each recursion attempt (so the post-module-load retry starts with a clean bprm), or everything about the bprm needs to stay off the stack (to allow rewrite modifications to be stable). I think the latter approach is best since it means we don't have to execute the rewrite logic twice. It just means we must take a closer look at the lifetime of that structure and make sure we're cleaning it up correctly. -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_script: do not leave interp on stack
Kees Cook writes: > + /* > + * Since bprm is already modified, we cannot continue if the the s/the the/the/ Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_script: do not leave interp on stack
Kees Cook keesc...@chromium.org writes: + /* + * Since bprm is already modified, we cannot continue if the the s/the the/the/ Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_script: do not leave interp on stack
On Fri, Oct 12, 2012 at 10:50 PM, halfdog m...@halfdog.net wrote: Kees Cook wrote: More importantly, I also wonder if interp handling to just be changed to be an allocation that needs to be cleaned up, as done with argv? You mean like an allocation on the stack of the new process' growing stack? This would be cleaned automatically if something goes wrong during exec. Either in userspace like argv, or just a straight kmalloc. Looking at the code, the problem is that binfmt_script and binfmt_misc are rewrite hacks (they change the bprm instead of actually starting a process), and the module loading is a hack in that it retries all the loaders a second time. These two hacks together aren't very compatible if the error path of the rewrite hacks expects to see the bprm go away instead of getting retried. I'm concerned that the proposed patch is really just a band-aid on top of a broken design. To make this safe, either the bprm needs to be explicitly copied for each recursion attempt (so the post-module-load retry starts with a clean bprm), or everything about the bprm needs to stay off the stack (to allow rewrite modifications to be stable). I think the latter approach is best since it means we don't have to execute the rewrite logic twice. It just means we must take a closer look at the lifetime of that structure and make sure we're cleaning it up correctly. -Kees -- Kees Cook Chrome OS Security -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_script: do not leave interp on stack
Kees Cook wrote:
> On Thu, Oct 11, 2012 at 07:32:40PM -0700, Kees Cook wrote:
>> +/*
>> + * Since bprm is already modified, we cannot continue if the the
>> + * handlers for starting the new interpreter have failed.
>> + * Make sure that we do not return -ENOEXEC, as that would
>> + * allow searching for handlers to continue.
>> + */
>> +if (retval == -ENOEXEC)
>> +retval = -EINVAL;
>
> After looking at this some more, I wonder if this should be -ELOOP
> instead? Or maybe that should happen if/when the recursion depth problem is
> fixed?
>
> This is much more obvious, instead of "Invalid argument":
> $ ./dotest.sh
> file-AAAfile-:
> bad interpreter: Too many levels of symbolic links
In my opinion, a different, more specific error code is nice, but when
not self-explanatory, it would need to be documented to avoid confusion.
I do not know, what would be the most accepted way to change syscall
return value semantics, if to change semantics or add new ones. From
man-pages, many have already some meaning and only some could be
re-interpreted in that way:
E2BIG: The total number of bytes in the environment (envp) and argument
list (argv) is too large. (not perfect, because usually only associated
with mem/file size issues)
ELOOP: Too many symbolic links were encountered in resolving filename
or the name of a script or ELF interpreter. (currently no distinction
from real symlink problems)
EMFILE: The process has the maximum number of files open. (too generic?)
This one has already a meaning, but only for ELF not script (but since
script might also call ELF in the end, user cannot know):
EINVAL: An ELF executable had more than one PT_INTERP segment (i.e.,
tried to name more than one interpreter).
Those are not yet unused, but I think it is a bad idea to add them,
since some programs might be confused by unexpected error code:
ELIBMAX: Attempting to link in too many shared libraries (not a really
good match)
EMLINK: Too many links (somehow generic, do not know if usually used
another way).
It is strange: from current description, this one suits best, the only
reason why we want to get rid of it is, that it triggers module
reloading and another round of execution.
ENOEXEC: An executable is not in a recognized format, is for the wrong
architecture, or has some other format error that means it cannot be
executed.
Perhaps it would be better to continue returning ENOEXEC from syscall in
that case but change the logic for module-reloading (use some other
return value meaning in binfmt handlers in kernel internally)?
> More importantly, I also wonder if interp handling to just be
> changed to be an allocation that needs to be cleaned up, as done with
> argv?
You mean like an allocation on the stack of the new process' growing
stack? This would be cleaned automatically if something goes wrong
during exec.
> Right now interp just points to the filename argument handed to
> do_execve. Especially since it looks like binfmt_misc is vulnerable
> to this as well, since it runs the risk of getting -ENOEXEC from
> search_binary_handler, leaving bprm->interp pointing into the stack,
> only to get it recalled after module loading attempts:
>
> static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
> {
> ...
> char iname[BINPRM_BUF_SIZE];
> ...
> bprm->interp = iname; /* for binfmt_script */
> ...
> retval = search_binary_handler (bprm, regs);
> if (retval < 0)
> goto _error;
> ...
> _ret:
> return retval;
> _error:
> if (fd_binary > 0)
> sys_close(fd_binary);
> bprm->interp_flags = 0;
> bprm->interp_data = 0;
> goto _ret;
> }
Correct. I hope the patch should be a formality, as soon as discussion
on this one is done.
--
http://www.halfdog.net/
PGP: 156A AE98 B91F 0114 FE88 2BD8 C459 9386 feed a bee
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_script: do not leave interp on stack
On Thu, Oct 11, 2012 at 07:32:40PM -0700, Kees Cook wrote:
> + /*
> + * Since bprm is already modified, we cannot continue if the the
> + * handlers for starting the new interpreter have failed.
> + * Make sure that we do not return -ENOEXEC, as that would
> + * allow searching for handlers to continue.
> + */
> + if (retval == -ENOEXEC)
> + retval = -EINVAL;
After looking at this some more, I wonder if this should be -ELOOP
instead? Or maybe that should happen if/when the recursion depth problem is
fixed?
This is much more obvious, instead of "Invalid argument":
$ ./dotest.sh
file-AAAfile-:
bad interpreter: Too many levels of symbolic links
More importantly, I also wonder if interp handling to just be
changed to be an allocation that needs to be cleaned up, as done with
argv? Right now interp just points to the filename argument handed to
do_execve. Especially since it looks like binfmt_misc is vulnerable
to this as well, since it runs the risk of getting -ENOEXEC from
search_binary_handler, leaving bprm->interp pointing into the stack,
only to get it recalled after module loading attempts:
static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
{
...
char iname[BINPRM_BUF_SIZE];
...
bprm->interp = iname; /* for binfmt_script */
...
retval = search_binary_handler (bprm, regs);
if (retval < 0)
goto _error;
...
_ret:
return retval;
_error:
if (fd_binary > 0)
sys_close(fd_binary);
bprm->interp_flags = 0;
bprm->interp_data = 0;
goto _ret;
}
:(
-Kees
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_script: do not leave interp on stack
On Thu, Oct 11, 2012 at 07:32:40PM -0700, Kees Cook wrote:
+ /*
+ * Since bprm is already modified, we cannot continue if the the
+ * handlers for starting the new interpreter have failed.
+ * Make sure that we do not return -ENOEXEC, as that would
+ * allow searching for handlers to continue.
+ */
+ if (retval == -ENOEXEC)
+ retval = -EINVAL;
After looking at this some more, I wonder if this should be -ELOOP
instead? Or maybe that should happen if/when the recursion depth problem is
fixed?
This is much more obvious, instead of Invalid argument:
$ ./dotest.sh
file-AAAfile-:
bad interpreter: Too many levels of symbolic links
More importantly, I also wonder if interp handling to just be
changed to be an allocation that needs to be cleaned up, as done with
argv? Right now interp just points to the filename argument handed to
do_execve. Especially since it looks like binfmt_misc is vulnerable
to this as well, since it runs the risk of getting -ENOEXEC from
search_binary_handler, leaving bprm-interp pointing into the stack,
only to get it recalled after module loading attempts:
static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
{
...
char iname[BINPRM_BUF_SIZE];
...
bprm-interp = iname; /* for binfmt_script */
...
retval = search_binary_handler (bprm, regs);
if (retval 0)
goto _error;
...
_ret:
return retval;
_error:
if (fd_binary 0)
sys_close(fd_binary);
bprm-interp_flags = 0;
bprm-interp_data = 0;
goto _ret;
}
:(
-Kees
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] binfmt_script: do not leave interp on stack
Kees Cook wrote:
On Thu, Oct 11, 2012 at 07:32:40PM -0700, Kees Cook wrote:
+/*
+ * Since bprm is already modified, we cannot continue if the the
+ * handlers for starting the new interpreter have failed.
+ * Make sure that we do not return -ENOEXEC, as that would
+ * allow searching for handlers to continue.
+ */
+if (retval == -ENOEXEC)
+retval = -EINVAL;
After looking at this some more, I wonder if this should be -ELOOP
instead? Or maybe that should happen if/when the recursion depth problem is
fixed?
This is much more obvious, instead of Invalid argument:
$ ./dotest.sh
file-AAAfile-:
bad interpreter: Too many levels of symbolic links
In my opinion, a different, more specific error code is nice, but when
not self-explanatory, it would need to be documented to avoid confusion.
I do not know, what would be the most accepted way to change syscall
return value semantics, if to change semantics or add new ones. From
man-pages, many have already some meaning and only some could be
re-interpreted in that way:
E2BIG: The total number of bytes in the environment (envp) and argument
list (argv) is too large. (not perfect, because usually only associated
with mem/file size issues)
ELOOP: Too many symbolic links were encountered in resolving filename
or the name of a script or ELF interpreter. (currently no distinction
from real symlink problems)
EMFILE: The process has the maximum number of files open. (too generic?)
This one has already a meaning, but only for ELF not script (but since
script might also call ELF in the end, user cannot know):
EINVAL: An ELF executable had more than one PT_INTERP segment (i.e.,
tried to name more than one interpreter).
Those are not yet unused, but I think it is a bad idea to add them,
since some programs might be confused by unexpected error code:
ELIBMAX: Attempting to link in too many shared libraries (not a really
good match)
EMLINK: Too many links (somehow generic, do not know if usually used
another way).
It is strange: from current description, this one suits best, the only
reason why we want to get rid of it is, that it triggers module
reloading and another round of execution.
ENOEXEC: An executable is not in a recognized format, is for the wrong
architecture, or has some other format error that means it cannot be
executed.
Perhaps it would be better to continue returning ENOEXEC from syscall in
that case but change the logic for module-reloading (use some other
return value meaning in binfmt handlers in kernel internally)?
More importantly, I also wonder if interp handling to just be
changed to be an allocation that needs to be cleaned up, as done with
argv?
You mean like an allocation on the stack of the new process' growing
stack? This would be cleaned automatically if something goes wrong
during exec.
Right now interp just points to the filename argument handed to
do_execve. Especially since it looks like binfmt_misc is vulnerable
to this as well, since it runs the risk of getting -ENOEXEC from
search_binary_handler, leaving bprm-interp pointing into the stack,
only to get it recalled after module loading attempts:
static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
{
...
char iname[BINPRM_BUF_SIZE];
...
bprm-interp = iname; /* for binfmt_script */
...
retval = search_binary_handler (bprm, regs);
if (retval 0)
goto _error;
...
_ret:
return retval;
_error:
if (fd_binary 0)
sys_close(fd_binary);
bprm-interp_flags = 0;
bprm-interp_data = 0;
goto _ret;
}
Correct. I hope the patch should be a formality, as soon as discussion
on this one is done.
--
http://www.halfdog.net/
PGP: 156A AE98 B91F 0114 FE88 2BD8 C459 9386 feed a bee
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] binfmt_script: do not leave interp on stack
When binfmt_script's load_script ran, it would manipulate bprm->buf and
leave bprm->interp pointing to the local stack. If a series of scripts
are executed, the final one will have leaked kernel stack contents into
the cmdline. For a proof of concept, see DoTest.sh from:
http://www.halfdog.net/Security/2012/LinuxKernelBinfmtScriptStackDataDisclosure/
Largely based on a patch by halfdog. Cleaned up various style issues,
including those reported by Randy Dunlap and scripts/checkpatch.pl.
Cc: halfdog
Cc: sta...@vger.kernel.org
Signed-off-by: Kees Cook
---
For more background, see the earlier thread:
https://lkml.org/lkml/2012/8/18/75
---
fs/binfmt_script.c | 126 +---
1 file changed, 101 insertions(+), 25 deletions(-)
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index d3b8c1f..15fe9e8 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -14,12 +14,22 @@
#include
#include
+/*
+ * Check if this handler is suitable to load the interpreter identified
+ * by first BINPRM_BUF_SIZE bytes in bprm->buf following "#!".
+ *
+ * Returns:
+ * 0: success; the new executable is ready in bprm->mm.
+ * -ve: interpreter not found, or other binfmts failed to find a
+ *suitable binary.
+ */
static int load_script(struct linux_binprm *bprm,struct pt_regs *regs)
{
const char *i_arg, *i_name;
char *cp;
struct file *file;
- char interp[BINPRM_BUF_SIZE];
+ char bprm_buf_copy[BINPRM_BUF_SIZE];
+ const char *bprm_old_interp_name;
int retval;
if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!') ||
@@ -30,34 +40,57 @@ static int load_script(struct linux_binprm *bprm,struct
pt_regs *regs)
* Sorta complicated, but hopefully it will work. -TYT
*/
- bprm->recursion_depth++;
- allow_write_access(bprm->file);
- fput(bprm->file);
- bprm->file = NULL;
+ /*
+* Keep bprm unchanged until we known that this is a script
+* to be handled by this loader. Copy bprm->buf for sure,
+* otherwise returning -ENOEXEC will make other handlers see
+* modified data.
+*/
+ memcpy(bprm_buf_copy, bprm->buf, BINPRM_BUF_SIZE);
- bprm->buf[BINPRM_BUF_SIZE - 1] = '\0';
- if ((cp = strchr(bprm->buf, '\n')) == NULL)
- cp = bprm->buf+BINPRM_BUF_SIZE-1;
+ /* Locate and truncate end of string. */
+ bprm_buf_copy[BINPRM_BUF_SIZE - 1] = '\0';
+ cp = strchr(bprm_buf_copy, '\n');
+ if (cp == NULL)
+ cp = bprm_buf_copy + BINPRM_BUF_SIZE - 1;
*cp = '\0';
- while (cp > bprm->buf) {
+ /* Truncate trailing white-space. */
+ while (cp > bprm_buf_copy) {
cp--;
if ((*cp == ' ') || (*cp == '\t'))
*cp = '\0';
else
break;
}
- for (cp = bprm->buf+2; (*cp == ' ') || (*cp == '\t'); cp++);
+ /* Skip leading white-space. */
+ for (cp = bprm_buf_copy + 2; (*cp == ' ') || (*cp == '\t'); cp++)
+ /* nothing */ ;
+
+ /*
+* No interpreter name found? No problem to let other handlers
+* retry, we did not change anything.
+*/
if (*cp == '\0')
- return -ENOEXEC; /* No interpreter name found */
+ return -ENOEXEC;
+
i_name = cp;
i_arg = NULL;
+ /* Find start of first argument. */
for ( ; *cp && (*cp != ' ') && (*cp != '\t'); cp++)
/* nothing */ ;
+ /* Truncate and skip leading white-space. */
while ((*cp == ' ') || (*cp == '\t'))
*cp++ = '\0';
if (*cp)
i_arg = cp;
- strcpy (interp, i_name);
+
+ /*
+* So this is our point-of-no-return: modification of bprm
+* will be irreversible, so if we fail to setup execution
+* using the new interpreter name (i_name), we have to make
+* sure that no other handler tries again.
+*/
+
/*
* OK, we've parsed out the interpreter name and
* (optional) argument.
@@ -68,34 +101,77 @@ static int load_script(struct linux_binprm *bprm,struct
pt_regs *regs)
* This is done in reverse order, because of how the
* user environment and arguments are stored.
*/
+
+ /*
+* Ugly: we store pointer to local stack frame in bprm,
+* so make sure to clean this up before returning.
+*/
+ bprm_old_interp_name = bprm->interp;
+ bprm->interp = i_name;
+
retval = remove_arg_zero(bprm);
if (retval)
- return retval;
- retval = copy_strings_kernel(1, >interp, bprm);
- if (retval < 0) return retval;
+ goto out;
+
+ /*
+* copy_strings_kernel is ok here, even when racy: since no
+* user can be attached to new mm, there is nobody to race
+*
[PATCH] binfmt_script: do not leave interp on stack
When binfmt_script's load_script ran, it would manipulate bprm-buf and
leave bprm-interp pointing to the local stack. If a series of scripts
are executed, the final one will have leaked kernel stack contents into
the cmdline. For a proof of concept, see DoTest.sh from:
http://www.halfdog.net/Security/2012/LinuxKernelBinfmtScriptStackDataDisclosure/
Largely based on a patch by halfdog. Cleaned up various style issues,
including those reported by Randy Dunlap and scripts/checkpatch.pl.
Cc: halfdog m...@halfdog.net
Cc: sta...@vger.kernel.org
Signed-off-by: Kees Cook keesc...@chromium.org
---
For more background, see the earlier thread:
https://lkml.org/lkml/2012/8/18/75
---
fs/binfmt_script.c | 126 +---
1 file changed, 101 insertions(+), 25 deletions(-)
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index d3b8c1f..15fe9e8 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -14,12 +14,22 @@
#include linux/err.h
#include linux/fs.h
+/*
+ * Check if this handler is suitable to load the interpreter identified
+ * by first BINPRM_BUF_SIZE bytes in bprm-buf following #!.
+ *
+ * Returns:
+ * 0: success; the new executable is ready in bprm-mm.
+ * -ve: interpreter not found, or other binfmts failed to find a
+ *suitable binary.
+ */
static int load_script(struct linux_binprm *bprm,struct pt_regs *regs)
{
const char *i_arg, *i_name;
char *cp;
struct file *file;
- char interp[BINPRM_BUF_SIZE];
+ char bprm_buf_copy[BINPRM_BUF_SIZE];
+ const char *bprm_old_interp_name;
int retval;
if ((bprm-buf[0] != '#') || (bprm-buf[1] != '!') ||
@@ -30,34 +40,57 @@ static int load_script(struct linux_binprm *bprm,struct
pt_regs *regs)
* Sorta complicated, but hopefully it will work. -TYT
*/
- bprm-recursion_depth++;
- allow_write_access(bprm-file);
- fput(bprm-file);
- bprm-file = NULL;
+ /*
+* Keep bprm unchanged until we known that this is a script
+* to be handled by this loader. Copy bprm-buf for sure,
+* otherwise returning -ENOEXEC will make other handlers see
+* modified data.
+*/
+ memcpy(bprm_buf_copy, bprm-buf, BINPRM_BUF_SIZE);
- bprm-buf[BINPRM_BUF_SIZE - 1] = '\0';
- if ((cp = strchr(bprm-buf, '\n')) == NULL)
- cp = bprm-buf+BINPRM_BUF_SIZE-1;
+ /* Locate and truncate end of string. */
+ bprm_buf_copy[BINPRM_BUF_SIZE - 1] = '\0';
+ cp = strchr(bprm_buf_copy, '\n');
+ if (cp == NULL)
+ cp = bprm_buf_copy + BINPRM_BUF_SIZE - 1;
*cp = '\0';
- while (cp bprm-buf) {
+ /* Truncate trailing white-space. */
+ while (cp bprm_buf_copy) {
cp--;
if ((*cp == ' ') || (*cp == '\t'))
*cp = '\0';
else
break;
}
- for (cp = bprm-buf+2; (*cp == ' ') || (*cp == '\t'); cp++);
+ /* Skip leading white-space. */
+ for (cp = bprm_buf_copy + 2; (*cp == ' ') || (*cp == '\t'); cp++)
+ /* nothing */ ;
+
+ /*
+* No interpreter name found? No problem to let other handlers
+* retry, we did not change anything.
+*/
if (*cp == '\0')
- return -ENOEXEC; /* No interpreter name found */
+ return -ENOEXEC;
+
i_name = cp;
i_arg = NULL;
+ /* Find start of first argument. */
for ( ; *cp (*cp != ' ') (*cp != '\t'); cp++)
/* nothing */ ;
+ /* Truncate and skip leading white-space. */
while ((*cp == ' ') || (*cp == '\t'))
*cp++ = '\0';
if (*cp)
i_arg = cp;
- strcpy (interp, i_name);
+
+ /*
+* So this is our point-of-no-return: modification of bprm
+* will be irreversible, so if we fail to setup execution
+* using the new interpreter name (i_name), we have to make
+* sure that no other handler tries again.
+*/
+
/*
* OK, we've parsed out the interpreter name and
* (optional) argument.
@@ -68,34 +101,77 @@ static int load_script(struct linux_binprm *bprm,struct
pt_regs *regs)
* This is done in reverse order, because of how the
* user environment and arguments are stored.
*/
+
+ /*
+* Ugly: we store pointer to local stack frame in bprm,
+* so make sure to clean this up before returning.
+*/
+ bprm_old_interp_name = bprm-interp;
+ bprm-interp = i_name;
+
retval = remove_arg_zero(bprm);
if (retval)
- return retval;
- retval = copy_strings_kernel(1, bprm-interp, bprm);
- if (retval 0) return retval;
+ goto out;
+
+ /*
+* copy_strings_kernel is ok here, even when racy: since no
+* user can be attached to new mm,
