[lvc-project] [PATCH] drm/amdgpu: check a user-provided number of BOs in list

[Christian König]

Am 09.04.25 um 19:27 schrieb Linus Torvalds:
> On Wed, 9 Apr 2025 at 00:29, Christian König <christian.koenig at amd.com> wrote:
>> I mean open coding the limit checks everywhere certainly works, but as far as I can see it would be more defensive if we do that inside kvmalloc_array().
> No.
>
> If we add some limit to kvmalloc_array(), I guarantee that people will
> just then call it with ~0UL.
>
> Making it all entirely pointless.

Hehe, yeah good point as well.

> So thus the "kvmalloc_array() warns about unreasonable uses
> unconditionally, at a limit that is high enough to be useful, and low
> enough that the automated code randomization tools will hopefully
> trigger it and find bad code that doesn't validate kernel input".
>
>> BTW we have been running into the kvmalloc() check on valid use cases as well.
> *IF* you do proper validation, you can just use the raw vmalloc()
> interfaces by hand and avoid it all.

The problem is that we have use cases where the same code needs to work from the low end mobile with just a few gigabytes all the way to the HPC data center with up to petabytes of system memory. The kvmalloc approach is just very convenient to handle that since it scales.

To summarize for some use cases I can't come up with good hard coded limits since they basically just depend on the system.

> The VM layer allows larger allocations. But the "this is a simple
> allocation, choose kmalloc or vmalloc automatically based on size"
> helper says "you are being simple, I'm going to check your arguments
> are actually sane".
>
> So the drm code can easily have a function that validates the input
> for your specific cases, and then you (a) don't need the helper
> function that does the overflow protection and (b) don't want it.
>
> But it should actually validate arguments for real sanity at that
> point. Not just open-code kvmalloc() without the sanity check.

Yeah, exactly that has been proposed by driver maintainers before and we just rejected it on the subsystem maintainers level.

For this particular use case here I will propose some hopefully high enough hard coded limit, but I can't guarantee that this will work for all use cases.

Thanks,
Christian.

>
>              Linus