On Tue, 8 Apr 2025 at 09:07, Fedor Pchelkin <pchel...@ispras.ru> 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
