Re: [PATCH RFC] drm/cirrus-qemu: Use actual VRAM size to prevent out-of-bounds write

Slawomir Stepien <[email protected]>
Newsgroups dev.linux.lists.syzbot
Message-ID <aowx9LUZgrhDZnSM@nr200>
On sie 16, 2026 23:42, 'syzbot' via syzkaller-upstream-moderation wrote:
> The `cirrus-qemu` driver previously relied on a hardcoded constant
> `CIRRUS_VRAM_SIZE` (4 MB) to validate framebuffer sizes. However, during
> device probe, the driver maps the VRAM using the actual size of the PCI
> device's BAR0.
> 
> If a privileged user unbinds a random PCI device with a BAR0 smaller than 4
> MB and binds the `cirrus-qemu` driver to it, the mapped VRAM will be
> smaller than 4 MB. Because the validation checks still used the hardcoded 4
> MB size, the driver would allow the creation of a framebuffer larger than
> the actually mapped VRAM.
> 
> When the DRM device is closed, `drm_release()` triggers a full atomic
> commit to restore the fbdev mode. This calls
> `cirrus_primary_plane_helper_atomic_update()`, which uses `drm_fb_memcpy()`
> to copy the framebuffer into the mapped VRAM. Since the mapped VRAM is
> smaller than the framebuffer, `memcpy_toio()` writes past the end of the
> mapped I/O memory, resulting in a supervisor write page fault:
> 
> BUG: unable to handle page fault for address: ffffc900033dd000
> #PF: supervisor write access in kernel mode
> #PF: error_code(0x0002) - not-present page
> ...
> RIP: 0010:rep_movs arch/x86/lib/iomem.c:13 [inline]
> RIP: 0010:string_memcpy_toio arch/x86/lib/iomem.c:64 [inline]
> RIP: 0010:memcpy_toio+0x7c/0xe0 arch/x86/lib/iomem.c:110
> ...
> Call Trace:
>  <TASK>
>  iosys_map_memcpy_to include/linux/iosys-map.h:285 [inline]
>  drm_fb_memcpy+0x325/0x5d0 drivers/gpu/drm/drm_format_helper.c:442
>  cirrus_primary_plane_helper_atomic_update+0x98a/0xb00
>  drivers/gpu/drm/tiny/cirrus-qemu.c:358
>  drm_atomic_helper_commit_planes+0x626/0xea0
>  drivers/gpu/drm/drm_atomic_helper.c:3038
>  drm_atomic_helper_commit_tail+0x60/0x510
>  drivers/gpu/drm/drm_atomic_helper.c:1989
>  commit_tail+0x2b1/0x3c0 drivers/gpu/drm/drm_atomic_helper.c:2074
>  drm_atomic_helper_commit+0xa77/0xb10
>  drivers/gpu/drm/drm_atomic_helper.c:2312
> ...
> 
> To fix this, remove the hardcoded `CIRRUS_VRAM_SIZE` and dynamically track
> the actual VRAM size in `struct cirrus_device`. Initialize this size from
> the PCI resource length during probe, ensuring it is non-zero. Update the
> validation hooks `cirrus_primary_plane_helper_atomic_check()` and
> `cirrus_mode_config_mode_valid()` to use the dynamically tracked VRAM size,
> ensuring that framebuffers strictly fit within the mapped memory.
> 
> Fixes: ab3e023b1b4c ("drm/cirrus: rewrite and modernize driver.")
> Assisted-by: Gemini:gemini-3.6-flash Gemini:gemini-3.1-pro-preview syzbot
> Reported-by: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=2442951a6abb004df963
> Link: https://syzkaller.appspot.com/ai_job?id=0995d2c9-3eea-4945-84e7-ab3d2acd4f21
> To: "David Airlie" <[email protected]>
> To: "Dave Airlie" <[email protected]>
> To: <[email protected]>
> To: "Gerd Hoffmann" <[email protected]>
> To: "Maarten Lankhorst" <[email protected]>
> To: "Maxime Ripard" <[email protected]>
> To: "Simona Vetter" <[email protected]>
> To: "Thomas Zimmermann" <[email protected]>
> To: <[email protected]>
> Cc: "Jani Nikula" <[email protected]>
> Cc: <[email protected]>
> Cc: "Luca Ceresoli" <[email protected]>
> 
> ---
> diff --git a/drivers/gpu/drm/tiny/cirrus-qemu.c b/drivers/gpu/drm/tiny/cirrus-qemu.c
> index 075221b43..7f8cb3264 100644
> --- a/drivers/gpu/drm/tiny/cirrus-qemu.c
> +++ b/drivers/gpu/drm/tiny/cirrus-qemu.c
> @@ -55,7 +55,6 @@
>  #define DRIVER_MINOR 0
>  
>  #define CIRRUS_MAX_PITCH (0x1FF << 3)      /* (4096 - 1) & ~111b bytes */
> -#define CIRRUS_VRAM_SIZE (4 * 1024 * 1024) /* 4 MB */
>  
>  struct cirrus_device {
>  	struct drm_device	       dev;
> @@ -68,6 +67,7 @@ struct cirrus_device {
>  
>  	/* HW resources */
>  	void __iomem		       *vram;
> +	size_t			       vram_size;
>  	void __iomem		       *mmio;
>  };
>  
> @@ -298,6 +298,7 @@ static const uint64_t cirrus_primary_plane_format_modifiers[] = {
>  static int cirrus_primary_plane_helper_atomic_check(struct drm_plane *plane,
>  						    struct drm_atomic_commit *state)
>  {
> +	struct cirrus_device *cirrus = to_cirrus(plane->dev);
>  	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
>  	struct drm_framebuffer *fb = new_plane_state->fb;
>  	struct drm_crtc *new_crtc = new_plane_state->crtc;
> @@ -319,7 +320,7 @@ static int cirrus_primary_plane_helper_atomic_check(struct drm_plane *plane,
>  	/* validate size constraints */
>  	if (fb->pitches[0] > CIRRUS_MAX_PITCH)
>  		return -EINVAL;
> -	else if (fb->pitches[0] > CIRRUS_VRAM_SIZE / fb->height)
> +	else if (fb->pitches[0] > cirrus->vram_size / fb->height)
>  		return -EINVAL;
>  
>  	return 0;
> @@ -514,6 +515,7 @@ static int cirrus_pipe_init(struct cirrus_device *cirrus)
>  static enum drm_mode_status cirrus_mode_config_mode_valid(struct drm_device *dev,
>  							  const struct drm_display_mode *mode)
>  {
> +	struct cirrus_device *cirrus = to_cirrus(dev);
>  	const struct drm_format_info *format = drm_format_info(DRM_FORMAT_XRGB8888);
>  	u64 pitch;
>  
> @@ -525,7 +527,7 @@ static enum drm_mode_status cirrus_mode_config_mode_valid(struct drm_device *dev
>  		return MODE_BAD_WIDTH;
>  	if (pitch > CIRRUS_MAX_PITCH)
>  		return MODE_BAD_WIDTH; /* maximum programmable pitch */
> -	if (pitch > CIRRUS_VRAM_SIZE / mode->vdisplay)
> +	if (pitch > cirrus->vram_size / mode->vdisplay)
>  		return MODE_MEM;
>  
>  	return MODE_OK;
> @@ -602,8 +604,12 @@ static int cirrus_pci_probe(struct pci_dev *pdev,
>  
>  	dev = &cirrus->dev;
>  
> +	if (pci_resource_len(pdev, 0) == 0)

How about checking here also if the returned value is == to CIRRUS_VRAM_SIZE and leave the
CIRRUS_VRAM_SIZE as it is right now? Or even better:

if (pci_resource_len(pdev, 0) != CIRRUS_VRAM_SIZE)
	return -ENODEV;

This will make this change a much simpler one!

It seems, after checking the qemu sources, that CLGD5446 must have 4MB and there is not other
option.

> +		return -ENODEV;
> +
> +	cirrus->vram_size = pci_resource_len(pdev, 0);
>  	cirrus->vram = devm_ioremap(&pdev->dev, pci_resource_start(pdev, 0),
> -				    pci_resource_len(pdev, 0));
> +				    cirrus->vram_size);
>  	if (cirrus->vram == NULL)
>  		return -ENOMEM;
>  
> 
> 
> base-commit: db2ddb87143519e20a95aa36c60b36107b736a58

-- 
Slawomir Stepien
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.