Re: [PATCH] irqchip/gic: prevent buffer overflow in gic_ipi_send_mask()

From: Marc Zyngier
Date: Tue Sep 10 2024 - 03:50:41 EST


On Mon, 09 Sep 2024 20:23:21 +0100,
Sergey Shtylyov <s.shtylyov@xxxxxx> wrote:
>
> On 9/5/24 10:29 AM, Thomas Gleixner wrote:
> [...]
>
> >> ARM GIC arch v2 spec claims support for just 8 CPU interfaces. However,
> >> looking at the GIC driver's irq_set_affinity() method, it seems that the
> >> passed CPU mask may contain the logical CPU #s beyond 8, and that method
>
> s/8/7/, of course... :-<
>
> >> filters them out before reading gic_cpu_map[], bailing out with
> >> -EINVAL.
> >
> > The reasoning is correct in theory, but in reality it's a non problem.
>
> Frankly, before finalizing this patch I had tried to ascertain whether
> cpumask could contain CPUs with the logical #s higher than 8 but that was
> taking way too much time and I gave up... :-)

You can't really work it out form the source code. The trick is that
the integration requirements prevent you from doing so. It is as
simple as that. People have built GICv2-like interrupt controllers
with more than 8 CPUs, but it is a different beast (see the hip04
driver for a good laugh).

Another possible hack is to have 2 GICs side by side, and connect up
to 8 CPUs to each. But then you cannot IPI one cluster from another,
and you end-up with the hilarious situation that plagued the Axxia
SoC, which Linux never really supported, because this is utter
nonsense.

>
> > Simply because processors which use this GIC version cannot have more
> > than 8 cores.
>
> And big.LITTLE not involved?

In what sense? Asymmetric configurations don't impact the number of
CPU interfaces that can be connected to a single GIC.

>
> > That means num_possible_cpus() <= 8 so the cpumask handed in cannot have
> > bits >= 8 set. Ergo for_each_cpu() can't return a bit which is >= 8.
>
> Perhaps adding WARN_ON() would make some sense though? :-)

But why? If someone builds something that cannot work, they have
bigger problems than an extra bit in a bitmap, and the kernel is not a
validation suite for idiotic integration (though I wonder at times).

M.

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