linux-kernel - Re: [PATCH] powerpc: drop port I/O helpers for CONFIG_HAS

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] [day] [month] [year] [list]

Message-Id: <16334340-d409-4be4-a89d-981baa7a4b82@app.fastmail.com>
Date: Fri, 19 Apr 2024 07:58:40 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Michael Ellerman" <mpe@...erman.id.au>, "Arnd Bergmann" <arnd@...nel.org>
Cc: "Nathan Chancellor" <nathan@...nel.org>,
 "Anders Roxell" <anders.roxell@...aro.org>,
 "Kees Cook" <keescook@...omium.org>, "Baoquan He" <bhe@...hat.com>,
 "Niklas Schnelle" <schnelle@...ux.ibm.com>,
 "Naresh Kamboju" <naresh.kamboju@...aro.org>,
 clang-built-linux <llvm@...ts.linux.dev>,
 "Nick Desaulniers" <ndesaulniers@...gle.com>, linux-kernel@...r.kernel.org,
 "Nicholas Piggin" <npiggin@...il.com>,
 "Aneesh Kumar K . V" <aneesh.kumar@...nel.org>,
 "Jeff Xu" <jeffxu@...omium.org>, "Mike Rapoport" <rppt@...nel.org>,
 "Justin Stitt" <justinstitt@...gle.com>,
 "Naveen N. Rao" <naveen.n.rao@...ux.ibm.com>,
 linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
 "Dan Carpenter" <dan.carpenter@...aro.org>,
 "Bill Wendling" <morbo@...gle.com>
Subject: Re: [PATCH] powerpc: drop port I/O helpers for CONFIG_HAS_IOPORT=n

On Fri, Apr 19, 2024, at 07:12, Michael Ellerman wrote:
> Michael Ellerman <mpe@...erman.id.au> writes:
>> "Arnd Bergmann" <arnd@...db.de> writes:
>>>
>>> I had included this at first, but then I still ran into
>>> the same warnings because it ends up pulling in the
>>> generic outsb() etc from include/asm-generic/io.h
>>> that relies on setting a non-NULL PCI_IOBASE.
>>
>> Yes you're right. The above fixes the gcc build, but not clang.
>>
>> So I think I'll just cherry pick f0a816fb12da ("/dev/port: don't compile
>> file operations without CONFIG_DEVPORT") into my next and then apply
>> this. But will see if there's any other build failures over night.
>
> That didn't work. Still lots of drivers in my tree (based on rc2) which
> use inb/outb etc, and barf on the empty #define inb.

Right, the patches from Niklas only went into linux-next so far,
and a few are missing (including the 8250 one I think), so -rc2
at the moment regresses, but that doesn't have the warning either.

The idea of my patch was to both fix the current linux-next
build regression and have something that works in the long
run, I didn't expect it to work by itself. Sorry that wasn't
clear from my description.

> So I think this patch needs to wait until all the CONFIG_HAS_IOPORT
> checks have been merged for various drivers.
>
> For now the below fixes the clang warning. AFAICS it's safe because any
> code using inb() etc. with CONFIG_PCI=n is currently just doing a plain
> load from virtual address ~zero which should fault anyway.

If the port number is high enough, the current code might end
up referencing a user space address, depending on mmap_min_addr,
which defaults to 4096.

Using POISON_POINTER_DELTA is clearly an improvement over that.

> diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
> index 08c550ed49be..1cd6eb6c8101 100644
> --- a/arch/powerpc/include/asm/io.h
> +++ b/arch/powerpc/include/asm/io.h
> @@ -37,7 +37,7 @@ extern struct pci_dev *isa_bridge_pcidev;
>   * define properly based on the platform
>   */
>  #ifndef CONFIG_PCI
> -#define _IO_BASE       0
> +#define _IO_BASE       POISON_POINTER_DELTA
>  #define _ISA_MEM_BASE  0
>  #define PCI_DRAM_OFFSET 0
>  #elif defined(CONFIG_PPC32)

You may need to double-check, but I think for ppc64 we
can just unconditionally set _IO_BASE to ISA_IO_BASE
regardless of CONFIG_PCI. 3d5134ee8341 ("[POWERPC]
Rewrite IO allocation & mapping on powerpc64") ensured
that the I/O space is only ever mapped to this virtual
address, and the same method is used with the
asm-generic/io.h implementation on arm/arm64/loongarch/
m68k/riscv/xtensa. Using this would be both safer and
more efficient than the current version.
It should also not cause any regressions ;-)

Unfortunately, ppc32 never got that cleanup, so
POISON_POINTER_DELTA is probably still best until Niklas's
series is merged. You could set _ISA_MEM_BASE to the same 
here for good measure.

[another side note: the non-zero PCI_DRAM_OFFSET looks
unnecessary as well now, apparently this was meant for
ibm cpc710 and ppc440 platforms that are no longer
supported.]

     Arnd