Re: [PATCH 2/2][BZ #12416] Use stack boundaries from /proc/PID/mapsto make stack executable

[KOSAKI Motohiro <kosaki dot motohiro at gmail dot com>]

On Mon, Jun 11, 2012 at 5:38 PM, Siddhesh Poyarekar <siddhesh@redhat.com> wrote:
> On 12 June 2012 02:44, KOSAKI wrote:
>> I received a complains against this change from ruby folks. Because
>> of, ruby interpreter show special error message when stack overflow is
>> detected. and they assume "stackaddr - stacksize" point to stack edge.
>>
>> But this patch only changed stackaddr. thus the assuption broke.
>> Is there any chance to change stacksize too?
>
> Can you or the ruby devs please give a more detailed explanation of the
> problem they're seeing? The patch does in fact change the stack size if
> the distance of the stack bottom from the vma immediately below the
> stack top is less than the stack rlimit. From nptl/pthread_getattr_np.c:
>
> ? ? ? ? ? ? ? ? ? ? ?iattr->stacksize = rl.rlim_cur;
> ? ? ? ? ? ? ? ? ? ? ?iattr->stackaddr = stack_end;
>
> ? ? ? ? ? ? ? ? ? ? ?/* The limit might be too high. ?*/
> ? ? ? ? ? ? ? ? ? ? ?if ((size_t) iattr->stacksize
> ? ? ? ? ? ? ? ? ? ? ? ? ?> (size_t) iattr->stackaddr - last_to)
> ? ? ? ? ? ? ? ? ? ? ? ?iattr->stacksize = (size_t) iattr->stackaddr -
> ? ? ? ? ? ? ? ? ? ? ?last_to;
>
> See how stacksize is derived from stackaddr, which in this case is the
> stack bottom. In fact, the stackaddr you get from
> pthread_attr_getstack() is again derived from the stack bottom and
> stack size to get the stack top, so doing stackaddr - stacksize with
> that value will lead you nowhere.

sorry, my explanation was not clear. "stackaddr - stacksize" meant to
call pthread_attr_getstack().

rl.rlim_cur represent kernel vma based size. then, if you ignore some
regions (e.g  argv, environ and auxv area), I think the right stack size
is following.

  ignore_size = to - stack_end;
  iattr->stacksize = rl.rlim_cur - ignore_size;

otherwise, a result of pthread_attr_getstack() may violate following sentence.

http://pubs.opengroup.org/onlinepubs/007904975/functions/pthread_attr_setstack.html

>All pages within the stack described by stackaddr and stacksize shall be
> both readable and writable by the thread.


If my parsing is correct,  following adjustment only works when previous vma is
enough near.

> ? ? ? ? ? ? ? ? ? ? ?/* The limit might be too high. ?*/
> ? ? ? ? ? ? ? ? ? ? ?if ((size_t) iattr->stacksize
> ? ? ? ? ? ? ? ? ? ? ? ? ?> (size_t) iattr->stackaddr - last_to)
> ? ? ? ? ? ? ? ? ? ? ? ?iattr->stacksize = (size_t) iattr->stackaddr -
> ? ? ? ? ? ? ? ? ? ? ?last_to;

Am I missing something?



> pthread_attr_getstackaddr does return the bottom of stack but that
> function is deprecated and that usage should be replaced with
> pthread_attr_getstack.
>
> A reproducer would help to understand what they're seeing.

Please give me some time. I need to build libc development sandbox.