linux-kernel - Re: [PATCH v2 0/6] Rework REFCOUNT_FULL using atomic_fetch

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 31 Aug 2019 12:02:43 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Ard Biesheuvel <ard.biesheuvel@...aro.org>
Cc:     Will Deacon <will@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Elena Reshetova <elena.reshetova@...el.com>,
        Hanjun Guo <guohanjun@...wei.com>,
        Jan Glauber <jglauber@...vell.com>
Subject: Re: [PATCH v2 0/6] Rework REFCOUNT_FULL using atomic_fetch_*
 operations

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