linux-kernel - Re: [PATCH] arm64: mm: take CWG into account in __inval_cache

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, 19 Apr 2016 15:08:27 +0200
From:	Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:	Mark Rutland <mark.rutland@....com>
Cc:	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will.deacon@....com>,
	James Morse <james.morse@....com>, robin.murphy@....com
Subject: Re: [PATCH] arm64: mm: take CWG into account in __inval_cache_range()

On 19 April 2016 at 14:56, Mark Rutland <mark.rutland@....com> wrote:
> Hi,
>
> On Tue, Apr 19, 2016 at 12:29:33PM +0200, Ard Biesheuvel wrote:
>> Currently, the arm64 implementation of __inval_cache_range() [aka
>> __dma_inv_range()] takes CTR_EL0.Dminline into account for two purposes:
>> - the stride to use for doing by-VA cache maintenance,
>> - to check whether the start and end arguments are unaligned with respect
>>   to the cache line size, in which case the unaligned extremes need to be
>>   cleaned before being invalidated, to avoid corrupting adjacent unrelated
>>   memory contents.
>>
>> In the second case, the use of Dminline is incorrect, and should use the
>> CWG field instead, since an invalidate operation could result in cache
>> lines that are larger than Dminline to be evicted at any level of the
>> cache hierarchy.
>
> Have you seen this in practice, or was this found by inspection?
>

Amusingly, I spotted it when a Huawei engineer proposing driver code
for Tianocore questioned my demand to take CWG into account when doing
cache invalidation, because the kernel does not do it either.

> I agree that we need to round addresses to CWG boundaries when
> performing maintenance to the PoC to prevent subsequent asynchronous
> writebacks of data falling in the same CWG, which could clobber data at
> the PoC.
>
> However, if we have unrelated data in the same CWG, surely we have no
> guarantee that said data will not be dirtied in caches by other kernel
> code, and thus we may still have issues with asynchronous writebacks?
>

Indeed.

> Is sharing a CWG broken by design, or is there some caveat I'm missing
> that prevents/prohibits unrelated data from being dirtied?
>

I think sharing a CWG window is broken by design, now that I think of
it. The invalidate is part of the dma_unmap() code path, which means
the cleaning we do on the edges of the buffer may clobber data in
memory written by the device, and not cleaning isn't an option either.