[PATCH] drm/tiny: fix VRAM/EDID/transfer bounds in bochs, gm12u320, pixpaper, and sharp-memory

[PATCH] drm/tiny: fix VRAM/EDID/transfer bounds in bochs, gm12u320, pixpaper, and sharp-memory

[Hui Peng]

Fix framebuffer size, EDID extension count, and transfer buffer bounds
checks across drivers/gpu/drm/tiny/ (bochs.c, gm12u320.c, pixpaper.c,
and sharp-memory.c).

Fixes: 77b8cabf3d52 ("drm/gm12u320: Move driver to drm/tiny")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index e2d957e51505..19d0af677321 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -422,6 +422,8 @@ static int bochs_primary_plane_helper_atomic_check(struct drm_plane *plane,
 						   struct drm_atomic_commit *state)
 {
 	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
+	struct drm_framebuffer *fb = new_plane_state->fb;
+	struct bochs_device *bochs = to_bochs_device(plane->dev);
 	struct drm_crtc *new_crtc = new_plane_state->crtc;
 	struct drm_crtc_state *new_crtc_state = NULL;
 	int ret;
@@ -438,6 +440,9 @@ static int bochs_primary_plane_helper_atomic_check(struct drm_plane *plane,
 	else if (!new_plane_state->visible)
 		return 0;
 
+	if (fb && (u64)fb->pitches[0] * fb->height > bochs->fb_size)
+		return -EINVAL;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
index 4ad074337af0..9dfbfc6bb1b6 100644
--- a/drivers/gpu/drm/tiny/gm12u320.c
+++ b/drivers/gpu/drm/tiny/gm12u320.c
@@ -268,12 +268,18 @@ static void gm12u320_copy_fb_to_blocks(struct gm12u320_device *gm12u320)
 	x2 = gm12u320->fb_update.rect.x2;
 	y1 = gm12u320->fb_update.rect.y1;
 	y2 = gm12u320->fb_update.rect.y2;
-	vaddr = gm12u320->fb_update.src_map.vaddr; /* TODO: Use mapping abstraction properly */
+
+	ret = drm_gem_fb_vmap(fb, &gm12u320->fb_update.src_map, NULL);
+	if (ret) {
+		GM12U320_ERR("drm_gem_fb_vmap err: %d\n", ret);
+		goto put_fb;
+	}
+	vaddr = gm12u320->fb_update.src_map.vaddr;
 
 	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
 	if (ret) {
 		GM12U320_ERR("drm_gem_fb_begin_cpu_access err: %d\n", ret);
-		goto put_fb;
+		goto vunmap_fb;
 	}
 
 	src = vaddr + y1 * fb->pitches[0] + x1 * 4;
@@ -311,6 +317,8 @@ static void gm12u320_copy_fb_to_blocks(struct gm12u320_device *gm12u320)
 	}
 
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
+vunmap_fb:
+	drm_gem_fb_vunmap(fb, &gm12u320->fb_update.src_map);
 put_fb:
 	drm_framebuffer_put(fb);
 	gm12u320->fb_update.fb = NULL;
@@ -418,6 +426,7 @@ static void gm12u320_fb_mark_dirty(struct drm_framebuffer *fb,
 	} else {
 		struct drm_rect *rect = &gm12u320->fb_update.rect;
 
+		gm12u320->fb_update.src_map = *map;
 		rect->x1 = min(rect->x1, dirty->x1);
 		rect->y1 = min(rect->y1, dirty->y1);
 		rect->x2 = max(rect->x2, dirty->x2);
@@ -583,8 +592,17 @@ static void gm12u320_pipe_update(struct drm_simple_display_pipe *pipe,
 	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
 	struct drm_rect rect;
 
-	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
+	if (!state->fb) {
+		gm12u320_stop_fb_update(to_gm12u320(pipe->crtc.dev));
+		return;
+	}
+
+	if (drm_atomic_helper_damage_merged(old_state, state, &rect)) {
 		gm12u320_fb_mark_dirty(state->fb, &shadow_plane_state->data[0], &rect);
+	} else if (old_state->fb != state->fb) {
+		drm_rect_init(&rect, 0, 0, state->fb->width, state->fb->height);
+		gm12u320_fb_mark_dirty(state->fb, &shadow_plane_state->data[0], &rect);
+	}
 }
 
 static const struct drm_simple_display_pipe_funcs gm12u320_pipe_funcs = {
diff --git a/drivers/gpu/drm/tiny/pixpaper.c b/drivers/gpu/drm/tiny/pixpaper.c
index d02ac26d007c..475e92c3410e 100644
--- a/drivers/gpu/drm/tiny/pixpaper.c
+++ b/drivers/gpu/drm/tiny/pixpaper.c
@@ -865,7 +865,7 @@ static void pixpaper_plane_atomic_update(struct drm_plane *plane,
 	struct drm_shadow_plane_state *shadow_plane_state =
 		to_drm_shadow_plane_state(plane_state);
 	struct drm_crtc *crtc = plane_state->crtc;
-	struct pixpaper_panel *panel = to_pixpaper_panel(crtc->dev);
+	struct pixpaper_panel *panel = to_pixpaper_panel(plane->dev);
 
 	struct drm_device *drm = &panel->drm;
 	struct drm_framebuffer *fb = plane_state->fb;
@@ -875,6 +875,9 @@ static void pixpaper_plane_atomic_update(struct drm_plane *plane,
 	__le32 *src_pixels = NULL;
 	struct pixpaper_error_ctx err = { .errno_code = 0 };
 
+	if (!crtc || !fb)
+		return;
+
 	if (!drm_dev_enter(drm, &idx))
 		return;
 
diff --git a/drivers/gpu/drm/tiny/sharp-memory.c b/drivers/gpu/drm/tiny/sharp-memory.c
index 7efd7b567f3b..b365e7549666 100644
--- a/drivers/gpu/drm/tiny/sharp-memory.c
+++ b/drivers/gpu/drm/tiny/sharp-memory.c
@@ -595,6 +595,9 @@ static int sharp_memory_probe(struct spi_device *spi)
 		smd->vcom_mode = SHARP_MEMORY_SOFTWARE_VCOM;
 		smd->sw_vcom_signal = kthread_run(sharp_memory_sw_vcom_signal_thread,
 						  smd, "sw_vcom_signal");
+		if (IS_ERR(smd->sw_vcom_signal))
+			return dev_err_probe(dev, PTR_ERR(smd->sw_vcom_signal),
+					     "Failed to start sw_vcom_signal thread\n");
 
 	} else if (!strcmp("external", vcom_mode_str)) {
 		smd->vcom_mode = SHARP_MEMORY_EXTERNAL_VCOM;
@@ -616,15 +619,21 @@ static int sharp_memory_probe(struct spi_device *spi)
 	ret = sharp_memory_pipe_init(drm, smd, sharp_memory_formats,
 				     ARRAY_SIZE(sharp_memory_formats),
 				     NULL);
-	if (ret)
+	if (ret) {
+		if (smd->sw_vcom_signal)
+			kthread_stop(smd->sw_vcom_signal);
 		return dev_err_probe(dev, ret, "Failed to initialize display pipeline.\n");
+	}
 
 	drm_plane_enable_fb_damage_clips(&smd->plane);
 	drm_mode_config_reset(drm);
 
 	ret = drm_dev_register(drm, 0);
-	if (ret)
+	if (ret) {
+		if (smd->sw_vcom_signal)
+			kthread_stop(smd->sw_vcom_signal);
 		return dev_err_probe(dev, ret, "Failed to register drm device.\n");
+	}
 
 	drm_client_setup(drm, NULL);
 

Re: [PATCH] drm/tiny: fix VRAM/EDID/transfer bounds in bochs, gm12u320, pixpaper, and sharp-memory

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] drm/gm12u320: fb offset dropped due to redundant drm_gem_fb_vmap call
- [High] drm/gm12u320: sleeping in atomic context via gm12u320_stop_fb_update
- [Medium] drm/gm12u320: redundant logic for FB swaps improperly executes for invisible planes
- [Medium] drm/tiny: commit message claims to fix EDID extension count but no code changes exist
--

commit f388ed102067844063cbe52109b60321de9bfe2f
Author: Hui Peng <benquike@gmail.com>

drm/tiny: fix VRAM/EDID/transfer bounds in bochs, gm12u320, pixpaper, and sharp-memory

This commit aims to fix framebuffer size, EDID extension count, and transfer
buffer bounds checks across several tiny DRM drivers.

> Fix framebuffer size, EDID extension count, and transfer buffer bounds
> checks across drivers/gpu/drm/tiny/ (bochs.c, gm12u320.c, pixpaper.c,
> and sharp-memory.c).

[Severity: Medium]
Is the commit message missing some context, or were the EDID extension
count changes left out of this patch? The diff modifies framebuffer
bounds and buffer maps, but it doesn't appear to contain any changes
related to EDID parsing or EDID extension counts.

> diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
> index 4ad074337af07..9dfbfc6bb1b64 100644
> --- a/drivers/gpu/drm/tiny/gm12u320.c
> +++ b/drivers/gpu/drm/tiny/gm12u320.c
> @@ -268,12 +268,18 @@ static void gm12u320_copy_fb_to_blocks(struct gm12u320_device *gm12u320)
>  	x2 = gm12u320->fb_update.rect.x2;
>  	y1 = gm12u320->fb_update.rect.y1;
>  	y2 = gm12u320->fb_update.rect.y2;
> -	vaddr = gm12u320->fb_update.src_map.vaddr; /* TODO: Use mapping abstraction properly */
> +
> +	ret = drm_gem_fb_vmap(fb, &gm12u320->fb_update.src_map, NULL);

[Severity: High]
Does this redundant mapping overwrite the correct mapping provided by the
shadow plane helper and drop the framebuffer offset?

The shadow plane helpers already map the framebuffer and apply the offset
into the shadow plane state data, which gets passed to gm12u320_fb_mark_dirty()
and saved in gm12u320->fb_update.src_map.

By calling drm_gem_fb_vmap() here in gm12u320_copy_fb_to_blocks() with a NULL
data parameter, doesn't it bypass the offset calculation, causing the driver
to read from the base GEM address instead of the correct offset address?

[ ... ]

> @@ -583,8 +592,17 @@ static void gm12u320_pipe_update(struct drm_simple_display_pipe *pipe,
>  	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
>  	struct drm_rect rect;
>  
> -	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
> +	if (!state->fb) {
> +		gm12u320_stop_fb_update(to_gm12u320(pipe->crtc.dev));
> +		return;
> +	}

[Severity: High]
Can this sequence cause us to sleep in an atomic context?

gm12u320_pipe_update() implements the plane's atomic_update callback, which
runs in atomic context where sleeping is forbidden. However, this patch
introduces a call to gm12u320_stop_fb_update(), which directly calls
cancel_delayed_work_sync() and mutex_lock() - both of which can sleep.

> +
> +	if (drm_atomic_helper_damage_merged(old_state, state, &rect)) {
>  		gm12u320_fb_mark_dirty(state->fb, &shadow_plane_state->data[0], &rect);
> +	} else if (old_state->fb != state->fb) {
> +		drm_rect_init(&rect, 0, 0, state->fb->width, state->fb->height);
> +		gm12u320_fb_mark_dirty(state->fb, &shadow_plane_state->data[0], &rect);
> +	}
>  }

[Severity: Medium]
Will this fallback logic bypass visibility checks and incorrectly trigger
redundant full updates?

For visible planes, drm_atomic_helper_damage_merged() detects FB swaps and
returns true, making this else-if block dead code. But if the plane
transitions to invisible (state->visible == false), the helper returns false.
Won't this else-if block then incorrectly trigger and queue a full frame
transfer for an invisible plane?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260919223513.3889412-1-benquike@gmail.com?part=1

Re: [PATCH] drm/tiny: fix VRAM/EDID/transfer bounds in bochs, gm12u320, pixpaper, and sharp-memory

[Thomas Zimmermann]

Hi

I suggest to put that LLM away for a bit and rather try to get one of 
these drivers to work. Once you have that, try to make a test case for 
one of these "problems". And when that is ready, we can see how to fix it.

Best regards
Thomas

Am 20.09.26 um 00:35 schrieb Hui Peng:
> Fix framebuffer size, EDID extension count, and transfer buffer bounds
> checks across drivers/gpu/drm/tiny/ (bochs.c, gm12u320.c, pixpaper.c,
> and sharp-memory.c).
>
> Fixes: 77b8cabf3d52 ("drm/gm12u320: Move driver to drm/tiny")
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---
> diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
> index e2d957e51505..19d0af677321 100644
> --- a/drivers/gpu/drm/tiny/bochs.c
> +++ b/drivers/gpu/drm/tiny/bochs.c
> @@ -422,6 +422,8 @@ static int bochs_primary_plane_helper_atomic_check(struct drm_plane *plane,
>   						   struct drm_atomic_commit *state)
>   {
>   	struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane);
> +	struct drm_framebuffer *fb = new_plane_state->fb;
> +	struct bochs_device *bochs = to_bochs_device(plane->dev);
>   	struct drm_crtc *new_crtc = new_plane_state->crtc;
>   	struct drm_crtc_state *new_crtc_state = NULL;
>   	int ret;
> @@ -438,6 +440,9 @@ static int bochs_primary_plane_helper_atomic_check(struct drm_plane *plane,
>   	else if (!new_plane_state->visible)
>   		return 0;
>   
> +	if (fb && (u64)fb->pitches[0] * fb->height > bochs->fb_size)
> +		return -EINVAL;
> +
>   	return 0;
>   }
>   
> diff --git a/drivers/gpu/drm/tiny/gm12u320.c b/drivers/gpu/drm/tiny/gm12u320.c
> index 4ad074337af0..9dfbfc6bb1b6 100644
> --- a/drivers/gpu/drm/tiny/gm12u320.c
> +++ b/drivers/gpu/drm/tiny/gm12u320.c
> @@ -268,12 +268,18 @@ static void gm12u320_copy_fb_to_blocks(struct gm12u320_device *gm12u320)
>   	x2 = gm12u320->fb_update.rect.x2;
>   	y1 = gm12u320->fb_update.rect.y1;
>   	y2 = gm12u320->fb_update.rect.y2;
> -	vaddr = gm12u320->fb_update.src_map.vaddr; /* TODO: Use mapping abstraction properly */
> +
> +	ret = drm_gem_fb_vmap(fb, &gm12u320->fb_update.src_map, NULL);
> +	if (ret) {
> +		GM12U320_ERR("drm_gem_fb_vmap err: %d\n", ret);
> +		goto put_fb;
> +	}
> +	vaddr = gm12u320->fb_update.src_map.vaddr;
>   
>   	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);
>   	if (ret) {
>   		GM12U320_ERR("drm_gem_fb_begin_cpu_access err: %d\n", ret);
> -		goto put_fb;
> +		goto vunmap_fb;
>   	}
>   
>   	src = vaddr + y1 * fb->pitches[0] + x1 * 4;
> @@ -311,6 +317,8 @@ static void gm12u320_copy_fb_to_blocks(struct gm12u320_device *gm12u320)
>   	}
>   
>   	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
> +vunmap_fb:
> +	drm_gem_fb_vunmap(fb, &gm12u320->fb_update.src_map);
>   put_fb:
>   	drm_framebuffer_put(fb);
>   	gm12u320->fb_update.fb = NULL;
> @@ -418,6 +426,7 @@ static void gm12u320_fb_mark_dirty(struct drm_framebuffer *fb,
>   	} else {
>   		struct drm_rect *rect = &gm12u320->fb_update.rect;
>   
> +		gm12u320->fb_update.src_map = *map;
>   		rect->x1 = min(rect->x1, dirty->x1);
>   		rect->y1 = min(rect->y1, dirty->y1);
>   		rect->x2 = max(rect->x2, dirty->x2);
> @@ -583,8 +592,17 @@ static void gm12u320_pipe_update(struct drm_simple_display_pipe *pipe,
>   	struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(state);
>   	struct drm_rect rect;
>   
> -	if (drm_atomic_helper_damage_merged(old_state, state, &rect))
> +	if (!state->fb) {
> +		gm12u320_stop_fb_update(to_gm12u320(pipe->crtc.dev));
> +		return;
> +	}
> +
> +	if (drm_atomic_helper_damage_merged(old_state, state, &rect)) {
>   		gm12u320_fb_mark_dirty(state->fb, &shadow_plane_state->data[0], &rect);
> +	} else if (old_state->fb != state->fb) {
> +		drm_rect_init(&rect, 0, 0, state->fb->width, state->fb->height);
> +		gm12u320_fb_mark_dirty(state->fb, &shadow_plane_state->data[0], &rect);
> +	}
>   }
>   
>   static const struct drm_simple_display_pipe_funcs gm12u320_pipe_funcs = {
> diff --git a/drivers/gpu/drm/tiny/pixpaper.c b/drivers/gpu/drm/tiny/pixpaper.c
> index d02ac26d007c..475e92c3410e 100644
> --- a/drivers/gpu/drm/tiny/pixpaper.c
> +++ b/drivers/gpu/drm/tiny/pixpaper.c
> @@ -865,7 +865,7 @@ static void pixpaper_plane_atomic_update(struct drm_plane *plane,
>   	struct drm_shadow_plane_state *shadow_plane_state =
>   		to_drm_shadow_plane_state(plane_state);
>   	struct drm_crtc *crtc = plane_state->crtc;
> -	struct pixpaper_panel *panel = to_pixpaper_panel(crtc->dev);
> +	struct pixpaper_panel *panel = to_pixpaper_panel(plane->dev);
>   
>   	struct drm_device *drm = &panel->drm;
>   	struct drm_framebuffer *fb = plane_state->fb;
> @@ -875,6 +875,9 @@ static void pixpaper_plane_atomic_update(struct drm_plane *plane,
>   	__le32 *src_pixels = NULL;
>   	struct pixpaper_error_ctx err = { .errno_code = 0 };
>   
> +	if (!crtc || !fb)
> +		return;
> +
>   	if (!drm_dev_enter(drm, &idx))
>   		return;
>   
> diff --git a/drivers/gpu/drm/tiny/sharp-memory.c b/drivers/gpu/drm/tiny/sharp-memory.c
> index 7efd7b567f3b..b365e7549666 100644
> --- a/drivers/gpu/drm/tiny/sharp-memory.c
> +++ b/drivers/gpu/drm/tiny/sharp-memory.c
> @@ -595,6 +595,9 @@ static int sharp_memory_probe(struct spi_device *spi)
>   		smd->vcom_mode = SHARP_MEMORY_SOFTWARE_VCOM;
>   		smd->sw_vcom_signal = kthread_run(sharp_memory_sw_vcom_signal_thread,
>   						  smd, "sw_vcom_signal");
> +		if (IS_ERR(smd->sw_vcom_signal))
> +			return dev_err_probe(dev, PTR_ERR(smd->sw_vcom_signal),
> +					     "Failed to start sw_vcom_signal thread\n");
>   
>   	} else if (!strcmp("external", vcom_mode_str)) {
>   		smd->vcom_mode = SHARP_MEMORY_EXTERNAL_VCOM;
> @@ -616,15 +619,21 @@ static int sharp_memory_probe(struct spi_device *spi)
>   	ret = sharp_memory_pipe_init(drm, smd, sharp_memory_formats,
>   				     ARRAY_SIZE(sharp_memory_formats),
>   				     NULL);
> -	if (ret)
> +	if (ret) {
> +		if (smd->sw_vcom_signal)
> +			kthread_stop(smd->sw_vcom_signal);
>   		return dev_err_probe(dev, ret, "Failed to initialize display pipeline.\n");
> +	}
>   
>   	drm_plane_enable_fb_damage_clips(&smd->plane);
>   	drm_mode_config_reset(drm);
>   
>   	ret = drm_dev_register(drm, 0);
> -	if (ret)
> +	if (ret) {
> +		if (smd->sw_vcom_signal)
> +			kthread_stop(smd->sw_vcom_signal);
>   		return dev_err_probe(dev, ret, "Failed to register drm device.\n");
> +	}
>   
>   	drm_client_setup(drm, NULL);
>   

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