lkml.org 
[lkml]   [2020]   [May]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 12/23] bpf: handle the compat string in bpf_trace_copy_string better
    On Thu, 21 May 2020 17:22:50 +0200 Christoph Hellwig <hch@lst.de> wrote:

    > User the proper helper for kernel or userspace addresses based on
    > TASK_SIZE instead of the dangerous strncpy_from_unsafe function.
    >
    > ...
    >
    > --- a/kernel/trace/bpf_trace.c
    > +++ b/kernel/trace/bpf_trace.c
    > @@ -331,8 +331,11 @@ static void bpf_trace_copy_string(char *buf, void *unsafe_ptr, char fmt_ptype,
    > switch (fmt_ptype) {
    > case 's':
    > #ifdef CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
    > - strncpy_from_unsafe(buf, unsafe_ptr, bufsz);
    > - break;
    > + if ((unsigned long)unsafe_ptr < TASK_SIZE) {
    > + strncpy_from_user_nofault(buf, user_ptr, bufsz);
    > + break;
    > + }
    > + fallthrough;
    > #endif
    > case 'k':
    > strncpy_from_kernel_nofault(buf, unsafe_ptr, bufsz);

    Another user of strncpy_from_unsafe() has popped up in linux-next's
    bpf. I did the below, but didn't try very hard - it's probably wrong
    if CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE=n?

    Anyway, please take a look at all the bpf_trace.c changes in
    linux-next.


    From: Andrew Morton <akpm@linux-foundation.org>
    Subject: bpf:bpf_seq_printf(): handle potentially unsafe format string better

    User the proper helper for kernel or userspace addresses based on
    TASK_SIZE instead of the dangerous strncpy_from_unsafe function.

    Cc: Christoph Hellwig <hch@lst.de>
    Cc: Alexei Starovoitov <ast@kernel.org>
    Cc: Daniel Borkmann <daniel@iogearbox.net>
    Cc: "H. Peter Anvin" <hpa@zytor.com>
    Cc: Ingo Molnar <mingo@elte.hu>
    Cc: Masami Hiramatsu <mhiramat@kernel.org>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    ---

    kernel/trace/bpf_trace.c | 13 ++++++++++---
    1 file changed, 10 insertions(+), 3 deletions(-)

    --- a/kernel/trace/bpf_trace.c~xxx
    +++ a/kernel/trace/bpf_trace.c
    @@ -588,15 +588,22 @@ BPF_CALL_5(bpf_seq_printf, struct seq_fi
    }

    if (fmt[i] == 's') {
    + void *unsafe_ptr;
    +
    /* try our best to copy */
    if (memcpy_cnt >= MAX_SEQ_PRINTF_MAX_MEMCPY) {
    err = -E2BIG;
    goto out;
    }

    - err = strncpy_from_unsafe(bufs->buf[memcpy_cnt],
    - (void *) (long) args[fmt_cnt],
    - MAX_SEQ_PRINTF_STR_LEN);
    + unsafe_ptr = (void *)(long)args[fmt_cnt];
    + if ((unsigned long)unsafe_ptr < TASK_SIZE) {
    + err = strncpy_from_user_nofault(
    + bufs->buf[memcpy_cnt], unsafe_ptr,
    + MAX_SEQ_PRINTF_STR_LEN);
    + } else {
    + err = -EFAULT;
    + }
    if (err < 0)
    bufs->buf[memcpy_cnt][0] = '\0';
    params[fmt_cnt] = (u64)(long)bufs->buf[memcpy_cnt];
    _
    \
     
     \ /
      Last update: 2020-05-28 04:04    [W:4.187 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site