linux-kernel - Re: [PATCH 0/5] v2: block subsystem refcounter conversions

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]

Message-ID: <20170420183319.GB103004@gmail.com>
Date:   Thu, 20 Apr 2017 11:33:19 -0700
From:   Eric Biggers <ebiggers3@...il.com>
To:     "Reshetova, Elena" <elena.reshetova@...el.com>
Cc:     Christoph Hellwig <hch@...radead.org>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "james.bottomley@...senpartnership.com" 
        <james.bottomley@...senpartnership.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "linux-btrfs@...r.kernel.org" <linux-btrfs@...r.kernel.org>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "fujita.tomonori@....ntt.co.jp" <fujita.tomonori@....ntt.co.jp>,
        "mingo@...hat.com" <mingo@...hat.com>, "clm@...com" <clm@...com>,
        "jbacik@...com" <jbacik@...com>,
        "dsterba@...e.com" <dsterba@...e.com>
Subject: Re: [PATCH 0/5] v2: block subsystem refcounter conversions

Hi Elena,

On Thu, Apr 20, 2017 at 04:10:16PM +0000, Reshetova, Elena wrote:
> 
> > All the objections from DaveM on the amount of cycles spent on the
> > new refcount_t apply to the block layer fast path operations as well.
> 
> Ok, could you please indicate the correct way to measure the impact for the block layer? 
> We can do the measurements. 
> 
> Best Regards,
> Elena.
> 
> > 
> > Please don't send any more conversions until those have been resolved.

Like I suggested months ago, how about doing an efficient implementation of
refcount_t which doesn't use the bloated cmpxchg loop?  Then there would be no
need for endless performance arguments.  In fact, in PaX there are already
example implementations for several architectures.  It's unfortunate that
they're still being ignored for some reason.

At the very least, what is there now could probably be made about twice as fast
by removing the checks that don't actually help mitigate refcount overflow bugs,
specifically all the checks in refcount_dec(), and the part of refcount_inc()
where it doesn't allow incrementing a 0 refcount.  Hint: if a refcount is 0, the
object has already been freed, so the attacker will have had the opportunity to
allocate it with contents they control already.

Of course, having extra checks behind a debug option is fine.  But they should
not be part of the base feature; the base feature should just be mitigation of
reference count *overflows*.  It would be nice to do more, of course; but when
the extra stuff prevents people from using refcount_t for performance reasons,
it defeats the point of the feature in the first place.

I strongly encourage anyone who has been involved in refcount_t to experiment
with removing a reference count decrement somewhere in their kernel, then trying
to exploit it to gain code execution.  If you don't know what you're trying to
protect against, you will not know which defences work and which don't.

- Eric