On Tue, 2017-01-03 at 08:41 -0800, Dave Hansen wrote:
On 12/27/2016 02:33 PM, Ricardo Neri wrote:
diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c index 6a75a75..71681d0 100644 --- a/arch/x86/mm/mpx.c +++ b/arch/x86/mm/mpx.c @@ -120,6 +120,13 @@ static int get_reg_offset(struct insn *insn, struct pt_regs *regs,
case REG_TYPE_BASE: regno = X86_SIB_BASE(insn->sib.value);
if (regno == 5 && X86_MODRM_RM(insn->modrm.value) == 0) {
WARN_ONCE(1, "An explicit displacement is required when %sBP used as SIB base.",
(IS_ENABLED(CONFIG_X86_64) && insn->x86_64) ?
"R13 or R" : "E");
return -EINVAL;
}
Now that I've read the cover letter, I see what's going on. This should not warn -- user code can easily trigger this deliberately.
OK, I'll remove it. Are you concerned about the warning printing the calltrace, even only once?
Yes. We don't let userspace spam the kernel, even once. If we have a couple thousand "only once" places, then userspace can overwhelm the kernel log.
This makes sense. I was not looking at it this way.
Also, this needs a much better description of what's going on in the code. Could you add a comment explaining what's going on, and why regno==5, etc...?
I will add more comments.
Thanks! Ricardo