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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAD8Lp446Zu=EhdzRE9pEoskMWeOHOovoAJ8qtrovEuZ2E7Z5VQ@mail.gmail.com>
Date:   Tue, 17 Jul 2018 07:30:22 -0500
From:   Daniel Drake <drake@...lessm.com>
To:     Daniel Kurtz <djkurtz@...omium.org>
Cc:     Shyam Sundar S K <Shyam-sundar.S-k@....com>,
        Nehal Shah <Nehal-bakulchandra.Shah@....com>,
        Ken Xue <Ken.Xue@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Linus Walleij <linus.walleij@...aro.org>,
        "open list:PIN CONTROL SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] pinctrl/amd: use byte access to clear irq/wake status bits

On Mon, Jul 16, 2018 at 7:57 PM, Daniel Kurtz <djkurtz@...omium.org> wrote:
> Commit 6afb10267c1692 ("pinctrl/amd: fix masking of GPIO interrupts")
> changed to the clearing of interrupt status bits to a RMW in a critical
> section.  This works, but is a bit overkill.
>
> The relevant interrupt/wake status bits are in the Most Significant Byte
> of a 32-bit word.  These two are the only write-able bits in this byte.

I don't have the hardware to test this any more, and I also don't have
any docs to double if those are really the only writable bits, but
looking at the existing driver code it does seem to be the case.

I think you should retain the comment noting that the value of the
register may have changed since it was read just a few lines above
(and hence explaining more precisely why we make the special effort
just to modify the MSB), just in case there is further rework of this
code in future and we end up walking into the same trap. It was one of
those issues that took a frustratingly long time to figure out...

Thanks
Daniel

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ