linux-kernel - Re: [PATCH 1/7] atomics/tty: add missing atomic_long

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:   Tue, 5 Jun 2018 16:20:51 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Mark Rutland <mark.rutland@....com>, linux-kernel@...r.kernel.org,
        Boqun Feng <boqun.feng@...il.com>,
        Will Deacon <will.deacon@....com>, Jiri Slaby <jslaby@...e.com>
Subject: Re: [PATCH 1/7] atomics/tty: add missing atomic_long_t * cast

On Tue, Jun 05, 2018 at 02:00:22PM +0200, Peter Zijlstra wrote:
> On Tue, May 29, 2018 at 07:07:40PM +0100, Mark Rutland wrote:
> > In ldsem_cmpxchg a pointer to unsigned long is passed to
> > atomic_long_cmpxchg(), which expects a pointer to atomic_long_t.
> 
> > -	long tmp = atomic_long_cmpxchg(&sem->count, *old, new);
> > +	long tmp = atomic_long_cmpxchg((atomic_long_t *)&sem->count, *old, new);
> 
> Needing to cast anything to atomic types is dodgy at best, so I had me a
> look at this code.
> 
> Would not the following patch be much better?
> 
> ---
>  drivers/tty/tty_ldsem.c   | 82 ++++++++++++++++++++---------------------------
>  include/linux/tty_ldisc.h |  4 +--
>  2 files changed, 37 insertions(+), 49 deletions(-)

At first glance, yes, this does look a lot better, it's less lines at
the least :)

I think Peter Hurley was just trying to hid the atomic mess behind
function calls, which is why he didn't open-code it like you did here.
But this makes it a bit more obvious as to what "magic" is happening, so
I like it.

Care to turn it into a real patch so I can queue it up after 4.18-rc1 is
out?  Or do you want me to do that?

thanks,

greg k-h