Hi Geert,
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
currently loaded when a) loading a drm driver (which would need that code
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.
> 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.
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.
Now, my understanding is that GPU_BUDDY is also not meant to be selectable
directly by the user, but rather implicitly by modules that need it. It has
nothing that needs actual input from the end user/sysadmin, either a module
needs it and builds it, or it doesn't and it doesn't build it.
The only conditional code I could find depending on CONFIG_GPU_BUDDY is in
rust where the gpu and buddy modules are exposed in the kernel crate only if
the config is selected, but I'm pretty sure it's just how they handle what
we do in Makefiles for modules with obj-{CONFIG_SOMETHING}; and some renames
of variables/functions also for rust. But no behavioral changes depending
on GPU_BUDDY.
So I see no reason to enable the end user to select GPU_BUDDY or not, I'd
just let it be an implicit dependency. It's not really a big fat broad
option like CONFIG_DRM so I'd just treat it like one of those specific
functionality ones due to how similar it is to those.
Then Kconfig for tests has to "select" instead of "depends on" because
there's otherwise no way for it to load buddy, which it needs to function.
> 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
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.
> Fixes: 8fe3fc37564920ae ("gpu: Fix dependencies in
> CONFIG_GPU_BUDDY_KUNIT_TEST")
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> drivers/gpu/Kconfig | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/Kconfig b/drivers/gpu/Kconfig
> index 3bcf08260d73a782..d881ab6f7a006a3b 100644
> --- 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.
> 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.
> --
> 2.43.0
>
Thanks
Krzysztof
