This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix stacksize returned for main process thread when RLIMIT_STACKis relevant


sorry for the delay.

> On Thu, 14 Jun 2012 16:09:59 -0400, KOSAKI wrote:
>>> rounds up the
>>> +		         stack extension request.  */
>>> +		      iattr->stacksize = (iattr->stacksize
>>> +					&  -(intptr_t)
>>> GLRO(dl_pagesize));
>>
>> Hmm..
>> This comment is unclear to me. Which round up are you afraid? kernel
>> always account vma size. And, when unaligned stacksize is happen?
>> Both "to" and "stack_end" seems always page size aligned.
> 
> I'm not rounding down to page size for the case where stack size is
> limited by the vma below since that will always be page-aligned. The
> case you see above is when the stack size is a function of the rlimit.

Ah, I realized I misunderstood setrlimit spec. thank you. yes, RLIMIT_STACK
is not guaranteed page aligned. getrlimit(RLIMIT_STACK) may return page unaligned
size, but stack expansion logic in kernel round it up into pagesize aligned.

Sorry, my last mail was wrong. btw, if you want getrlimit(RLIM_STACK) return page 
aligned value, please let me know.

And then, one more question is upcoming. Why do you only round up pthread_getattr_np()
case and don't round up getrlimit(RLIM_STACK) case? I'm still not convinced when this 
rounding up is needed and they seems inconsistent. Can you please help me clarify?



>>> +  if (stack_limit.rlim_cur>  stacksize)
>>> +    {
>>> +      stack_limit.rlim_cur = stacksize - 4095;
>>> +      printf ("Adjusting RLIMIT_STACK to %zu¥n",
>>> stack_limit.rlim_cur);
>>> +      if ((ret = setrlimit (RLIMIT_STACK,&stack_limit)))
>>
>> This test is also unclear to me.
>>
>> Why "if (stack_limit.rlim_cur>  stacksize)" check is necessary? It is
>> always true because we have no way to create a process w/o auxv.
>>
>> And if I understand correctly, this setrlimit() never change a
>> stacksize because current pthread_getattr_np() is always returned
>> page aligned size. i.e. roundup(N*pagesize - 4095) == N*pagesize.
> 
> Agreed, the test is in fact moot and I should be doing the reduction
> unconditionally. Now to explain why I am doing that reduction:
> 
> When I get the first stacksize, it could either be a function of
> RLIMIT_STACK, i.e. RLIMIT_STACK - auxv pages, or it could be
> __libc_stack_end - 'to' of previous vma. I want to test the case where
> stacksize is a function of RLIMIT_STACK, so I will try to influence the
> stacksize I return in the next pthread_getattr_np.
> 
> If the current pthread_getattr_np returned as a function of the
> previous vma, then size - 4095 becomes an unaligned rlimit. The rlimit
> will count the auxv pages, so even if I reduce by less than a page, I
> am effectively reducing the stacksize further by auxv pages + 4095.
> However I don't really care about that since that is only incidental.
> What I really care about is that rlimit is now small enough to ensure
> that the next pthread_getattr_np is a function of rlimit.

got it.


> Now if the next stacksize returned is a function of rlimit, it could be
> an unaligned value and hence stackaddr too would be unaligned. If I try
> to access stackaddr, the kernel rounds *up* to page size and hence
> results in a request larger than rlimit, resulting in a request
> violation. 

Hmmm...
I haven't caught why do you think this is violation. the spec only says
if a request is smaller than rlimit, it shouldn't make SEGV. but don't
say anything about opposition. I think kernel round up is not invalid.
Which scenario do you worry about?


> To prevent this, pthread_getattr_np must round the stacksize
> *down* to make it page aligned so that the kernel does not convert our
> request to something larger than rlimit.

And, please teach me why we should round down at pthread_getattr_np instead
of dropping kernel rounding up logic in stack expansion.


> All of this should be clearer to you when you actually run the test case
> I have written with an unpatched glibc. You should be able to see this
> problem even without my earlier patch. Replace the do_test with main
> and run it independently.
> 
>> Finally, if we should care "the stack is limited by the vma below"
>> case, we should reduce a stack pagesize(), I think
>> allocate_and_test() assume there is at least 1 unmapped (or
>> PROT_NONE) region at neighbor of a stack.
> 
> No, I *don't* care about the case where stack is limited by the vma
> below stack since that is safe from the pagesize issue as well as the
> problem of auxv. I only care about the rlimit case. I should ideally be
> writing a test case that tests both nevertheless, but I cannot do both
> tests without making the test dependent on proc output.

fair point.


> I can go about doing the test in two ways, The first way, I check the
> general case without reducing the rlimit and then reduce the rlimit to
> ensure I am checking that case. This is useless because of the way
> rlimit_stack works. If I run the first test, the second test will
> always succeed because the stack vma is already expanded to the
> previous stacksize. Even if I reduce the rlimit, I will never get a
> segfault since I will never run into the rlimit check, which occurs
> only on stack expansion.

I confirmed kernel code. yes, you are right.


> Checking the other way around, i.e. reducing rlimit and testing first
> and then increasing rlimit to beyond the 'to' of the previous vma
> requires me to look for the previous vma as well as the end of the sack
> vma in the test case, which will make the test case linux-specific. We
> avoid doing that in test cases since we want them to be as generic as
> possible.
> 
> Given that though, I believe it should be OK to use linux-specific code
> in test cases within NPTL since NPTL itself is linux-specific. Roland,
> can I do this or do we aim to make the NPTL implementation less linux
> specific in future?




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]