From: David H. <dho...@re...> - 2000-11-23 16:50:29
|
> user code to emulate unaligned accesses, which can be written as someone > actually found a userspace programs that needs them. We have the code anyway to fix up kernel accesses. > user -> kernel -> user shouldn't be significantly slower than > user -> kernel -> SIGBUS handler -> user Surely it should be somewhat faster. > I think you're wrong. there are many insns the kernel doesn't use (which > your patch doesn't emulate either), and the kernel version needs to validate > every pointer it dereferences. All the instructions emulated in my patch can be produced by gcc (there aren't that many really, it's just that delay slots complicate matters). The multiply from memory is admittedly one I don't emulate, but I don't think that's likely to occur - but I could be wrong. Furthermore, the hard part of pointer validation is handled by the MMU, and so incurs minimal penalty. The unfortunate part is that gcc can't handle two different exits from an asm statement, and so it can't be made to goto a label somewhere else in the function upon faulting. > It never passes it back to userspace. it generates a segv for userspace. > it is a system call return value, and there is no system call directly > involved here. Indeed... it's entirely internal > use "1". use SIGSEGV directly. use any other random value as long as it > makes sense as a constant. EFAULT doesn't. Actually, EFAULT just means "Bad Address" doesn't it? > > > It just means your comment is wrong. > /* Argh. Fault on the instruction itself. > This should never happen non-SMP > */ > > > No... The comment says that the "if" fires if the attempt to read the > > Yes. It can happen on UP systems. No, I don't think it should. I discussed it with David Woodhouse who actually wrote the comment, and he pointed out that the only reason we should ever fault there is if on an SMP system, another CPU unmapped the page at an inconvenient time. If the page wasn't there in the first place, then we'll not come in through do_address_error() since there wasn't an unaligned access (as the CPU won't have been able to read the instruction to see if it was an unaligned access). David Howells |
From: Philipp R. <pr...@pa...> - 2000-11-23 17:30:47
|
On Thu, Nov 23, 2000 at 04:50:26PM +0000, David Howells wrote: > > user code to emulate unaligned accesses, which can be written as someone > > actually found a userspace programs that needs them. > > We have the code anyway to fix up kernel accesses. no. we have the code for the few insns actually used by the kernel, and it can go away as soon as someone convinced DaveM having the network stack use unaligned accesses is silly. > > I think you're wrong. there are many insns the kernel doesn't use (which > > your patch doesn't emulate either), and the kernel version needs to validate > > every pointer it dereferences. > > All the instructions emulated in my patch can be produced by gcc (there aren't > that many really, it's just that delay slots complicate matters). The multiply yes, but they're not all the instructions gcc generates. > from memory is admittedly one I don't emulate, but I don't think that's likely > to occur - but I could be wrong. Do it correctly or don't do it at all. "We emulate all unaligned memory accesses except 3 or 4 we didn't bother with" isn't a sensible rule. > Furthermore, the hard part of pointer validation is handled by the MMU, and so True for userspace emulation as well. > incurs minimal penalty. The unfortunate part is that gcc can't handle two > different exits from an asm statement, and so it can't be made to goto a label > somewhere else in the function upon faulting. indeed. __label__ a; asm volatile("bra %0" : : "l" (a)); a: would be cool. > > It never passes it back to userspace. it generates a segv for userspace. > > it is a system call return value, and there is no system call directly > > involved here. > > Indeed... it's entirely internal which makes using a syscall return value constant confusing. > > use "1". use SIGSEGV directly. use any other random value as long as it > > makes sense as a constant. EFAULT doesn't. > > Actually, EFAULT just means "Bad Address" doesn't it? In the context of system calls, yes. > > > > It just means your comment is wrong. > > /* Argh. Fault on the instruction itself. > > This should never happen non-SMP > > */ > > > > > No... The comment says that the "if" fires if the attempt to read the > > > > Yes. It can happen on UP systems. > > No, I don't think it should. I discussed it with David Woodhouse who actually > wrote the comment, and he pointed out that the only reason we should ever > fault there is if on an SMP system, another CPU unmapped the page at an which isn't true. see my earlier mail. < > inconvenient time. If the page wasn't there in the first place, then we'll not > come in through do_address_error() since there wasn't an unaligned access (as > the CPU won't have been able to read the instruction to see if it was an > unaligned access). address error doesn't necessarily mean unaligned access. |
From: David W. <dw...@in...> - 2000-11-23 17:56:22
|
pr...@pa... said: > no. we have the code for the few insns actually used by the kernel, > and it can go away as soon as someone convinced DaveM having the > network stack use unaligned accesses is silly. Not going to happen, mainly because having the network stack use unaligned access _isn't_ silly. Unaligned accesses in the network stack are a _very_ uncommon case, but a case which we have be able to deal with nonetheless. Having checks for alignment in all the networking fast paths would be insane. What we're doing here is very similar to what we do in __copy_user() - optimising the common case and letting the CPU raise an exception so we can fix up the rarer cases. Doing the check in software is silly when the CPU can do it for you instead. pr...@pa... said: > Do it correctly or don't do it at all. "We emulate all unaligned > memory accesses except 3 or 4 we didn't bother with" isn't a sensible > rule. It's correct, or certainly getting there. It's not _complete_ but I'm sure someone will contribute the missing insns, even if _we_ don't actually get round to it. pr...@pa... said: > which makes using a syscall return value constant confusing. It doesn't confuse me. Standard error numbers are used _internally_ throughout the kernel, even where they're not actually being returned to userspace. pr...@pa... said: > address error doesn't necessarily mean unaligned access. P485 of the SH-3/SH-3E/SH3-DSP Programming Manual says: "Address errors are caused by instruction fetches and by data reads or writes. Fetching an instruction from an odd address or fetching an instruction from an on-chip peripheral register causes an instruction fetch address error. Accessing word data from other than a word boundary, accessing longword data from other than a longword boundary, and accessing an on-chip peripheral register 8-bit space by longword cause a read or write address error." OK, so PC could be the address of an on-chip peripheral register. Sick, but vaguely possible. I'd still say that such is a 'should never happen' event, but I have no particular attachment to that comment so if it really offends you it can go. Are there other causes of address error in SH-[45]? Perhaps we also need to explicitly check for the case of a wrongly-sized access to the on-chip peripheral space. Simply refusing to fix up any exceptions on accesses to on-chip peripheral space should be sufficient. -- dwmw2 |
From: Philipp R. <pr...@pa...> - 2000-11-23 18:13:36
|
On Thu, Nov 23, 2000 at 05:55:40PM +0000, David Woodhouse wrote: > > pr...@pa... said: [dropped the network stack discussion, as it's offtopic here] > pr...@pa... said: > > Do it correctly or don't do it at all. "We emulate all unaligned > > memory accesses except 3 or 4 we didn't bother with" isn't a sensible > > rule. > > It's correct, or certainly getting there. It's not _complete_ but I'm sure generating SEGV for unknown unaligned instructions isn't correct. > someone will contribute the missing insns, even if _we_ don't actually get > round to it. I think that would add prohibitively large amounts of code, and quite possibly new security holes. > pr...@pa... said: > > which makes using a syscall return value constant confusing. > > It doesn't confuse me. Standard error numbers are used _internally_ > throughout the kernel, even where they're not actually being returned to > userspace. I can remember no case where it's impossible for userspace to see the error, except for module_init until a few versions ago. > Are there other causes of address error in SH-[45]? unprivileged accesses to 0x80000000 .. 0xffffffff |
From: NIIBE Y. <gn...@ch...> - 2000-11-24 00:15:42
|
Philipp Rumpf wrote: > On Thu, Nov 23, 2000 at 04:50:26PM +0000, David Howells wrote: > > > user code to emulate unaligned accesses, which can be written as someone > > > actually found a userspace programs that needs them. > > > > We have the code anyway to fix up kernel accesses. > > no. we have the code for the few insns actually used by the kernel, and it > can go away as soon as someone convinced DaveM having the network stack use > unaligned accesses is silly. I also think that supporting kernel unaligned access is not good idea, but Jesper said that DoS possibility (we should check), which I care. > Do it correctly or don't do it at all. "We emulate all unaligned memory > accesses except 3 or 4 we didn't bother with" isn't a sensible rule. Agreed. I installed the patch hoping we could do it correctly. If it's difficult, and we can't see enough rationale about DoS possibility, it's better to drop the kernel unaligned access support. Local user access is more likely than network attack. Network attack can be handled by other devices and feature (router, firewall rules), so we should have more care for local user access. -- |
From: Philipp R. <pr...@pa...> - 2000-11-24 11:55:48
|
On Fri, Nov 24, 2000 at 09:15:39AM +0900, NIIBE Yutaka wrote: > Philipp Rumpf wrote: > > On Thu, Nov 23, 2000 at 04:50:26PM +0000, David Howells wrote: > > > > user code to emulate unaligned accesses, which can be written as someone > > > > actually found a userspace programs that needs them. > > > > > > We have the code anyway to fix up kernel accesses. > > > > no. we have the code for the few insns actually used by the kernel, and it > > can go away as soon as someone convinced DaveM having the network stack use > > unaligned accesses is silly. > > I also think that supporting kernel unaligned access is not good idea, > but Jesper said that DoS possibility (we should check), which I care. We currently need kernel unaligned accesses for weird network protocols; there's also a remote exploit possibility. As for the DoS, I think what Jesper meant was that if you do emulate unaligned accesses, userspace can keep doing kernel->user->kernel transitions, which is true anyway. > > Do it correctly or don't do it at all. "We emulate all unaligned memory > > accesses except 3 or 4 we didn't bother with" isn't a sensible rule. > > Agreed. > > I installed the patch hoping we could do it correctly. If it's > difficult, and we can't see enough rationale about DoS possibility, > it's better to drop the kernel unaligned access support. I think we have four groups of accesses we might emulate or not: 1. kernel accesses to kernel memory 2. kernel accesses to user memory 3. user accesses to user memory, using "standard" instructions 4. user accesses to user memory, using non-standard instructions We definitely need to emulate 1) - at least unless someone convinces the network guys to drop that requirement. I don't think we need to emulate 2) - there is nothing wrong, I think, with returning -EFAULT for unaligned user space pointers passed to syscalls that try get_user or put_user (we'd lose the atomicity anyway). Emulating 2) adds some bloat to the unaligned access handler (__copy_user instead of memcpy). I don't think we need to emulate 3) either, and I think it's a terrible precedent to emulate 3) but not 4); emulating 3) adds yet another bit of bloat to the unaligned access handler (copy_*_user instead of __copy_user). emulating 4) means adding support for many new instructions, and I'm not sure all of them can be emulated satisfactorily. > Local user access is more likely than network attack. Network attack > can be handled by other devices and feature (router, firewall rules), > so we should have more care for local user access. I don't think there are any security holes left in the patch posted yesterday (which emulates 1, 2, and 3), but I need to have another look. |
From: Jesper S. <js...@re...> - 2000-11-24 13:10:36
|
>>>>> "Philipp" == Philipp Rumpf <pr...@pa...> writes: Philipp> On Fri, Nov 24, 2000 at 09:15:39AM +0900, NIIBE Yutaka wrote: >> Philipp Rumpf wrote: > On Thu, Nov 23, 2000 at 04:50:26PM +0000, >> David Howells wrote: > > > user code to emulate unaligned accesses, >> which can be written as someone > > > actually found a userspace >> programs that needs them. > > > > We have the code anyway to fix >> up kernel accesses. > > no. we have the code for the few insns >> actually used by the kernel, and it > can go away as soon as >> someone convinced DaveM having the network stack use > unaligned >> accesses is silly. >> >> I also think that supporting kernel unaligned access is not good >> idea, but Jesper said that DoS possibility (we should check), which >> I care. Philipp> We currently need kernel unaligned accesses for weird network Philipp> protocols; there's also a remote exploit possibility. As for Philipp> the DoS, I think what Jesper meant was that if you do emulate Philipp> unaligned accesses, userspace can keep doing Philipp> kernel->user->kernel transitions, which is true anyway. That's what I meant. But on second thought, I guess the exit path goes past the schedule code, doesn't it? If so, it's not an issue. My concern was if there was a separate path in and out of the kernel to the fixup code - but that's probably not the case. Jesper |
From: NIIBE Y. <gn...@ch...> - 2000-11-25 01:36:55
|
Philipp Rumpf wrote: > I think we have four groups of accesses we might emulate or not: > 1. kernel accesses to kernel memory > 2. kernel accesses to user memory > 3. user accesses to user memory, using "standard" instructions > 4. user accesses to user memory, using non-standard instructions Thanks for the summary. My opinion is we only need #1. > We definitely need to emulate 1) - at least unless someone convinces the > network guys to drop that requirement. Is there anyone try to use ISOFS? Takashi Yoshii said there're plenty of code which might cause unaligned access. For #2, #3, and #4, it would be interesting problem to solve, technically, to supports the feature. However, when people (of user space application) rely on such a feature, in the long run, bad quality would remain forever. At least, we should output warning message when handling userspace unaligned access. -- |
From: Mitch D. <md...@po...> - 2000-11-26 01:43:41
|
NIIBE Yutaka wrote: > > Philipp Rumpf wrote: > > I think we have four groups of accesses we might emulate or not: > > 1. kernel accesses to kernel memory > > 2. kernel accesses to user memory > > 3. user accesses to user memory, using "standard" instructions > > 4. user accesses to user memory, using non-standard instructions > > Thanks for the summary. > > My opinion is we only need #1. ... > At least, we should output warning message when handling userspace > unaligned access. I agree. If there was a non-trivial body of userspace code which did unaligned access, I would consider #3 and #4 a necessary but evil kludge. But we don't, and I hope we never do. And I'd like to make sure we never do by not supporting the emulation of userspace unaligned instructions. (Just do a printk and a SEGV). Just my two cents worth. You can collect the other 2 cents here. http://www.geocrawler.com/lists/3/SourceForge/3076/50/4621975/ At the time the response to this was, "it's only for kernel mode". Now it appears it's not, and my original comments still stand. Regards, Mitch. |
From: Philipp R. <pr...@pa...> - 2000-11-26 02:15:08
|
On Sun, Nov 26, 2000 at 12:40:28PM +1100, Mitch Davis wrote: > to make sure we never do by not supporting the emulation of userspace > unaligned instructions. (Just do a printk and a SEGV). SIGBUS is the traditional signal. |
From: Mitch D. <md...@po...> - 2000-11-26 04:06:31
|
Philipp Rumpf wrote: > > On Sun, Nov 26, 2000 at 12:40:28PM +1100, Mitch Davis wrote: > > to make sure we never do by not supporting the emulation of userspace > > unaligned instructions. (Just do a printk and a SEGV). > > SIGBUS is the traditional signal. Right you are. Regards, Mitch. |