lkml.org 
[lkml]   [2019]   [Apr]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: x86/paravirt: Detect over-sized patching bugs in paravirt_patch_call()

    * Peter Zijlstra <peterz@infradead.org> wrote:

    > On Thu, Apr 25, 2019 at 11:50:39AM +0200, Ingo Molnar wrote:
    > >
    > > * Peter Zijlstra <peterz@infradead.org> wrote:
    > >
    > > > On Thu, Apr 25, 2019 at 11:17:17AM +0200, Ingo Molnar wrote:
    > > > > It basically means that we silently won't do any patching and the kernel
    > > > > will crash later on in mysterious ways, because paravirt patching is
    > > > > usually relied on.
    > > >
    > > > That's OK. The compiler emits an indirect CALL/JMP to the pv_ops
    > > > structure contents. That _should_ stay valid and function correctly at
    > > > all times.
    > >
    > > It might result in a correctly executing kernel in terms of code
    > > generation, but it doesn't result in a viable kernel: some of the places
    > > rely on the patching going through and don't know what to do when it
    > > doesn't and misbehave or crash in interesting ways.
    > >
    > > Guess how I know this. ;-)
    >
    > What sites would that be? It really should work AFAIK.

    So for example I tried to increasing the size of one of the struct
    patch_xxl members:

    --- a/arch/x86/kernel/paravirt_patch.c
    +++ b/arch/x86/kernel/paravirt_patch.c
    @@ -28,7 +28,7 @@ struct patch_xxl {
    const unsigned char irq_restore_fl[2];
    # ifdef CONFIG_X86_64
    const unsigned char cpu_wbinvd[2];
    - const unsigned char cpu_usergs_sysret64[6];
    + const unsigned char cpu_usergs_sysret64[60];
    const unsigned char cpu_swapgs[3];
    const unsigned char mov64[3];
    # else
    Which with the vanilla kernel crashes on boot much, much later:

    [ 2.478026] PANIC: double fault, error_code: 0x0

    But in any case, even if many of the others will work if the patching
    fails silently, is there any case where we'd treat patching failure as an
    acceptable case?

    BUG_ON() in paravirt kernels is an easily debuggable condition and beats
    the above kinds of symptoms. But I can turn it into a WARN_ON_ONCE() if
    you think that's better?

    Thanks,

    Ingo

    \
     
     \ /
      Last update: 2019-04-25 12:58    [W:3.560 / U:0.112 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site