lkml.org 
[lkml]   [2022]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.15 238/247] powerpc/32: Fix overread/overwrite of thread_struct via ptrace
    Date
    From: Michael Ellerman <mpe@ellerman.id.au>

    commit 8e1278444446fc97778a5e5c99bca1ce0bbc5ec9 upstream.

    The ptrace PEEKUSR/POKEUSR (aka PEEKUSER/POKEUSER) API allows a process
    to read/write registers of another process.

    To get/set a register, the API takes an index into an imaginary address
    space called the "USER area", where the registers of the process are
    laid out in some fashion.

    The kernel then maps that index to a particular register in its own data
    structures and gets/sets the value.

    The API only allows a single machine-word to be read/written at a time.
    So 4 bytes on 32-bit kernels and 8 bytes on 64-bit kernels.

    The way floating point registers (FPRs) are addressed is somewhat
    complicated, because double precision float values are 64-bit even on
    32-bit CPUs. That means on 32-bit kernels each FPR occupies two
    word-sized locations in the USER area. On 64-bit kernels each FPR
    occupies one word-sized location in the USER area.

    Internally the kernel stores the FPRs in an array of u64s, or if VSX is
    enabled, an array of pairs of u64s where one half of each pair stores
    the FPR. Which half of the pair stores the FPR depends on the kernel's
    endianness.

    To handle the different layouts of the FPRs depending on VSX/no-VSX and
    big/little endian, the TS_FPR() macro was introduced.

    Unfortunately the TS_FPR() macro does not take into account the fact
    that the addressing of each FPR differs between 32-bit and 64-bit
    kernels. It just takes the index into the "USER area" passed from
    userspace and indexes into the fp_state.fpr array.

    On 32-bit there are 64 indexes that address FPRs, but only 32 entries in
    the fp_state.fpr array, meaning the user can read/write 256 bytes past
    the end of the array. Because the fp_state sits in the middle of the
    thread_struct there are various fields than can be overwritten,
    including some pointers. As such it may be exploitable.

    It has also been observed to cause systems to hang or otherwise
    misbehave when using gdbserver, and is probably the root cause of this
    report which could not be easily reproduced:
    https://lore.kernel.org/linuxppc-dev/dc38afe9-6b78-f3f5-666b-986939e40fc6@keymile.com/

    Rather than trying to make the TS_FPR() macro even more complicated to
    fix the bug, or add more macros, instead add a special-case for 32-bit
    kernels. This is more obvious and hopefully avoids a similar bug
    happening again in future.

    Note that because 32-bit kernels never have VSX enabled the code doesn't
    need to consider TS_FPRWIDTH/OFFSET at all. Add a BUILD_BUG_ON() to
    ensure that 32-bit && VSX is never enabled.

    Fixes: 87fec0514f61 ("powerpc: PTRACE_PEEKUSR/PTRACE_POKEUSER of FPR registers in little endian builds")
    Cc: stable@vger.kernel.org # v3.13+
    Reported-by: Ariel Miculas <ariel.miculas@belden.com>
    Tested-by: Christophe Leroy <christophe.leroy@csgroup.eu>
    Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
    Link: https://lore.kernel.org/r/20220609133245.573565-1-mpe@ellerman.id.au
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    arch/powerpc/kernel/ptrace/ptrace-fpu.c | 20 ++++++++++++++------
    arch/powerpc/kernel/ptrace/ptrace.c | 3 +++
    2 files changed, 17 insertions(+), 6 deletions(-)

    --- a/arch/powerpc/kernel/ptrace/ptrace-fpu.c
    +++ b/arch/powerpc/kernel/ptrace/ptrace-fpu.c
    @@ -17,9 +17,13 @@ int ptrace_get_fpr(struct task_struct *c

    #ifdef CONFIG_PPC_FPU_REGS
    flush_fp_to_thread(child);
    - if (fpidx < (PT_FPSCR - PT_FPR0))
    - memcpy(data, &child->thread.TS_FPR(fpidx), sizeof(long));
    - else
    + if (fpidx < (PT_FPSCR - PT_FPR0)) {
    + if (IS_ENABLED(CONFIG_PPC32))
    + // On 32-bit the index we are passed refers to 32-bit words
    + *data = ((u32 *)child->thread.fp_state.fpr)[fpidx];
    + else
    + memcpy(data, &child->thread.TS_FPR(fpidx), sizeof(long));
    + } else
    *data = child->thread.fp_state.fpscr;
    #else
    *data = 0;
    @@ -39,9 +43,13 @@ int ptrace_put_fpr(struct task_struct *c

    #ifdef CONFIG_PPC_FPU_REGS
    flush_fp_to_thread(child);
    - if (fpidx < (PT_FPSCR - PT_FPR0))
    - memcpy(&child->thread.TS_FPR(fpidx), &data, sizeof(long));
    - else
    + if (fpidx < (PT_FPSCR - PT_FPR0)) {
    + if (IS_ENABLED(CONFIG_PPC32))
    + // On 32-bit the index we are passed refers to 32-bit words
    + ((u32 *)child->thread.fp_state.fpr)[fpidx] = data;
    + else
    + memcpy(&child->thread.TS_FPR(fpidx), &data, sizeof(long));
    + } else
    child->thread.fp_state.fpscr = data;
    #endif

    --- a/arch/powerpc/kernel/ptrace/ptrace.c
    +++ b/arch/powerpc/kernel/ptrace/ptrace.c
    @@ -446,4 +446,7 @@ void __init pt_regs_check(void)
    * real registers.
    */
    BUILD_BUG_ON(PT_DSCR < sizeof(struct user_pt_regs) / sizeof(unsigned long));
    +
    + // ptrace_get/put_fpr() rely on PPC32 and VSX being incompatible
    + BUILD_BUG_ON(IS_ENABLED(CONFIG_PPC32) && IS_ENABLED(CONFIG_VSX));
    }

    \
     
     \ /
      Last update: 2022-06-13 15:25    [W:2.437 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site