linux-kernel - Re: [PATCH v18 6/7] crypto: caam: cleanup CONFIG

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:   Wed, 4 Jul 2018 20:32:20 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Horia Geanta <horia.geanta@....com>
Cc:     Logan Gunthorpe <logang@...tatee.com>,
        Fabio Estevam <festevam@...il.com>,
        Aymen Sghaier <aymen.sghaier@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Linux-Arch <linux-arch@...r.kernel.org>,
        "linux-ntb@...glegroups.com" <linux-ntb@...glegroups.com>,
        "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" 
        <linux-crypto@...r.kernel.org>, Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Dan Douglass <dan.douglass@....com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH v18 6/7] crypto: caam: cleanup CONFIG_64BIT ifdefs when
 using io{read|write}64

On Wed, Jul 4, 2018 at 2:45 PM, Horia Geanta <horia.geanta@....com> wrote:
> On 7/4/2018 2:58 AM, Logan Gunthorpe wrote:
>>
>>
>> On 03/07/18 04:21 PM, Andy Shevchenko wrote:
>>> It is an explicit call to BUG().
>>> That's why we see wrong instruction trap.
>>
>> Ok, I think I see the problem... the code is rather confusing:
>>
>> Prior to the patch, IOs were BE depending on caam_little_end but if
>> caam_imx was set, then it wrote two LE writes with the high one first.
>> After the patch, it writes two BE writes with the high one first.
>>
> I think there are two separate issues here:
>
> 1. Semantics of operations in io-64-nonatomic-lo-hi.h, io-64-nonatomic-hi-lo.h
>
> Logan, you mentioned the following (which unfortunately I somehow missed):
> https://lore.kernel.org/lkml/c3f2e061-5ed1-5c74-b955-3d2bfb0da074@deltatee.com
> The lo_hi/hi_lo functions seem to always refer to the data being written
> or read not to the address operated on.
>
> OTOH, initial commit that added asm-generic/io-64-nonatomic-lo-hi.h and
> asm-generic/io-64-nonatomic-hi-lo.h mentions:
> 797a796a13df6 ("asm-generic: architecture independent readq/writeq for 32bit
> environment")
> - <asm-generic/io-64-nonatomic-lo-hi.h> provides non-atomic readq/ writeq with
> the order of lower address -> higher address
> - <asm-generic/io-64-nonatomic-hi-lo.h> provides non-atomic readq/ writeq with
> reversed order
>
> I think we should keep the initial semantics when adding support for
> io{read|write}64, i.e. "lo" and "hi" to refer to address and not to value.

There is a ambiguity with the commit message.
Since it had implemented only LE IO accessors the address semantics is
the same as data flow.

As a driver writer intuitively I would expect data flow semantics.

It means it would be invariant to LE BE accessors, right?

lo-hi: LE (0x0 0x4) BE (0x4 0x0)
hi-lo: LE (0x4 0x0) BE (0x0 0x4)

-- 
With Best Regards,
Andy Shevchenko