yes:
if((ulong)name < KZERO){
validaddr((ulong)name, 1, 0);
if(!dup)
print("warning: validname called from %lux with user
pointer", pc);
p = name;
t = BY2PG-((ulong)p&(BY2PG-1));
while((ename=vmemchr(p, 0, t)) == nil){
-> p += t;
t = BY2PG;
}
}else
when moving p to the start of the next page... it is not checked that this
address is
valid as vmemchr() assumes the start address to be already checked, and it will
crash when vmemchr() touches it on the next round. vmemchr() will successive
check pages if the pointer to pointer + len span page boundries anyway so the
while is not really needed:
name = aname;
if((ulong)name < KZERO){
validaddr((ulong)name, 1, 0);
if(!dup)
print("warning: validname called from %lux with user
pointer", pc);
ename = vmemchr(name, 0, (1<<16));
}else
--
cinap
--- Begin Message ---
On Sat, 1 Aug 2009, Russ Cox wrote:
calling vmemchr assumes that the memory isn't being changed
by some other proc mapping the same page. if you find the
NUL in one pass and then call strcpy or strlen on the pointer
later, the other proc might have pulled the NUL in the interim.
With you so far.
there is a function in the kernel called validnamedup
that both validates a string argument and at the same time
makes an in-kernel-memory copy. it's the easiest safe
way to handle strings passed to the kernel. namec uses
it and luckily almost every string pointer passed to the kernel
ends up being interpreted by namec. exec is an exception.
sysstat() uses namec in what I believe is considered to be the correct
fashion:
959 validaddr(arg[0], 1, 0);
960 c = namec((char*)arg[0], Aaccess, 0, 0);
namec() then calls validanamedup() which calls validname0(), which appears
to do the right thing. However, the following reliably crashes the kernel:
1 #include <u.h>
2 #include <libc.h>
3
4 #define SEGBASE (char*)0x40000000
5 #define SEGSIZE 4096
6
7 int main() {
8 uchar buf[128];
9 segattach(0, "shared", SEGBASE, SEGSIZE);
10 *(char*)(SEGBASE + SEGSIZE - 1) = 'a';
11 stat((char*)SEGBASE + SEGSIZE - 1, buf, sizeof(buf));
12 return 0;
13 }
This suggests to me that something more unpleasant is afoot. Perhaps
validname0() is incorrect in some way?
when i was working on 9vx, i rewrote exec to remove
crashes like this one as well as a handful of other bugs.
the code is at
http://code.swtch.com/vx32/src/tip/src/9vx/a/sysproc.c#cl-220
and could easily be dropped back into plan 9.
russ
-- Elly
--- End Message ---
