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: Mon, 29 Jan 2024 13:22:57 -0800
From: Dan Williams <dan.j.williams@...el.com>
To: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Dan Williams
	<dan.j.williams@...el.com>, Vishal Verma <vishal.l.verma@...el.com>, "Dave
 Jiang" <dave.jiang@...el.com>
CC: <linux-kernel@...r.kernel.org>, Mathieu Desnoyers
	<mathieu.desnoyers@...icios.com>, Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>, <linux-mm@...ck.org>,
	<linux-arch@...r.kernel.org>, Matthew Wilcox <willy@...radead.org>,
	<linux-cxl@...r.kernel.org>, <nvdimm@...ts.linux.dev>
Subject: Re: [RFC PATCH 0/7] Introduce cache_is_aliasing() to fix DAX
 regression

Mathieu Desnoyers wrote:
> This commit introduced in v5.13 prevents building FS_DAX on 32-bit ARM,
> even on ARMv7 which does not have virtually aliased dcaches:
> 
> commit d92576f1167c ("dax: does not work correctly with virtual aliasing caches")
> 
> It used to work fine before: I have customers using dax over pmem on
> ARMv7, but this regression will likely prevent them from upgrading their
> kernel.
> 
> The root of the issue here is the fact that DAX was never designed to
> handle virtually aliased dcache (VIVT and VIPT with aliased dcache). It
> touches the pages through their linear mapping, which is not consistent
> with the userspace mappings on virtually aliased dcaches. 
> 
> This patch series introduces cache_is_aliasing() with new Kconfig
> options:
> 
>   * ARCH_HAS_CACHE_ALIASING
>   * ARCH_HAS_CACHE_ALIASING_DYNAMIC
> 
> and implements it for all architectures. The "DYNAMIC" implementation
> implements cache_is_aliasing() as a runtime check, which is what is
> needed on architectures like 32-bit ARMV6 and ARMV6K.
> 
> With this we can basically narrow down the list of architectures which
> are unsupported by DAX to those which are really affected.
> 
> Feedback is welcome,

Hi Mathieu, this looks good overall, just some quibbling about the
ordering.

I would introduce dax_is_supported() with the current overly broad
interpretation of "!(ARM || MIPS || SPARC)" using IS_ENABLED(), then
fixup the filesystems to use the new helper, and finally go back and
convert dax_is_supported() to use cache_is_aliasing() internally.

Separately, it is not clear to me why ARCH_HAS_CACHE_ALIASING_DYNAMIC
needs to exist. As long as all paths that care are calling
cache_is_aliasing() then whether it is dynamic or not is something only
the compiler cares about. If those dynamic archs do not want to pay the
text size increase they can always do CONFIG_FS_DAX=n, right?

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ