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

[cinap_lenrek@gmx.de]

[-- Attachment #1: Type: text/plain, Size: 105 bytes --]

... but you can still shrink segments for other processes :-)

(this crashes the kernel)

--
cinap

[-- Attachment #2: Type: message/rfc822, Size: 7027 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 199 bytes --]

hm... what about the other stuff port/sysfile.c?

they also just checks pointers with validaddr and then call
into the device handler without locking anything / holding
anything.

--
cinap


[-- Attachment #2.1.2: Type: message/rfc822, Size: 4214 bytes --]

From: Sape Mullender <sape@plan9.bell-labs.com>
To: 9fans@9fans.net
Cc: geoff@plan9.bell-labs.com, jmk@plan9.bell-labs.com
Subject: Re: [9fans] Race condition in /sys/src/9/pc/trap.c?
Date: Thu, 30 Jul 2009 16:37:50 +0200
Message-ID: <a77443fd0762504eb1179278b34d0351@plan9.cs.bell-labs.com>

> 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

I think you may be right, Elly.  Multithreaded programs indeed have their
stack running outside the stack segment, so this could happen there.
splhi won't even do on a multiprocessor.  One should probably lock down
the segment.
We've never seen this happen, of course — or rather, we haven't noticed
this as the cause of a crash.

	Sape

[-- Attachment #3: crash.c --]
[-- Type: text/plain, Size: 408 bytes --]

#include <u.h>
#include <libc.h>

void
main(int argc, char **argv)
{
	char *buf;
	int fd;

	if((fd = open("/dev/zero", OREAD)) < 0)
		sysfatal("open");

	buf = (char*)0x600000;
	segattach(0, "memory", buf, 2*4096);

	switch(rfork(RFPROC|RFMEM)){
	case -1:
		sysfatal("fork");
		break;
	case 0:
		for(;;){
			read(fd, buf+4096, 4096);
		}
	}
	sleep(1000);
	segbrk(buf, buf+4096);
}