Hi

Am 01.06.26 um 21:52 schrieb Jani Nikula:
On Mon, 01 Jun 2026, Thomas Zimmermann <[email protected]> wrote:
Hi

Am 01.06.26 um 17:49 schrieb Jani Nikula:
On Mon, 01 Jun 2026, Jani Nikula <[email protected]> wrote:
On Mon, 01 Jun 2026, Jani Nikula <[email protected]> wrote:
On Fri, 29 May 2026, Thomas Zimmermann <[email protected]> wrote:
Rajat Gupta (1):
        drm: prevent integer overflows in dumb buffer creation helpers
Looks like this commit 5ab62dd3687b ("drm: prevent integer overflows in
dumb buffer creation helpers") regressed in our CI, awaiting
confirmation.
That CI report is where?
It's currently really thin on details, I'm afraid [1].

[1] https://gitlab.freedesktop.org/drm/i915/kernel/-/work_items/16308

The IGT test kms_big_fb uses max width and height from GetResources, and
i915 and xe use max_width 16384 and max_height 16384 in mode config.

The regressing commit adds random hard limits not based on anything:

+       /* Reject unreasonable inputs early.  Dumb buffers are for software
+        * rendering; nothing legitimate needs more than 8192x8192 at 32bpp.
+        * This prevents overflows in downstream alignment helpers.
+        */
+       if (args->width >= 8192 || args->height >= 8192 || args->bpp > 32)
+               return -EINVAL;

This is now in v7.1-rc6. Please revert ASAP.
Ah, missed this clue in the pull request:

On Fri, 29 May 2026, Thomas Zimmermann <[email protected]> wrote:
here is this week's PR from drm-misc-fixes. There's one cross-subsys
commit to the dma-buf code. Commit 5ab62dd3687b ("drm: prevent integer
overflows in dumb buffer creation helpers") has not Link tag because
it went through the security list.
We have the whole review and CI processes in place to catch silly
mistakes, and then we proceed to shoot ourselves in the foot and bypass
all of that because "security", and expedite the regressions
everywhere. I'll bet this will be in stable kernels in no time too. This
is stupid.
Indeed. But that's how this fix got in.

It fixes a possible overflow elsewhere and using dumb buffers with
higher values that given here is questionable. Instead of outright
reverting this, let's first look what actually broke.
There's the kms_big_fb.c test [2]. It uses drmModeGetResources() to get
the max framebuffer size for the device. It's 16384x16384 for most Intel
devices that aren't ancient [3]. (Ditto for AMD I think.) Now the
CREATE_DUMB ioctl fails, because 5ab62dd3687b introduced new arbitrary
fixed scanout size limits that don't reflect the actual device limits.

Then let's increase the size limits to 16384 and the bpp to 64. That's still within uint32 and (AFAICT) should support all devices.


Why is it questionable to use dumb buffers that are within the device
limits? Why is it not questionable to add arbitrary fixed limits?

Buffers of that size can be larger than the PCI BAR, which results in mapping errors. So we'll have to enforce some limits on dumb buffers at some point soon. (Driver-native ioctls is another story.)



Don't get me wrong, fixing overflows is great, but this part is an
unwarranted UABI change, and they shouldn't be rushed in masqueraded as
security fixes.

It's all there to make it safe. [1] I've gone through drivers and converted the majority of dumb-buffer code to the helpers, but it's whack-a-mole in the end.

[1] https://elixir.bootlin.com/linux/v7.0.10/source/drivers/gpu/drm/drm_dumb_buffers.c#L94

Best regards
Thomas



BR,
Jani.


[2] 
https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/intel/kms_big_fb.c?ref_type=heads#L994

[3] 
https://gitlab.freedesktop.org/drm/i915/kernel/-/blob/drm-intel-next/drivers/gpu/drm/i915/display/intel_display_driver.c?ref_type=heads#L133



--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)


Reply via email to