Re: [9fans] Race condition in /sys/src/9/pc/trap.c?

[Elizabeth Jones <elly1@andrew.cmu.edu>]

On Thu, 30 Jul 2009, erik quanstrom wrote:

> On Thu Jul 30 00:05:45 EDT 2009, elly1@andrew.cmu.edu wrote:
>> My familiarity with the kernel source code is superficial to say the
>> least, but it seems to me that this code (from /sys/src/9/pc/trap.c)
>> contains a race condition:
>>
>>   702         if(sp<(USTKTOP-BY2PG) || sp>(USTKTOP-sizeof(Sargs)-BY2WD))
>>   703             validaddr(sp, sizeof(Sargs)+BY2WD, 0);
>>   704
>>   705         up->s = *((Sargs*)(sp+BY2WD));
>>
>> We verify that the address is correct; is there any reason another thread
>> in the same address space cannot start running after line 703 completes
>> and unmap that memory, causing us to access unmapped memory at line 705?
>> The system call entry is itself an interrupt gate, but line 689 is
>> spllo(), and we appear to hold no locks at this point.
>
> plan 9 threads are cooperatively scheduled.  so
> the correct term is proc.  but you are correct,
> another proc sharing memory with this one
> could be running.  however, that proc would
> not have access to this proc's stack.  (rfork
> doesn't allow shared stack.)  and even if it
> did, plan 9 stacks don't shrink.

What if sp points inside a segment which is not the actual stack segment?
Then could someone else come along and segdetach() it in between the two
mentioned lines?

> let's suppose that the address is invalid later.
> the kernel always moves data to/from user
> buffers outside of any locks because even
> valid targets may be paged out.  if the address
> is truely invalid, waserror() will be true and
> the else branch starting at 714 will be executed
>
> - erik

-- Elly