Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 31 Aug 2019 12:02:43 -0700
From:   Kees Cook <keescook@chromium.org>
To:     Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc:     Will Deacon <will@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        Elena Reshetova <elena.reshetova@intel.com>,
        Hanjun Guo <guohanjun@huawei.com>,
        Jan Glauber <jglauber@marvell.com>
Subject: Re: [PATCH v2 0/6] Rework REFCOUNT_FULL using atomic_fetch_*
 operations
Message-ID: <201908311200.926B5C0F@keescook>
References: <20190827163204.29903-1-will@kernel.org>
 <20190828073052.GL2332@hirez.programming.kicks-ass.net>
 <20190828141439.sqnpm5ff4tgyn66r@willie-the-truck>
 <201908281353.0EFD0776@keescook>
 <CAKv+Gu_Q=o_6xDW_7YTd3J6psqs-o+qBxW4r9MXCBwjmsGpTbQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAKv+Gu_Q=o_6xDW_7YTd3J6psqs-o+qBxW4r9MXCBwjmsGpTbQ@mail.gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Sat, Aug 31, 2019 at 08:48:56PM +0300, Ard Biesheuvel wrote:
> It's been ~2 years since I looked at this code in detail, but IIRC, it
> looked like the inc-from-zero check was missing from the x86
> implementation because it requires a load/compare/increment/store
> sequence instead of a single increment instruction taking a memory
> operand. Was there more rationale at the time for omitting this
> particular case, and if so, was it based on a benchmark? Can we run it
> against this implementation as well?

It was based on providing a protection against the pre-exploitation case
(overflow: "something bad is about to happen, let's stop it") rather
than the post-exploitation case (inc from zero, "something bad already
happened, eek") with absolutely the fewest possible extra cycles, as
various subsystem maintainers had zero tolerance for any measurable
changes in refcounting performance.

I much prefer the full coverage, even if it's a tiny bit slower. And
based on the worse-case timings (where literally nothing else is
happening) it seems like these changes should be WELL under the noise.

-- 
Kees Cook