Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1434843AbdDZEmw (ORCPT ); Wed, 26 Apr 2017 00:42:52 -0400 Received: from mail-it0-f54.google.com ([209.85.214.54]:36852 "EHLO mail-it0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1953860AbdDZEml (ORCPT ); Wed, 26 Apr 2017 00:42:41 -0400 MIME-Version: 1.0 In-Reply-To: <5900026E.29602.1FC65392@pageexec.freemail.hu> References: <20170421220939.GA65363@beast> <58FF3273.8060.1C99E526@pageexec.freemail.hu> <5900026E.29602.1FC65392@pageexec.freemail.hu> From: Kees Cook Date: Tue, 25 Apr 2017 21:42:39 -0700 X-Google-Sender-Auth: vqmet0msAEbv3GuKK36s4CGM0U4 Message-ID: Subject: Re: [PATCH] x86/refcount: Implement fast refcount_t handling To: PaX Team Cc: Peter Zijlstra , LKML , 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 , Jann Horn , "David S. Miller" , linux-arch , "kernel-hardening@lists.openwall.com" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3103 Lines: 60 On Tue, Apr 25, 2017 at 7:14 PM, PaX Team wrote: > On 25 Apr 2017 at 9:39, Kees Cook wrote: > >> On Tue, Apr 25, 2017 at 4:26 AM, PaX Team wrote: >> > INT_MAX threads would be needed when the leaking path is locked so >> > that it can only be exercised once and you'll need to get normal >> > (balanced) paths preempted just after the increment. if the leaking >> > path is lockless (can be exercised in parallel without bounds) then >> > 2 threads are enough where the one triggering the signed overflow >> > would have to be preempted while the other one does INT_MAX increments >> > and trigger the UAF. this is where the other mechanisms i talked about >> > in the past become relevant: preemption or interrupts can be disabled >> > or negative refcount values can be detected and acted upon (your blind >> > copy-pasting effort passed upon this latter opportunity by not >> > specializing the 'jo' into 'js' for the refcount case). >> >> Well, it's not "blind" -- I'm trying to bring the code as-is to >> upstream for discussion/examination with as little functional >> differences as possible so it's easier to compare apples to apples. > > you copied code from a version which is at least 2 major kernel revisions > behind (so much for those apples) Hmm, this was from your 4.9 port. Linus hasn't quite released 4.11 yet, so that's actually "at most 2 major kernel revisions behind". :) Regardless, I'd be happy to refresh the port. Will you share a URL to your latest rebase against upstream? > you chose the one version which had a > bug that you didn't spot nor fix properly, you didn't realize the opportunity > that a special refcount type represents, you claimed refcount underflows > aren't exploitable but copied code that would detect signed underflow, you > didn't understand the limits and edge cases i explained above... need i go As I said, I was trying to minimize changes to your implementation, which included the bug and the other issues. The point of this was to share it with others, and work collaboratively on it. I think this clearly succeeded with benefits to both upstream and PaX: Jann spotted the fix for the bug causing weird crashes I saw when doing initial testing, you pointed out the benefit of using js over jo, I've reorganized the RMWcc macros for more easily adding trailing instructions, Peter is thinking about ways around the protection, etc. > on? doesn't leave one with great confidence in your ability to understand > and maintain this code... Well, that's your opinion. I think the patch and its discussion helped several people, including myself, understand this code. Since many people will share its maintenance, I think this is the right way to handle upstreaming these kinds of things. I don't claim to be omniscient, just persistent. Getting this protection into upstream means every Linux user will benefit from what you created, which I think is awesome; this whole class of refcount flaws goes away. Thank you for writing it, sharing it, and discussing it! -Kees -- Kees Cook Pixel Security