Re: [gtk-vnc-devel] PATCH: Fix args passed to makecontext()

[Anthony Liguori <anthony codemonkey ws>]

Looks good to me.

Thanks for tracking this down!

Regards,

Anthony Liguori

Daniel P. Berrange wrote:
> POSIX has the following to say about makecontext()
>
> [quote makecontext(3p)]
>        allocated for it. The application shall  ensure  that  the
>        value  of argc matches the number of arguments of type int
>        passed to func; otherwise, the behavior is undefined.
> [/quote]
>
>
> In the continuation.c file though we're passing a 'struct continuation *'
> as the arg. On i386 this works fine since sizeof(void*) == sizeof(int).
> On x86_64 this is truncating the top 4 bytes of the arg. This practically
> never matters because very few apps use so much memory that allocations
> need to live > 4 GB mark in address space. There is no guarentee that
> memory is allocated from the bottom up though. On Fedora 9 perhaps 1 in 3
> times I run virt-manager, the continuation is allocated beyond the 4gb
> mark. This results in a fairly predictable crash in coroutine_trampoline()
> because the arg has garbage for the top 4 bytes. The patch is reasonably
> straightforward, just have a union of a void * and an int[2], and pass
> in a pair of int args. This resolves the crashes I see on x86_64 Fedora
>
> The symptoms of this bug are that you'll get a SEGV showing with the top
> of the stack at coroutine_trampoline() or possibly in swapcontext() itself
>
> Dan.
>
> diff -r 2ca361cd74cd src/continuation.c
> --- a/src/continuation.c	Wed Apr 02 09:22:31 2008 -0500
> +++ b/src/continuation.c	Wed Apr 02 13:55:09 2008 -0400
> @@ -10,8 +10,31 @@
>  
>  #include "continuation.h"
>  
> +/*
> + * va_args to makecontext() must be type 'int', so passing
> + * the pointer we need may require several int args. This
> + * union is a quick hack to let us do that
> + */
> +union cc_arg {
> +	void *p;
> +	int i[2];
> +};
> +
> +static void continuation_trampoline(int i0, int i1)
> +{
> +	union cc_arg arg;
> +	struct continuation *cc;
> +	arg.i[0] = i0;
> +	arg.i[1] = i1;
> +	cc = arg.p;
> +
> +	cc->entry(cc);
> +}
> +
>  int cc_init(struct continuation *cc)
>  {
> +	union cc_arg arg;
> +	arg.p = cc;
>  	if (getcontext(&cc->uc) == -1)
>  		return -1;
>  
> @@ -20,7 +43,7 @@
>  	cc->uc.uc_stack.ss_size = cc->stack_size;
>  	cc->uc.uc_stack.ss_flags = 0;
>  
> -	makecontext(&cc->uc, (void *)cc->entry, 1, cc);
> +	makecontext(&cc->uc, (void *)continuation_trampoline, 2, arg.i[0], arg.i[1]);
>  
>  	return 0;
>  }
>
>