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

[Linus Torvalds]

On Tue, 8 Apr 2025 at 09:07, Fedor Pchelkin <pchelkin@xxxxxxxxx> wrote:
>
> > Linus comment is about kvmalloc(), but the code here is using
> > kvmalloc_array() which as far as I know is explicitly made to safely
> > allocate arrays with parameters provided by userspace.

No.

ABSOLUTELY NOTHING CAN ALLOCATE ARRAYS WITH PARAMETERS PROVIDED BY USER SPACE.

All kvmalloc_array() does is to check for overflow on the multiplication.

That does *not* mean that you can then just blindly take user space
input and pass it to kvmalloc_array().

That could easily cause the machine to run out of memory immediately,
for example. Or just cause huge latency issues. Or any number of other
things.

> [27651.163361] WARNING: CPU: 3 PID: 183060 at mm/util.c:657 __kvmalloc_node_noprof+0xc1/0xd0
> [27651.163411] CPU: 3 UID: 0 PID: 183060 Comm: a.out Not tainted 6.13.9-200.fc41.x86_64 #1
> [27651.163412] Hardware name: ASUS System Product Name/PRIME X670E-PRO WIFI, BIOS 3035 09/05/2024
> [27651.163413] RIP: 0010:__kvmalloc_node_noprof+0xc1/0xd0
> [27651.163424] Call Trace:
> That's just
>
>     union drm_amdgpu_bo_list bo_list;
>     int fd, ret;
>
>     memset(&bo_list, 0, sizeof(bo_list));
>
>     fd = open(DEVICE_PATH, O_RDWR);
>
>     bo_list.in.bo_number = 1 << 31;
>     ret = ioctl(fd, DRM_IOCTL_AMDGPU_BO_LIST, &bo_list);

Yes, exactly, and that's bogus code in the DRM layer to just blindly
trust user space.

User space input absolutely has to be validated for sanity.

There's a very real reason why we have things like PATH_MAX.

Could we allocate any amount of memory for user paths, with the
argument that path length shouldn't be limited to some (pretty small)
number?

Sure. We *could* do that.

And that would be a huge mistake. Limiting and sanity-checking user
space arguments isn't just a good idea - it's an absolute requirement.

So that kvmalloc warning exists *exactly* so that you will get a
warning if you do something stupid like just blindly trust user space.

Because no, "doesn't overflow" isn't even remotely a valid limit. A
real limit on memory allocations - and most other things, for that
matter - needs to be about practical real issues, not about something
like  "this doesn't overflow".

            Linus