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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHk-=whLixL8-iYt1qH0-YvEnVsYtryZaN5Da0qoBBhKsBnumw@mail.gmail.com>
Date: Tue, 8 Apr 2025 19:39:55 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Fedor Pchelkin <pchelkin@...ras.ru>
Cc: Christian König <christian.koenig@....com>, 
	Denis Arefev <arefev@...mel.ru>, Alex Deucher <alexander.deucher@....com>, 
	Simona Vetter <simona@...ll.ch>, lvc-project@...uxtesting.org, 
	linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	amd-gfx@...ts.freedesktop.org, David Airlie <airlied@...il.com>
Subject: Re: [lvc-project] [PATCH] drm/amdgpu: check a user-provided number of
 BOs in list

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