Recent Features
| |||||||||||||
Stable kernel 2.6.27.5Stable kernel 2.6.27.5Posted Nov 9, 2008 14:20 UTC (Sun) by tialaramex (subscriber, #21167)In reply to: Stable kernel 2.6.27.5 by mb Parent article: Stable kernel 2.6.27.5
I would guess that PaXTeam has spotted that the i386 code is different to the x86-64 code here. The i386 code is easier to understand, because it essentially marks out boundaries such that any 32-bit access will be inside the kernel stack. On the other hand it's a bit more complicated because x86 calling conventions are a bit weird. In contrast the patched x86-64 code is less explicit, forbidding an access exactly at the edge (outside) of the stack but the convention it is implementing is more straight forward.
If the pointer was somehow not to be 64-bit aligned, then the patch for x86-64 leaves open the possibility of an unaligned pointer access overlapping the edge of the stack, which would be caught by the equivalent code from the i386 kernel. My understanding is that the x86-64 ABI forbids this in general, but perhaps PaXTeam knows better, at least in regard to the kernel stacks.
(Log in to post comments)
Stable kernel 2.6.27.5 Posted Nov 9, 2008 14:31 UTC (Sun) by tialaramex (subscriber, #21167) [Link]
Oh, and this trick...
ip = *(u64 *)(fp+8);
...would obviously back-fire if e.g. (fp = stack+THREAD_SIZE - 8) but it's not clear to me that this can happen either. Maybe I need to go read more of the kernel stack code.
Stable kernel 2.6.27.5 Posted Nov 9, 2008 16:27 UTC (Sun) by PaXTeam (subscriber, #24616) [Link]
no, i haven't even looked at the i386 code, it was obviously wrong just by reading the patch. and the problem has nothing to do with alignment, it's an untrusted pointer (that's why it's bounds checked in the first place, else it wouldn't have to be), so no alignment assumptions can be made.
Stable kernel 2.6.27.5 Posted Nov 9, 2008 18:02 UTC (Sun) by tialaramex (subscriber, #21167) [Link]
It is usually helpful to look a bit further than the patch.
If you're right that there's nothing outside this function to protect the state it's inspecting, (ie the target process might start running) since that's the only way for this value to become "untrusted" then the i386 code is exactly the right place to look, since that takes the proper care of ordering e.g.
sp = p->thread.sp;
We have to stash the stack pointer because otherwise the other process can race us, changing it just after we confirm that it's inside bounds and before we dereference it. Similarly with results read from the stack, although the x86-64 code gets the latter right already.
But really I don't have enough information to make a good judgement about how much paranoia is justified. What locks is a caller of get_wchan expected to hold? Can this process structure go away from underneath us while we're executing? What about the associated stack?
Stable kernel 2.6.27.5 Posted Nov 9, 2008 19:30 UTC (Sun) by PaXTeam (subscriber, #24616) [Link]
i actually never said that the task's stack couldn't change while this code is running. in fact, quite the opposite is true, hence all the various checks in the code.
|
|||||||||||||