| |||||||||||||
Stable kernel 2.6.27.5Stable kernel 2.6.27.5Posted Nov 9, 2008 16:27 UTC (Sun) by PaXTeam (subscriber, #24616)In reply to: Stable kernel 2.6.27.5 by tialaramex Parent article: Stable kernel 2.6.27.5
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.
(Log in to post comments)
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.
|
|||||||||||||