lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 31 Aug 2018 15:26:24 +0200 From: Jann Horn <jannh@...gle.com> To: Luc Van Oostenryck <luc.vanoostenryck@...il.com> Cc: Thomas Gleixner <tglx@...utronix.de>, kernel list <linux-kernel@...r.kernel.org>, Kees Cook <keescook@...omium.org>, bp@...e.de Subject: Re: [PATCH] x86/dumpstack: fix address space casting in show_opcodes() On Fri, Aug 31, 2018 at 10:27 AM Luc Van Oostenryck <luc.vanoostenryck@...il.com> wrote: > > On Thu, Aug 30, 2018 at 09:47:36PM +0200, Jann Horn wrote: > > I sloppily passed a kernel-typed pointer to __range_not_ok(), and sparse > > doesn't like that. > > Make `prologue` a __user pointer (to protect against accidental > > dereferences) and force-cast it to a kernel pointer when calling > > probe_kernel_read(), which will then immediately force-cast it back to a > > user pointer. > > It's a bit sad to have to do this. > __range_not_ok() explicitly requires a __user pointer (I don't know > if there is a good reason for it) but the real job is done by > __chk_range_not_ok(). Can't you use this later instead? Yeah, I guess I can do that. Will send a v2 in a bit... By the way, here are all 60 probe_kernel_read() callers: arch/arm/kernel/ftrace.c: if (probe_kernel_read(&replaced, (void *)pc, MCOUNT_INSN_SIZE)) arch/arm/kernel/kgdb.c: err = probe_kernel_read(bpt->saved_instr, (char *)bpt->bpt_addr, arch/arm64/kernel/insn.c: ret = probe_kernel_read(&val, addr, AARCH64_INSN_SIZE); arch/ia64/kernel/ftrace.c: if (probe_kernel_read(replaced, (void *)ip, MCOUNT_INSN_SIZE)) arch/ia64/kernel/ftrace.c: if (probe_kernel_read(replaced, (void *)ip, MCOUNT_INSN_SIZE)) arch/mips/kernel/kprobes.c: if ((probe_kernel_read(&prev_insn, p->addr - 1, arch/powerpc/kernel/module_64.c: if (probe_kernel_read(&magic, &stub->magic, sizeof(magic))) { arch/powerpc/kernel/module_64.c: if (probe_kernel_read(&funcdata, &stub->funcdata, sizeof(funcdata))) { arch/powerpc/kernel/trace/ftrace.c: if (probe_kernel_read(&replaced, (void *)ip, MCOUNT_INSN_SIZE)) arch/powerpc/kernel/trace/ftrace.c: if (probe_kernel_read(&op, (void *)ip, sizeof(int))) { arch/powerpc/kernel/trace/ftrace.c: if (probe_kernel_read(&op, (void *)(ip - 4), 4)) { arch/powerpc/kernel/trace/ftrace.c: if (probe_kernel_read(&op, (void *)(ip+4), MCOUNT_INSN_SIZE)) { arch/powerpc/kernel/trace/ftrace.c: if (probe_kernel_read(&op, (void *)ip, MCOUNT_INSN_SIZE)) arch/powerpc/kernel/trace/ftrace.c: if (probe_kernel_read(jmp, (void *)tramp, sizeof(jmp))) { arch/powerpc/kernel/trace/ftrace.c: if (probe_kernel_read(op, ip, sizeof(op))) arch/powerpc/kernel/trace/ftrace.c: if (probe_kernel_read(&op, (void *)ip, MCOUNT_INSN_SIZE)) arch/powerpc/kernel/trace/ftrace.c: if (probe_kernel_read(&op, (void *)ip, sizeof(int))) { arch/powerpc/perf/core-book3s.c: if (probe_kernel_read(&instr, (void *)addr, sizeof(instr))) arch/riscv/kernel/ftrace.c: if (probe_kernel_read(replaced, (void *)hook_pos, MCOUNT_INSN_SIZE)) arch/s390/kernel/ftrace.c: if (probe_kernel_read(&old, (void *) rec->ip, sizeof(old))) arch/s390/kernel/ftrace.c: if (probe_kernel_read(&old, (void *) rec->ip, sizeof(old))) arch/sh/kernel/ftrace.c: if (probe_kernel_read(replaced, (void *)ip, MCOUNT_INSN_SIZE)) arch/sh/kernel/ftrace.c: if (probe_kernel_read(code, (void *)ip, MCOUNT_INSN_SIZE)) arch/x86/kernel/dumpstack.c: if (probe_kernel_read(opcodes, rip - PROLOGUE_SIZE, OPCODE_BUFSIZE)) { arch/x86/kernel/ftrace.c: if (probe_kernel_read(replaced, (void *)ip, MCOUNT_INSN_SIZE)) arch/x86/kernel/ftrace.c: if (probe_kernel_read(replaced, (void *)ip, MCOUNT_INSN_SIZE)) arch/x86/kernel/ftrace.c: if (probe_kernel_read(ins, (void *)ip, MCOUNT_INSN_SIZE)) arch/x86/kernel/ftrace.c: ret = probe_kernel_read(trampoline, (void *)start_offset, size); arch/x86/kernel/ftrace.c: ret = probe_kernel_read(&calc, ptr, MCOUNT_INSN_SIZE); arch/x86/kernel/kgdb.c: err = probe_kernel_read(bpt->saved_instr, (char *)bpt->bpt_addr, arch/x86/kernel/kgdb.c: err = probe_kernel_read(opc, (char *)bpt->bpt_addr, BREAK_INSTR_SIZE); arch/x86/kernel/kgdb.c: err = probe_kernel_read(opc, (char *)bpt->bpt_addr, BREAK_INSTR_SIZE); arch/x86/kernel/kprobes/core.c: if (probe_kernel_read(buf, (void *)addr, arch/x86/kernel/kprobes/core.c: if (probe_kernel_read(dest, (void *)recovered_insn, MAX_INSN_SIZE)) arch/x86/kernel/kprobes/opt.c: if (probe_kernel_read(buf, (void *)addr, arch/x86/xen/enlighten_pv.c: probe_kernel_read(&dummy, v, 1); drivers/char/mem.c: probe = probe_kernel_read(bounce, ptr, sz); drivers/dio/dio.c: if (probe_kernel_read(&i, (unsigned char *)va + DIO_IDOFF, 1)) { drivers/dio/dio.c: if (probe_kernel_read(&i, (unsigned char *)va + DIO_IDOFF, 1)) { drivers/input/serio/hp_sdc.c: if (!probe_kernel_read(&i, (unsigned char *)hp_sdc.data_io, 1)) drivers/misc/kgdbts.c: probe_kernel_read(before, (char *)kgdbts_break_test, drivers/misc/kgdbts.c: probe_kernel_read(after, (char *)kgdbts_break_test, drivers/video/fbdev/hpfb.c: err = probe_kernel_read(&i, (unsigned char *)INTFBVADDR + DIO_IDOFF, 1); fs/proc/kcore.c: if (probe_kernel_read(buf, (void *) start, tsz)) { include/linux/uaccess.h: probe_kernel_read(&retval, addr, sizeof(retval)) kernel/debug/debug_core.c: err = probe_kernel_read(bpt->saved_instr, (char *)bpt->bpt_addr, kernel/debug/gdbstub.c: err = probe_kernel_read(tmp, mem, count); kernel/debug/kdb/kdb_main.c: if (!p || probe_kernel_read(&tmp, (char *)p, sizeof(unsigned long))) kernel/debug/kdb/kdb_support.c: int ret = probe_kernel_read((char *)res, (char *)addr, size); kernel/debug/kdb/kdb_support.c: int ret = probe_kernel_read((char *)addr, (char *)res, size); kernel/debug/kdb/kdb_support.c: if (!p || probe_kernel_read(&tmp, (char *)p, sizeof(unsigned long))) kernel/kthread.c: probe_kernel_read(&data, &kthread->data, sizeof(data)); kernel/trace/bpf_trace.c: ret = probe_kernel_read(dst, unsafe_ptr, size); kernel/workqueue.c: probe_kernel_read(&fn, &worker->current_func, sizeof(fn)); kernel/workqueue.c: probe_kernel_read(&pwq, &worker->current_pwq, sizeof(pwq)); kernel/workqueue.c: probe_kernel_read(&wq, &pwq->wq, sizeof(wq)); kernel/workqueue.c: probe_kernel_read(name, wq->name, sizeof(name) - 1); kernel/workqueue.c: probe_kernel_read(desc, worker->desc, sizeof(desc) - 1); mm/slab.c: if (probe_kernel_read(&v, dbg_userword(c, p), sizeof(v))) mm/slub.c: probe_kernel_read(&p, (void **)freepointer_addr, sizeof(p)); 41 of these (or something like that, I counted by hand) have some sort of cast in the call expression. probe_kernel_read() is kinda special in that expected types for the second argument are both kernel pointers and unsigned longs. It might make sense to have a wrapper macro around probe_kernel_read() that accepts anything as long as it's as wide as a pointer... maybe something for a future refactor.
Powered by blists - more mailing lists