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] [day] [month] [year] [list]
Message-ID: <20161122103721.GO3092@twins.programming.kicks-ass.net>
Date:   Tue, 22 Nov 2016 11:37:21 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     David Windsor <dwindsor@...il.com>
Cc:     "Reshetova, Elena" <elena.reshetova@...el.com>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Kees Cook <keescook@...omium.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Will Deacon <will.deacon@....com>,
        Arnd Bergmann <arnd@...db.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Daniel Borkmann <daniel@...earbox.net>
Subject: Re: [RFC][PATCH 2/7] kref: Add kref_read()

On Mon, Nov 21, 2016 at 03:12:33PM -0500, David Windsor wrote:
> On Mon, Nov 21, 2016 at 2:27 PM, Reshetova, Elena
> <elena.reshetova@...el.com> wrote:
> >> On Mon, Nov 21, 2016 at 04:49:15PM +0100, Peter Zijlstra wrote:
> >> > > Speaking of non-fitting patterns. This one is quite common in
> >> > > networking code for refcounters:
> >> > >
> >> > > if (atomic_cmpxchg(&cur->refcnt, 1, 0) == 1) {} This is from
> >> > > net/netfilter/nfnetlink_acct.c, but there are similar ones in other
> >> > > places.
> >> >
> >> > Cute, but weird it doesn't actually decrement if not 1.
> >>
> >> Hurgh.. creative refcounting that. The question is how much of that do
> >> we want to support? It really must not decrement there.

Now, arguably the 1->0 case is special, and we can provide limited
support for that, but I'd be hesitant to provide the full cmpxchg.

We could for instance provide: refcount_dec_if_one().

> > And one more creative usage:
> >
> > http://lxr.free-electrons.com/source/net/ipv4/udp.c#L1940
> >
> > if (!sk || !atomic_inc_not_zero_hint(&sk->sk_refcnt, 2))
> >     return;
> >
> > I didn't even guess anyone is using atomic_inc_not_zero_hint...
> > But network code keeps surprising me today :)
> > So, yes, I guess the question is what to do with these cases really?
> 
> Many of the calls to non-supported functions can be decomposed into
> calls to supported functions. 

So it really depends on what the network guys are willing to put up
with, if their primary goal is to avoid the SHARED state, we could add a
load-exclusive. But I suspect they'd not be happy with that either...

> The ones that may prove interesting are
> ones like atomic_cmpxchg(), in which some sort of external locking is
> going to be required to achieve the same atomicity guarantees provided
> by cmpxchg, like so:
> 
> mutex_lock(lock);
> cnt = refcount_read(ref);
> if (cnt == val1) {
>     refcount_set(ref, val2);
> }
> mutex_unlock(lock);
> return cnt;

That cannot actually work in the presence of actual atomic instructions
not serialized by that lock.

Also, the network guys will absolutely kill you if you propose something
like that.



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ