From mboxrd@z Thu Jan 1 00:00:00 1970 From: Josh Poimboeuf Subject: Re: [PATCH v2 2/2] x86, refcount: Implement fast refcount overflow protection Date: Mon, 1 May 2017 17:45:42 -0500 Message-ID: <20170501224542.bbfhs7c4x2wibomz@treble> References: <1493160997-126108-1-git-send-email-keescook@chromium.org> <1493160997-126108-3-git-send-email-keescook@chromium.org> <20170501163009.kbemdhpsabdrsfex@treble> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Return-path: Received: from mx1.redhat.com ([209.132.183.28]:41428 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750730AbdEAWpq (ORCPT ); Mon, 1 May 2017 18:45:46 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-arch-owner@vger.kernel.org List-ID: To: Kees Cook Cc: LKML , Peter Zijlstra , PaX Team , Jann Horn , Eric Biggers , Christoph Hellwig , "axboe@kernel.dk" , James Bottomley , Elena Reshetova , Hans Liljestrand , David Windsor , "x86@kernel.org" , Ingo Molnar , Arnd Bergmann , Greg Kroah-Hartman , "David S. Miller" , Rik van Riel , linux-arch , "kernel-hardening@lists.openwall.com" On Mon, May 01, 2017 at 10:36:59AM -0700, Kees Cook wrote: > >> +#ifdef CONFIG_FAST_REFCOUNT > >> +static DEFINE_RATELIMIT_STATE(refcount_ratelimit, 15 * HZ, 3); > >> + > >> +void refcount_error_report(struct pt_regs *regs, const char *kind) > >> +{ > >> + do_send_sig_info(SIGKILL, SEND_SIG_FORCED, current, true); > >> + > >> + if (!__ratelimit(&refcount_ratelimit)) > >> + return; > >> + > >> + pr_emerg("%s detected in: %s:%d, uid/euid: %u/%u\n", > >> + kind ? kind : "refcount error", > >> + current->comm, task_pid_nr(current), > >> + from_kuid_munged(&init_user_ns, current_uid()), > >> + from_kuid_munged(&init_user_ns, current_euid())); > >> + print_symbol(KERN_EMERG "refcount error occurred at: %s\n", > >> + instruction_pointer(regs)); > >> + preempt_disable(); > >> + show_regs(regs); > >> + preempt_enable(); > >> +} > > > > Why is preemption disabled before calling show_regs()? > > I thought it was to avoid interleaving show_regs() output (I can't > think of a way regs would be externally modified). This code is running from interrupt context, so preemption shouldn't be an issue unless I'm missing something. -- Josh From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:41428 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750730AbdEAWpq (ORCPT ); Mon, 1 May 2017 18:45:46 -0400 Date: Mon, 1 May 2017 17:45:42 -0500 From: Josh Poimboeuf Subject: Re: [PATCH v2 2/2] x86, refcount: Implement fast refcount overflow protection Message-ID: <20170501224542.bbfhs7c4x2wibomz@treble> References: <1493160997-126108-1-git-send-email-keescook@chromium.org> <1493160997-126108-3-git-send-email-keescook@chromium.org> <20170501163009.kbemdhpsabdrsfex@treble> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Sender: linux-arch-owner@vger.kernel.org List-ID: To: Kees Cook Cc: LKML , Peter Zijlstra , PaX Team , Jann Horn , Eric Biggers , Christoph Hellwig , "axboe@kernel.dk" , James Bottomley , Elena Reshetova , Hans Liljestrand , David Windsor , "x86@kernel.org" , Ingo Molnar , Arnd Bergmann , Greg Kroah-Hartman , "David S. Miller" , Rik van Riel , linux-arch , "kernel-hardening@lists.openwall.com" Message-ID: <20170501224542.RMEJw8tle4zN7vN5cT7kW4LIQ0TDRy9hBPtuQkvUtsw@z> On Mon, May 01, 2017 at 10:36:59AM -0700, Kees Cook wrote: > >> +#ifdef CONFIG_FAST_REFCOUNT > >> +static DEFINE_RATELIMIT_STATE(refcount_ratelimit, 15 * HZ, 3); > >> + > >> +void refcount_error_report(struct pt_regs *regs, const char *kind) > >> +{ > >> + do_send_sig_info(SIGKILL, SEND_SIG_FORCED, current, true); > >> + > >> + if (!__ratelimit(&refcount_ratelimit)) > >> + return; > >> + > >> + pr_emerg("%s detected in: %s:%d, uid/euid: %u/%u\n", > >> + kind ? kind : "refcount error", > >> + current->comm, task_pid_nr(current), > >> + from_kuid_munged(&init_user_ns, current_uid()), > >> + from_kuid_munged(&init_user_ns, current_euid())); > >> + print_symbol(KERN_EMERG "refcount error occurred at: %s\n", > >> + instruction_pointer(regs)); > >> + preempt_disable(); > >> + show_regs(regs); > >> + preempt_enable(); > >> +} > > > > Why is preemption disabled before calling show_regs()? > > I thought it was to avoid interleaving show_regs() output (I can't > think of a way regs would be externally modified). This code is running from interrupt context, so preemption shouldn't be an issue unless I'm missing something. -- Josh