On 09/12/2014 06:10 AM, Thomas Gleixner wrote: >> >> I'm not wedded to that concept, by the way, but using the generic parser had a >> whole bunch of its own problems, including the fact that you're getting bytes >> from user space. > > Errm. The instruction decoder does not even know about user space. > > u8 buf[MAX_INSN_SIZE]; > > memset(buf, 0, MAX_INSN_SIZE); > if (copy_from_user(buf, addr, MAX_INSN_SIZE)) > return 0; > > insn_init(insn, buf, is_64bit(current)); > > /* Process the entire instruction */ > insn_get_length(insn); > > /* Decode the faulting address */ > return mpx_get_addr(insn, regs); > > I really can't see why that should not work. insn_get_length() > retrieves exactly the information which is required to call > mpx_get_addr(). > > Sure it might be a bit slower because the generic decoder does a bit > more than the mpx private sauce, but this happens in the context of a > bounds violation and it really does not matter at all whether SIGSEGV > is delivered 5 microseconds later or not. > > The only difference is the insn->limit handling in the MPX > decoder. The existing decoder has a limit check of: > > #define MAX_INSN_SIZE 16 > > and MPX private one makes that > > #define MAX_MPX_INSN_SIZE 15 > > and limits it runtime further to: > > MAX_MPX_INSN_SIZE - bytes_not_copied_from_user_space; > > This is beyond silly, really. If we cannot copy 16 bytes from user > space, why bother in dealing with a partial copy at all. > The correct limit is 15 bytes, not anything else, so this is a bug in the existing decoder. A sequence of bytes longer than 15 bytes will #UD, regardless of being "otherwise valid". Keep in mind the instruction may not be aligned, and you could fit an instruction plus a jump and still overrun a page in 15 bytes. > Aside of that the existing decoder handles the 32bit app on a 64bit > kernel already correctly while the extra magic MPX decoder does > not. It just adds some magically optimized and different copy of the > existing decoder for exactly ZERO value. > >> It might be worthwhile to compare the older patchset which did use the generic >> parser to make sure that it actually made sense. > > I can't find such a thing. The first version I found contains an even > more convoluted private parser. Intelnal mail perhaps? Yes, I suspect so. -hpa -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>