Hi Krzysztof,
On Tue, 25 Aug 2026 at 13:39, Krzysztof Niemiec
<[email protected]> wrote:
> On 2026-08-24 at 17:12:30 +0200, Geert Uytterhoeven wrote:
> > Enabling a (modular) test should not silently enable additional kernel
> > functionality, as that may increase the attack vector for a product.
>
> What vector? What product, exactly? As far as I know buddy allocator code is
Any product that uses DRM and has CONFIG_KUNIT_ALL_TESTS=m, so
developers can easily run any relevant test when the need arises.
> currently loaded when a) loading a drm driver (which would need that code
Not just "a drm driver": very few DRM drivers use it.
Hence one can have a product with DRM that does not need the buddy
allocator.
> to function anyway) and/or b) loading the kunit test for the buddy allocator,
> which would need that code to function anyway. And AFAIK neither are
> built unconditionally into the kernel. So the amount of potential vuln
> surface is not exposed beyond what is reasonably required for the system to
> function as intended.
>
> If you've found a concrete bug/exploit/"attack vector" utilizing buddy code,
> then please address that directly.
So let's include all available code in the kernel image anyway, unless
you can point at a concrete bug/exploit/"attack vector"???
> > Fix this by reverting the select to a dependency.
>
> The way drm kunit tests do it is by select-ing the appropriate
> functionality out of the available ones, with "broad" depends-ons for
> DRM and such (say "depends on DRM\n\n select DRM_BUDDY"). DRM_BUDDY,
> for example, is not directly selectable in .config, as it's meant to be
> selected automatically if a module uses it.
In general, test code must not select the code to be tested, but depend
on it, to avoid that enabling CONFIG_KUNIT_ALL_TESTS pulls in lots of
unneeded code.
> This is also the case for drivers, with Xe having depends on DRM and
> then like 40 lines of selects for specific functionality, same for i915,
> amdgpu, so on.
For drivers, it is different: if you want the driver, you obviously
need the helper or library code.
> > Make sure the GPU
> > buddy allocator can be tested without enabling another driver that uses
> > DRM_BUDDY by making GPU_BUDDY visible if KUNIT is enabled.
>
> This was the case even without your patch, and was the entire point of
> 8fe3fc37564920ae mentioned in Fixes. The kunit test kernel can be built with
Yes, that was the intent of commit 8fe3fc37564920ae, but it used the
wrong method.
It is customary to make library code that is only selected by its
users, and cannot be enabled explicitly, visible for testing.
See e.g. commit bf2d44d07de726b0 ("ASoC: wm_adsp: Add kunit test for
firmware file search")
> a rather minimal .kunitconfig (included in the gpu/tests directory). So
> it only loads whatever kunit_tool would load, plus CONFIG_KUNIT, plus
> CONFIG_GPU_BUDDY_KUNIT_TEST, plus (implicitly, which I discussed above)
> CONFIG_GPU_BUDDY. It doesn't load DRM_BUDDY at all (which was the bug
> being fixed in that commit, you used to be forced to load it, now you
> aren't)
>
> > The page based buddy allocator for GPU memory is only used by DRM.
> > Hence add a dependency on DRM to GPU_BUDDY, to prevent asking the user
> > about this code when configuring a kernel without DRM support.
>
> It seems to be *technically* true, however, the entire point of moving the
> buddy allocator outside the DRM directory is to expose it to other non-DRM
> drivers, so the lack of dependency is kind of by design.
>
> The change was rather anticipatory, but it was clearly well reviewed and
> sent by a Joel, a nova-core dev, I assume it's gonna be needed at least
> by them at one point or another.
>
> If, say, nova decides to use the "communal" buddy allocator (if it
> doesn't do it already, I only briefly skimmed it looking for kernel::gpu)
> then the DRM dependency from this patch would have to be reverted (yet
> again). Maybe moving the buddy allocator up from DRM should have been done
> along with the first non-DRM user and only then, but it just didn't go that
> way.
>
> If the issue is that this option is too annoying during make or in
> menuconfig, then I don't think this is the way to address it.
Once the symbol is visible, it needs a proper dependency, to avoid
asking the user irrelevant questions.
> > Fixes: 8fe3fc37564920ae ("gpu: Fix dependencies in
> > CONFIG_GPU_BUDDY_KUNIT_TEST")
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > --- a/drivers/gpu/Kconfig
> > +++ b/drivers/gpu/Kconfig
> > @@ -1,14 +1,15 @@
> > # SPDX-License-Identifier: GPL-2.0
> >
> > config GPU_BUDDY
> > - bool
> > + bool "Page based buddy allocator for GPU memory" if KUNIT
> > + depends on DRM || COMPILE_TEST
>
> What's the justification for COMPILE_TEST here? It might be needed but
> you didn't explain why or mention it in the patch description at all.
It is customary to add "|| COMPILE_TEST" when adding dependencies that
are not strictly needed for a successful build.
> > help
> > A page based buddy allocator for GPU memory.
> >
> > config GPU_BUDDY_KUNIT_TEST
> > tristate "KUnit tests for GPU buddy allocator" if !KUNIT_ALL_TESTS
> > depends on KUNIT
> > - select GPU_BUDDY
> > + depends on GPU_BUDDY
> > default KUNIT_ALL_TESTS
> > help
> > KUnit tests for the GPU buddy allocator.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
