[PATCH 08/10] uml: cleanup run_helper() API to fix a leak



From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@xxxxxxxx>

Freeing the stack is left uselessly to the caller of run_helper in some cases -
this is taken from run_helper_thread, but here it is useless, so no caller needs
it and the only place where this happens has a potential leak - in case of error
neither run_helper() nor xterm_open() call free_stack().
At this point passing a pointer is not needed - the stack pointer should be passed
directly, but this change is not done here.

Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@xxxxxxxx>
---

arch/um/drivers/xterm.c | 2 --
arch/um/os-Linux/helper.c | 7 +++----
2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/um/drivers/xterm.c b/arch/um/drivers/xterm.c
index 386f8b9..850221d 100644
--- a/arch/um/drivers/xterm.c
+++ b/arch/um/drivers/xterm.c
@@ -136,8 +136,6 @@ int xterm_open(int input, int output, in
return(pid);
}

- if(data->stack == 0) free_stack(stack, 0);
-
if (data->direct_rcv) {
new = os_rcv_fd(fd, &data->helper_pid);
} else {
diff --git a/arch/um/os-Linux/helper.c b/arch/um/os-Linux/helper.c
index e887179..c316dfc 100644
--- a/arch/um/os-Linux/helper.c
+++ b/arch/um/os-Linux/helper.c
@@ -52,7 +52,8 @@ static int helper_child(void *arg)
}

/* Returns either the pid of the child process we run or -E* on failure.
- * XXX The alloc_stack here breaks if this is called in the tracing thread */
+ * XXX The alloc_stack here breaks if this is called in the tracing thread, so
+ * we need to receive a preallocated stack (a local buffer is ok). */
int run_helper(void (*pre_exec)(void *), void *pre_data, char **argv,
unsigned long *stack_out)
{
@@ -118,10 +119,8 @@ out_close:
close(fds[1]);
close(fds[0]);
out_free:
- if (stack_out == NULL)
+ if ((stack_out == NULL) || (*stack_out == 0))
free_stack(stack, 0);
- else
- *stack_out = stack;
return ret;
}

Chiacchiera con i tuoi amici in tempo reale!
http://it.yahoo.com/mail_it/foot/*http://it.messenger.yahoo.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



Relevant Pages

  • Re: StdCall vs. CDecl
    ... I don't understand why "leaving arguments on the stack" is benign. ... If the caller expects the callee to "clean up the stack" (that is, restore the stack pointer to the value it had before the caller pushed a call frame onto the stack), then I would expect the caller to be thoroughly messed up after the call returns and the caller tries to get to its own data on the stack. ... That's for calling a cdecl function as stdcall. ...
    (microsoft.public.dotnet.framework.interop)
  • Re: StdCall vs. CDecl
    ... I don't understand why "leaving arguments on the stack" is benign. ... If the caller expects the callee to "clean up the stack" (that is, ... by storing the old stack pointer away somewhere before the call so that it can ... customary to use stdcall for exported DLL functions, ...
    (microsoft.public.dotnet.framework.interop)
  • Re: How to define a function which return a matrix to caller by value?
    ... It is when the whole object is copied to the caller stack on return. ... What is returned is the value of the object, not a reference to the heap ... To allocate space, the caller clearly needs to know how much space to allocate. ...
    (comp.lang.fortran)
  • Re: Is it possible to use the value of the PROGRAM ID within the source code?
    ... Every called program has a way of returning to its caller. ... >information about calling history in some sort of hardware stack? ... True but discordant with the culture of operating systems and other languages. ... premise is that called programs may irrationally corrupt parameter values. ...
    (comp.lang.cobol)
  • Re: Stack Overflow
    ... Sub Caller() ... '<<< COMPILER STORES THIS LOCATION ... Sub CalledFunction() ... (Think of it like a stack of plates. ...
    (microsoft.public.excel.programming)