From patchwork Tue Oct 4 13:06:31 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans Liljestrand X-Patchwork-Id: 9361831 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4C2EE600C8 for ; Tue, 4 Oct 2016 13:06:55 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3D777285B5 for ; Tue, 4 Oct 2016 13:06:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 31CCA28739; Tue, 4 Oct 2016 13:06:55 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.wl.linuxfoundation.org (Postfix) with SMTP id 11F48285B5 for ; Tue, 4 Oct 2016 13:06:49 +0000 (UTC) Received: (qmail 13428 invoked by uid 550); 4 Oct 2016 13:06:48 -0000 Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: kernel-hardening@lists.openwall.com Delivered-To: mailing list kernel-hardening@lists.openwall.com Received: (qmail 13393 invoked from network); 4 Oct 2016 13:06:46 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=k2bng32VsSekuO5Ph5rOy3FfOIfV7gbfSvh6ujSgCXY=; b=mR298cUhraZW8vqHXViu2Lj4JGSet0vEv2AoqIIgwoOCtOlJJ3M1wuESSjLVMLtm3G 2BbAG5FelAsBigpZq2G7UILykVU+rG60KzlYd3WbxUvNpg5OoFUINuj1ewtnlv4A0f6f Cf56upuYXyN3r8w+j7kQaPRkOZjL3VWxahzvjiYMe0LfV+J3xb4I62uW40S7U7fPIOBd 46YLI0W1BE05Akjs8tJUu1HIgBcr9X1gIj6BM8TKqXMGQb5zo/FO/79zVV+CDxLJeE+y tkZfR788iAIu/YKNP+vAkiyu59qzh5VYc8xE/twKNYniPUKXGYdMhMtBryAnn3E/puFb 0+fg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=k2bng32VsSekuO5Ph5rOy3FfOIfV7gbfSvh6ujSgCXY=; b=SVTEmVcIUODS8ZhS56DeLXrW3WQOPtZ20/9qdWS8t74Me7vOs1iqrGzfZNWinr5HXp Rl/NvocmBQLN3xm+h1RCDJT7DYK+9n6OnaecP3Z3evQAHdsbzSPAwsZ7MphC3iGrzGZj snA5mwkf955Ti2iVIP/UCFNhtoIJukKzg8PoSE63ZDr0EntokooBt0czZYEaz7E5uAHE wxUXoAdQFPBh0xczjcMmWxoEHd+xNMh87MlU8WqIR2k7Kp4sGnXWrbVTYC4DTAkZZuxh Cc/OFWtmZUsSFrFkbHD/usWc3qytdZsy62p/Jj+ANDSZIun9f7Qbg3wUb9cnGmF9rbyU 67/A== X-Gm-Message-State: AA6/9RmTyIhG0mfzY5h9YLtzHWUB5RqAPNSDytLJcTtw2lfTBgKwDdN0r6RVPPWMeuaHow== X-Received: by 10.25.24.154 with SMTP id 26mr1236692lfy.3.1475586394990; Tue, 04 Oct 2016 06:06:34 -0700 (PDT) Date: Tue, 4 Oct 2016 16:06:31 +0300 From: Hans Liljestrand To: "Reshetova, Elena" Cc: Kees Cook , "kernel-hardening@lists.openwall.com" , David Windsor Message-ID: <20161004130631.GA17762@thigreal> References: <1475476886-26232-1-git-send-email-elena.reshetova@intel.com> <1475476886-26232-3-git-send-email-elena.reshetova@intel.com> <2236FBA76BA1254E88B949DDB74E612B41BDAB84@IRSMSX102.ger.corp.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <2236FBA76BA1254E88B949DDB74E612B41BDAB84@IRSMSX102.ger.corp.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Subject: [kernel-hardening] Re: [RFC PATCH 02/13] percpu-refcount: leave atomic counter unprotected X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Oct 04, 2016 at 06:24:29AM +0000, Reshetova, Elena wrote: > On Sun, Oct 2, 2016 at 11:41 PM, Elena Reshetova wrote: > > From: Hans Liljestrand > > > > This is a temporary solution, and a deviation from the PaX/Grsecurity > > implementation where the counter in question is protected against > > overflows. That however necessitates decreasing the PERCPU_COUNT_BIAS > > which is used in lib/percpu-refcount.c. Such a change effectively cuts > > the safe counter range down by half, and still allows the counter to, > > without warning, prematurely reach zero (which is what the bias aims > > to prevent). > > >It might be useful to include a link to the earlier discussions that led to this solution. > > Big part of it was in private emails, not sure how to reference that. Maybe we can just add more explanation here? I can try to summarize the discussion/reasoning here. Please correct me if/when I'm wrong. percpu-refcount uses an atomic, which should be protected similarly to other reference counters that this patch series tries to address. But it is not. guaranteed to do just that). The PaX/Grsecurity solution is to decrease this bias, effectively cutting the safe range in half (now [1,MAX]). And while overflows at MAX would be caught, the counter could still prematurely reach zero. (Although since the counter can be off at most by it's true value, presumably an overflow would still trigger at some point during the percpu ref additions, but not necessarily before zero had been reached one or more times.) The immediate solution would be to go with the bias decrease (and document the repercussions), but we had already seen some objections to that due to the reasons above. Other solutions would seem to require more comprehensive changes percpu-ref, which we felt were not suited for this patch series. We therefore decided to switch the counter to an atomic_long_wrap_t and just document the issue for now. --- a/include/linux/percpu-refcount.h +++ b/include/linux/percpu-refcount.h @@ -81,7 +81,17 @@ ... struct percpu_ref { - atomic_long_t count; ... + atomic_long_wrap_t count; The way it works (before and after our patch) is that the count needs to be updated in a non-atomic way. This means that before all the percpu refs are added the value could be off in either direction, but no more than the actual "true" value of the counter. In order to prevent the counter prematurely reaching zero, a bias (defined in lib/percup-refcount.c) is used to offset the range from [MIN,MAX] to [1,MAX]+[MIN,-1] (with "zero" in the middle, as far from 0 as possible). https://github.com/ereshetova/linux-stable/commit/af44298668d12bf79f48e14396568e9f29ca4bef#diff-be7e4fe901ed6a9d5292276fef233468R34 The problem is then that if the atomic is protected it cannot wrap (and zero is already offset next to the "wrap-barrier", so it is practically