linux-kernel - Re: [PATCH] irqdesc: Fail check on early_irq

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: <87edgvxb56.wl-maz@kernel.org>
Date:   Sun, 12 Nov 2023 15:08:05 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     Paran Lee <p4ranlee@...il.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>, linux-kernel@...r.kernel.org,
        shjy180909@...il.com, austindh.kim@...il.com
Subject: Re: [PATCH] irqdesc: Fail check on early_irq_init allocation.

On Sun, 12 Nov 2023 14:19:28 +0000,
Paran Lee <p4ranlee@...il.com> wrote:
> On 2023-11-12 오후 11:00, Marc Zyngier wrote:
> 
> Thanks for the code review Marc!
> 
> I think function alloc_descs() in irqdesc.c has also alloc_desc() fail
> handling, and there's kernel-wide code consistency checking for
> allocation failures, and I thought it would be nice to mark it.

alloc_descs() and early_irq_init() are very different beasts. The
former can be used *at any time* over the kernel's lifetime, while the
latter is only used *once*. This makes a whole lot a difference, don't
you think?

> So that the code is aware of it.
> 
> Even if it panics with a null derefence reference.

Don't you think it is a bit pointless to trade a fatal error for
another one?

> 
> > A failing allocation already results in a massive splat describing how
> > the allocation failed. Further use of the NULL pointer will also
> > result in a terminal oops, particularly if this happens this early in
> > the boot sequence.
> > 
> > So what do these BUG_ON() calls buy us?
> > 
> > 	M.
> > 
> 
> If anyone has any ideas on how to get a little fancier with the allocation,
> I'll send a v2 patch in that direction.

It's not about being fancy. It is about being useful. Your BUG_ON()s
are not making things any better for early allocation failures.

A much better idea would be to *get rid* of early allocation failures
altogether, by moving all architectures to SPARSE_IRQ and making sure
that NR_LEGAY_IRQ is always zero, meaning there is nothing to
allocate. That would be something useful.

But adding random BUG_ON() based on the dogma that all allocations
must be checked doesn't bring value to the kernel as a whole.

	M.

-- 
Without deviation from the norm, progress is not possible.