linux-kernel - Re: [PATCH v11 2/5] tty: goldfish: introduce gf_ioread32()/gf

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:   Thu, 20 Jan 2022 09:50:30 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Laurent Vivier <laurent@...ier.eu>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Alessandro Zummo <a.zummo@...ertech.it>,
        linux-rtc@...r.kernel.org, John Stultz <john.stultz@...aro.org>,
        Arnd Bergmann <arnd@...db.de>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Jiaxun Yang <jiaxun.yang@...goat.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        linux-m68k <linux-m68k@...ts.linux-m68k.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Stephen Boyd <sboyd@...nel.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>
Subject: Re: [PATCH v11 2/5] tty: goldfish: introduce gf_ioread32()/gf_iowrite32()

On Thu, Jan 20, 2022 at 9:03 AM Laurent Vivier <laurent@...ier.eu> wrote:
>
> Revert
> commit da31de35cd2f ("tty: goldfish: use __raw_writel()/__raw_readl()")
>
> to use accessors defined by the architecture.
>
> Define by default the accessor to be little-endian as we
> have only little-endian architectures using goldfish devices.
>
> Signed-off-by: Laurent Vivier <laurent@...ier.eu>

The patch looks good, but the description seems wrong to me:

Talking about "little-endian architectures" makes no sense here, the
point is that the device was clearly defined as having little-endian
registers, and your earlier patch broke this driver when running
on big-endian kernels (if anyone ever tried this).

This means you should also add

Cc: stable@...r.kernel.org # v5.11+
Fixes: da31de35cd2f ("tty: goldfish: use __raw_writel()/__raw_readl()")

The fact that m68k gets this wrong is just a bug in qemu, but it's
probably impossible to fix that since there is no way of knowing which
other operating systems have started relying on that bug over the years.

It might be a good idea to revisit the qemu implementation and make
sure that the extra byteswap is only inserted on m68k and not on
other targets, but hopefully there are no new targets based on goldfish
anymore and we don't need to care.

        Arnd