linux-kernel - Re: [PATCH] nvme-pci: Make nvme_pci_npages_prp() __always

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]

Message-ID: <202505070849.1F48789B6@keescook>
Date: Wed, 7 May 2025 08:53:06 -0700
From: Kees Cook <kees@...nel.org>
To: Christoph Hellwig <hch@....de>
Cc: Keith Busch <kbusch@...nel.org>, kernel test robot <lkp@...el.com>,
	Jens Axboe <axboe@...nel.dk>, Sagi Grimberg <sagi@...mberg.me>,
	linux-nvme@...ts.infradead.org, Chaitanya Kulkarni <kch@...dia.com>,
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH] nvme-pci: Make nvme_pci_npages_prp() __always_inline

On Wed, May 07, 2025 at 08:59:13AM +0200, Christoph Hellwig wrote:
> On Tue, May 06, 2025 at 10:55:31PM -0700, Kees Cook wrote:
> > On Wed, May 07, 2025 at 06:47:54AM +0200, Christoph Hellwig wrote:
> > > On Tue, May 06, 2025 at 08:35:40PM -0700, Kees Cook wrote:
> > > > The only reason nvme_pci_npages_prp() could be used as a compile-time
> > > > known result in BUILD_BUG_ON() is because the compiler was always choosing
> > > > to inline the function. Under special circumstances (sanitizer coverage
> > > > functions disabled for __init functions on ARCH=um), the compiler decided
> > > > to stop inlining it:
> > > 
> > > Can we place just fix um to still force inlining inline functions instead
> > > of needing these workarounds?
> > 
> > Oh, I don't have the history here. Is there something about UM and
> > forcing off inlining?
> 
> Maybe I'm misunderstandng your report, but what causes the failure
> to inline?

I don't know precisely, but whatever internal heuristics the compiler
uses to change a function from "static" to "static inline" got disrupted
by the build options, and manifested with this failure. It's fully
reproducible on all architectures if I mark the function as "noinline".
:)

So, the solution for the "accidentally depending on a function to be
inlined by the compiler" is to mark it as _required_ to be inlined,
which given its singular use in BUILD_BUG_ON(), looks like the correct
solution.

I took your comment about ARCH=um to mean there was some kind of
long-standing "UM regularly fails to inline stuff; can we fix UM
instead?" But regardless, I think this patch is still correct given
that the compiler could, at any time, decide to make this function not
inline, since it's not marked that way at all (but its usage depends on
it being inline).

-Kees

-- 
Kees Cook