linux-kernel - Re: [PATCH 0/4] iomap: fix multiple consistency issues, interface cleanup

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: <20190218203007.GB18457@behemoth.owl.eu.com.local>
Date:   Mon, 18 Feb 2019 21:30:07 +0100
From:   Hugo Lefeuvre <hle@....eu.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        linux-arch <linux-arch@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/4] iomap: fix multiple consistency issues, interface
 cleanup

Hi,

> Nice cleanup! Applied to my asm-generic tree now.

Thanks!

> I notice that you did not add a 'volatile' qualifier in addition to 'const'.
> Is that something you considered doing? I think the traditional
> readl/writel style accessors have them partly for historic reasons,
> and partly to simply the trivial implementation that is based on
> a volatile pointer dereference. Neither is needed here, and I don't
> think we have any drivers that pass volatile pointers into ioread()/iowrite()
> (which would cause a warning), so we can probably leave it at that,
> but it's still a bit inconsistent.

Right, I intentionally omitted the volatile qualifier here.

Adding it to the definitions from asm-generic/iomap.h would require to
also update lib/iomap.c and arch/ implementations. This should, imo, only
be done if really necessary.

>From what I've seen, almost all ioread() implementations simply redirect
to readl style accessors. I could not find one implementation that does
something with the addr argument which requires the volatile qualifier.

Also, I'm not even sure to understand why we need the volatile qualifier
in the ioread() definitions from asm-generic/io.h...

Drivers wishing to pass volatile pointers to ioread() should probably use
a more low level api.

regards,
 Hugo

-- 
                Hugo Lefeuvre (hle)    |    www.owl.eu.com
RSA4096_ 360B 03B3 BF27 4F4D 7A3F D5E8 14AA 1EB8 A247 3DFD
ed25519_ 37B2 6D38 0B25 B8A2 6B9F 3A65 A36F 5357 5F2D DC4C