Re: possible buffer overflow in calls.c?

[Ian Lance Taylor <iant at google dot com>]

Jaka MoÄnik <jaka@xlab.si> writes:

> in emit_library_call_value_1(), local var stack_usage_map_buf is
> allocated with
>
>   stack_usage_map_buf = XNEWVEC (char, highest_outgoing_arg_in_use);
>   stack_usage_map = stack_usage_map_buf;
>
> allocating a buffer with 1 char for each byte of stack required for
> outgoing args.
>
> then in a loop iterating over args, upper and lower bounds for indexing
> a part of that buffer are calculated with:
>
> #ifdef ARGS_GROW_DOWNWARD
>     /* stack_slot is negative, but we want to index stack_usage_map
>        with positive values.  */
>     upper_bound = -argvec[argnum].locate.offset.constant + 1;
>     lower_bound = upper_bound - argvec[argnum].locate.size.constant;
> #else
>     lower_bound = argvec[argnum].locate.offset.constant;
>     upper_bound = lower_bound + argvec[argnum].locate.size.constant;
> #endif
>
> the problem with this code is that argvec[argnum].locate.offset is the
> offset to the actual arg on the stack *excluding* any padding, while
> argvec[argnum].locate.size contains the size *including* any possible
> padding (according to comments on struct locate_and_pad_arg_data in
> expr.h). therefore, upper_bound for the last arg can be up to a stack
> slot size too large than the space allocated for stack_usage_map.

I don't quite see it.  highest_outgoing_args_in_use is at least as large
as args_size.constant, and that counts the locate.size for each
argument.  So it should always include the extra padding.

That said, it would not be shocking if there were a bug in this code.
It's not particularly common to use emit_library_call_value_1 with code
that passes parameters on the stack.  And it's even less common to use
it with parameters that require padding when they are pushed on the
stack.  So a miscalculation in such a scenario could survive for quite
along time.

Ian