Messages in this thread | | | Date | Mon, 14 Dec 2015 11:18:23 -0700 | Subject | Re: [PATCH 1/1] Fix int1 recursion when no perf_bp_event is registeredy | From | Jeff Merkey <> |
| |
On 12/14/15, Andy Lutomirski <luto@amacapital.net> wrote: > On Mon, Dec 14, 2015 at 9:52 AM, Jeff Merkey <linux.mdb@gmail.com> wrote: >> On 12/14/15, Ingo Molnar <mingo@kernel.org> wrote: >>> >>> * Jeff Merkey <linux.mdb@gmail.com> wrote: >>> >>>> On 12/14/15, Ingo Molnar <mingo@kernel.org> wrote: >>>> > >>>> > A: Because it messes up the order in which people normally read text. >>>> > Q: Why is top-posting such a bad thing? >>>> > A: Top-posting. >>>> > Q: What is the most annoying thing in e-mail? >>>> > >>>> > * Jeff Merkey <linux.mdb@gmail.com> wrote: >>>> > >>>> >> I trigger it by writing to the dr7 and dr1, 2, 3 or four register >>>> >> and >>>> >> set an >>>> >> execute breakpoint without going through arch_install_hw_breakpoint. >>>> >> When >>>> >> the breakpoint fires, the system crashes and hangs on the processor >>>> >> stuck in >>>> >> an endless loop inside the int1 handler in hw_breakpoint.c -- >>>> > >>>> > What is still not clear to me, can you trigger the hang not via some >>>> > special >>>> > >>>> > kernel driver that goes outside regular APIs and messes with the >>>> > state >>>> > of the >>>> > debug registers, but via the proper access methods, i.e. various >>>> > user-space >>>> > ABIs? >>>> >>>> Any process that can get access to the debug registers can trigger this >>>> condition. [...] >>> >>> A process on an unmodified Linux kernel can only modify debug registers >>> via >>> the >>> proper APIs: >>> >>>> [...] As it stands, if restricted to the established API in >>>> hw_breakpoint.c >>>> this bug should not occur unless someone triggers an errant breakpoint. >>>> [...] >>> >>> So am I interpreting your report correctly: >>> >>> "If the Linux kernel is modified to change debug registers without >>> using >>> the >>> proper APIs (such as loading a module that changes hardware registers >>> in >>> a raw >>> fashion), things may break and a difficult to debug hang may occur." >>> >>> right? >>> >>> This key piece of information should have been part of the original >>> report. >>> >>> So I'm wondering, why does your module modify debug registers in a raw >>> fashion? >>> Why doesn't it use the proper APIs? >>> >>> Thanks, >>> >>> Ingo >>> >> >> Hi Ingo, >> >> This will be a lengthy reply to properly explain this to you. First >> some fundamental assumptions to clear up. >> >> 1. The MDB Debugger Module does not cause this problem. This is an >> existing bug in the kernel in an exception code path. > >> 8. If any process or module sets a breakpoint outside of linux >> breakpoint API in this code path the system will crash. Its A BUG, >> and it's been in linux 13 years. I am certain people have seen it >> while running perf stuff but since it provides no diagnostic info, >> someone would just reset the system. > > Putting "BUG" in caps doesn't make it so. What's wrong with it? > >> 9. This breakpoint API needs to be rewritten to be global breakpoint >> aware, have an on/off switch so when a debugger enters an int1 >> exception, breakpoints are globally disabled (a requirement), among >> other things. > > A "requirement" for what? > >> >> The patch simply fixes the bug in the int handler that will cause a >> lockup. The perf event system, kgdb, kdb, and any one of a number of >> programs can trigger this bug, and probably have. People would blame >> the debugger when its a bug in the int handler. > > You ignored feedback from me and from tglx, and you still haven't > explained why this is a bug in the first place.
It crashes the system -- crash = bug -- right?
Maybe the code could > degrade more gracefully if you use it wrong, but the int1 handler and > the rest of the kernel are very much aware of each other, and the int1 > handler's failure to do what something that isn't in the kernel wants > it to do isn't a bug. > > If you submit a clean patch to improve robustness of the handler and > if the new code is at least as clean as the old code, that might be a > different story. > > --Andy >
| |