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: <fgd6hrllcwj2guhr4mwzfblhausluczprlbjqhsqiqeshoq2g2@jgi4rgufn6wx>
Date: Sun, 13 Apr 2025 14:31:16 +0300
From: Fedor Pchelkin <pchelkin@...ras.ru>
To: Christian König <christian.koenig@....com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>, 
	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 Thu, 10. Apr 11:07, Christian König wrote:
> Am 09.04.25 um 19:27 schrieb Linus Torvalds:
> > 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.

FWIW, the current code anyway has this limit being some sort of 4Gb, not
more.

The resulting calculation of `bytes` wraps at 32 bits albeit itself being
of type *unsigned long*.

	/* copy the handle array from userspace to a kernel buffer */
	r = -EFAULT;
	if (likely(info_size == in->bo_info_size)) {
		unsigned long bytes = in->bo_number *
			in->bo_info_size;

		if (copy_from_user(info, uptr, bytes))
			goto error_free;