Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20161004124616.GF2040@pc.thejh.net>
Date: Tue, 4 Oct 2016 14:46:16 +0200
From: Jann Horn <jann@...jh.net>
To: "Reshetova, Elena" <elena.reshetova@...el.com>
Cc: "kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>,
	"keescook@...omium.org" <keescook@...omium.org>,
	Hans Liljestrand <ishkamiel@...il.com>,
	David Windsor <dwindsor@...il.com>
Subject: Re: [RFC PATCH 12/13] x86: x86 implementation for
 HARDENED_ATOMIC

On Tue, Oct 04, 2016 at 07:15:56AM +0000, Reshetova, Elena wrote:
> >An additional idea for future development:
> 
> >One way to work around that would be to interpret the stored value 2^30 as zero, and interpret other values accordingly. Like this:
> 
> >#define SIGNED_ATOMIC_BASE 0x40000000U
> 
> >static __always_inline int atomic_read(const atomic_t *v) {
>   return READ_ONCE((v)->counter) - SIGNED_ATOMIC_BASE; }
> 
> >static __always_inline void atomic_set(atomic_t *v, int i) {
>   WRITE_ONCE(v->counter, i + SIGNED_ATOMIC_BASE); }
> 
> >static __always_inline int atomic_add_return(int i, atomic_t *v) {
>   return i + xadd_check_overflow(&v->counter, i) - SIGNED_ATOMIC_BASE; }
> 
> >With this change, atomic_t could still be used as a signed integer with half the range of an int, but its stored value would only become negative on overflow. Then, the "jno" instruction in the hardening code could be replaced with "jns" to reliably block overflows.
> 
> >The downsides of this approach would be:
> > - One extra increment or decrement every time an atomic_t is read
>    or written. This should be relatively cheap - it should be
>    operating on a register -, but it's still not ideal. atomic_t
>    users could perhaps opt out with something like
>    atomic_unsigned_t.
>  - Implicit atomic_t initialization to zero by zeroing memory
>    would stop working. This would probably be the biggest issue
>    with this approach.
> 
> I am not sure the BIAS is a good idea at all. Makes things much more complicated, potentially impacts performance...

Yeah, it does make things more complicated. And I just noticed that with the
BIAS, atomic_sub_and_test() would likely have to be implemented with cmpxchg,
so it probably doesn't help much performance-wise.

In summary: Nevermind, it was a stupid idea.

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.