Messages in this thread | | | From | "Ren, Qiaowei" <> | Subject | RE: [PATCH 2/5] x86, mpx: hook #BR exception handler to allocate bound tables | Date | Sun, 19 Jan 2014 12:50:14 +0000 |
| |
> -----Original Message----- > From: Steven Rostedt [mailto:rostedt@goodmis.org] > Sent: Saturday, January 18, 2014 1:15 AM > To: H. Peter Anvin > Cc: Borislav Petkov; Ren, Qiaowei; Thomas Gleixner; Ingo Molnar; > x86@kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 2/5] x86, mpx: hook #BR exception handler to allocate > bound tables > > On Fri, 17 Jan 2014 08:51:03 -0800 > "H. Peter Anvin" <hpa@zytor.com> wrote: > > > On 01/17/2014 08:47 AM, Borislav Petkov wrote: > > > > > > Right, so Steve and I played a couple of scenarios in IRC with this. > > > So #BR is comparable with #PF, AFAICT, and as expected we don't take > > > any locks when handling page faults in kernel space as we might deadlock. > > > > > > Now, what happens if a thread is sleeping on some lock down that > > > GFP_KERNEL allocation path and another thread gets a #BR and goes > > > that same mmap_pgoff path and tries to grab that same lock? > > > > It goes to sleep. Same as if we take a page fault and have to page > > something in. > > Yep, which is what I was explaining to Boris on IRC. > > > > > > Also, what happens if you take a #BR in NMI context, say the NMI > > > handler? > > > > You should never, ever do that. We should never take a #BR in the > > kernel, full stop -- if we do it is panic time. > > Right. It should actually do what a page fault does too. If we page fault in NMI, > it reports it and crashes. > > > > > > All I'm trying to say is, it might not be such a good idea to sleep > > > in a fault handler... > > > > A fault handler from user space is really nothing other than a > > different kind of system call. It is nothing magic about it. > > Exactly. I was saying that #BR should be just like #PF, as it can detect bugs in > the kernel too. The first thing the handler should do is check to see if the fault > occurred in userspace or kernel space. If it is userspace, then there's no > restrictions. If it is kernel space then we should do the bare minimum to report > the bug and then kill whatever task happened to do it. > Yes. I guess I know what you mean and this detection is necessary.
The following check should be added into at the beginning of this handler.
if (!user_mode(regs)) { if (!fixup_exception(regs)) { tsk->thread.error_code = error_code; tsk->thread.trap_nr = trapnr; die(str, regs, error_code); } }
Thanks, Qiaowei
| |