linux-kernel - Re: Problem with atomic accesses in pstore on some ARM CPUs

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:   Fri, 19 Aug 2016 05:47:24 -0700
From:   Guenter Roeck <groeck@...gle.com>
To:     Russell King - ARM Linux <linux@...linux.org.uk>
Cc:     Colin Cross <ccross@...roid.com>,
        Mark Rutland <mark.rutland@....com>,
        Tony Luck <tony.luck@...el.com>,
        Kees Cook <keescook@...omium.org>,
        Jeffy Chen <jeffy.chen@...k-chips.com>,
        Will Deacon <will.deacon@....com>,
        Douglas Anderson <dianders@...omium.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Robin Murphy <robin.murphy@....com>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: Problem with atomic accesses in pstore on some ARM CPUs

On Fri, Aug 19, 2016 at 2:35 AM, Russell King - ARM Linux
<linux@...linux.org.uk> wrote:
> On Tue, Aug 16, 2016 at 10:35:52AM -0700, Colin Cross wrote:
>> persistent_ram uses atomic ops in uncached memory to store the start
>> and end positions in the ringbuffer so that the state of the
>> ringbuffer will be valid if the kernel crashes at any time.  This was
>> inherited from Android's ram_console implementation, and worked
>> through armv7.
>
> That statement is actually inaccurate.  It may have worked on _some_
> ARMv7 implementations, but it's not architecturally compliant.
>
> The exclusive access instructions are not portable to anything but
> "normal memory":
>
>    It is IMPLEMENTATION DEFINED whether LDREX and STREX operations can
>    be performed to a memory region with the Device or Strongly-ordered
>    memory attribute. Unless the implementation documentation explicitly
>    states that LDREX and STREX operations to a memory region with the
>    Device or Strongly-ordered attribute are permitted, the effect of
>    such operations is UNPREDICTABLE.
>
> pgprot_noncached() gives strongly-ordered memory, and so is unsuitable
> to place semaphores in for all ARMv7 implementations.
>
> Also:
>
> +       if (memtype)
> +               va = ioremap(start, size);
>
> this returns *device* memory, which is also unsuitable for all ARMv7
> implementations.
>
> It seems that pstore is playing in areas of the architecture which are
> implementation defined, so it's no surprise that folk are seeing
> different behaviours with different implementations.
>
> The code isn't architecturally wrong, it just isn't portable to all ARMv7
> implementations.
>
> So, saying that it works on some ARMv7 implementations is irrelevent.
>
> Note that LDREX and STREX are used for all operations that require
> atomicity - iow, atomics and locks.
>
> pgprot_writecombine() and ioremap_wc() will return memory which is
> suitable for these exclusive accesses - it maps to the architectures'
> "normal memory, uncacheable".
>

Unfortunately, pgprot_writecombine() doesn't work in my case (for
rk3288 and rk3399). It was also reported not to work on Tegra Logan by
Nvidia. As mentioned earlier, PAGE_KERNEL works, at least for rk3288
and rk3399.

Guenter

> So, I suspect the OP should not be using mem_type=1 or using the
> "unbuffered" DT attribute, but should leave it was the default
> (mem_type=0).
>
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.