linux-kernel - Re: [PATCH v2 1/3] ACPI: osl: Add __force attribute in acpi_os_map

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, 11 Aug 2021 12:40:28 +0200
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Christoph Hellwig <hch@...radead.org>
Cc:     Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Veronika kabatova <vkabatov@...hat.com>,
        Robin Murphy <robin.murphy@....com>,
        Will Deacon <will@...nel.org>,
        Hanjun Guo <guohanjun@...wei.com>,
        Sudeep Holla <sudeep.holla@....com>,
        Catalin Marinas <catalin.marinas@....com>
Subject: Re: [PATCH v2 1/3] ACPI: osl: Add __force attribute in
 acpi_os_map_iomem() cast

On Tue, 10 Aug 2021 at 18:46, Christoph Hellwig <hch@...radead.org> wrote:
>
> On Mon, Aug 02, 2021 at 04:23:57PM +0100, Lorenzo Pieralisi wrote:
> > Add a __force attribute to the void* cast in acpi_os_map_iomem()
> > to prevent sparse warnings.
>
> Err, no.  These annotation are there for a reason and need to
> be propagated instead.  And independent of that a __force cast
> without a comment explaining it is a complete no-go.

The whole problem we are solving here is that ACPI, being based on
x86, conflates MMIO mappings with memory mappings, and has been using
the same underlying infrastructure for either. On arm64, this is not
sufficient, given that the semantics of uncached memory vs device are
different (the former permits unaligned accesses and clear cacheline
instructions, but the latter doesn't). A recent optimization applied
to memcpy() on arm64 (which now relies more on unaligned accesses for
performance) has uncovered an issue where firmware tables being mapped
non-cacheable by the ACPI core will end up using device mappings,
which causes memcpy() to choke on their contents.

So propagating the annotation makes no sense, as we are creating a
memory mapping using the iomem primitive. I wouldn't object to a
comment being added, but I think the context should have been obvious
to anyone who had bothered to look at the entire series.