netdev - Re: [RFC PATCH] lib: Introduce generic __cmpxchg

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:   Thu, 1 Nov 2018 17:54:50 +0000
From:   Paul Burton <paul.burton@...s.com>
To:     Trond Myklebust <trondmy@...merspace.com>
CC:     "linux@...ck-us.net" <linux@...ck-us.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "ralf@...ux-mips.org" <ralf@...ux-mips.org>,
        "jlayton@...nel.org" <jlayton@...nel.org>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        "bfields@...ldses.org" <bfields@...ldses.org>,
        "linux-mips@...ux-mips.org" <linux-mips@...ux-mips.org>,
        "linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "anna.schumaker@...app.com" <anna.schumaker@...app.com>,
        "jhogan@...nel.org" <jhogan@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "arnd@...db.de" <arnd@...db.de>,
        "paulus@...ba.org" <paulus@...ba.org>,
        "mpe@...erman.id.au" <mpe@...erman.id.au>,
        "benh@...nel.crashing.org" <benh@...nel.crashing.org>
Subject: Re: [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it
 where needed

Hi Trond,

On Thu, Nov 01, 2018 at 12:17:31AM +0000, Trond Myklebust wrote:
> On Wed, 2018-10-31 at 23:32 +0000, Paul Burton wrote:
> > In this particular case I have no idea why
> > net/sunrpc/auth_gss/gss_krb5_seal.c is using cmpxchg64() at all. It's
> > essentially reinventing atomic64_fetch_inc() which is already
> > provided
> > everywhere via CONFIG_GENERIC_ATOMIC64 & the spinlock approach. At
> > least
> > for atomic64_* functions the assumption that all access will be
> > performed using those same functions seems somewhat reasonable.
> > 
> > So how does the below look? Trond?
> 
> My one question (and the reason why I went with cmpxchg() in the first
> place) would be about the overflow behaviour for atomic_fetch_inc() and
> friends. I believe those functions should be OK on x86, so that when we
> overflow the counter, it behaves like an unsigned value and wraps back
> around.  Is that the case for all architectures?
> 
> i.e. are atomic_t/atomic64_t always guaranteed to behave like u32/u64
> on increment?
> 
> I could not find any documentation that explicitly stated that they
> should.

Based on other replies it seems like it's at least implicitly assumed by
other code, even if not explicitly stated.

>From a MIPS perspective where atomics are implemented using load-linked
& store-conditional instructions the actual addition will be performed
using the same addu instruction that a plain integer addition would
generate (regardless of signedness), so there'll be absolutely no
difference in arithmetic between your gss_seq_send64_fetch_and_inc()
function and atomic64_fetch_inc(). I'd expect the same to be true for
other architectures with load-linked & store-conditional style atomics.

In any case, for the benefit of anyone interested who I didn't copy on
the patch submission, here it is:

    https://lore.kernel.org/lkml/20181101175109.8621-1-paul.burton@mips.com/

Thanks,
    Paul