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-=wjLQzkTTDwJ+ZdVdgNKMg958petkdsu-+m7s9UL6PVCRg@mail.gmail.com>
Date: Wed, 9 Apr 2025 10:27:13 -0700
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Christian König <christian.koenig@....com>
Cc: Fedor Pchelkin <pchelkin@...ras.ru>, 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 Wed, 9 Apr 2025 at 00:29, Christian König <christian.koenig@....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.

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 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.

             Linus