[PATCH] drm/vkms: fix gamma LUT size check

[PATCH] drm/vkms: fix gamma LUT size check

[Renjun Wang]

vkms_atomic_check() computed the gamma LUT entry count using
sizeof(struct drm_color_lut *), which uses pointer size and
can incorrectly reject or accept LUT sizes. Use
drm_color_lut_size() instead to validate against VKMS_LUT_SIZE.

Signed-off-by: Renjun Wang <renjunw0@foxmail.com>
---
 drivers/gpu/drm/vkms/vkms_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index dd1402f43773..a09589949f48 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -17,6 +17,7 @@
 #include <drm/drm_gem.h>
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_color_mgmt.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fbdev_shmem.h>
 #include <drm/drm_file.h>
@@ -111,8 +112,7 @@ static int vkms_atomic_check(struct drm_device *dev, struct drm_atomic_state *st
 		if (!new_crtc_state->gamma_lut || !new_crtc_state->color_mgmt_changed)
 			continue;
 
-		if (new_crtc_state->gamma_lut->length / sizeof(struct drm_color_lut *)
-		    > VKMS_LUT_SIZE)
+		if (drm_color_lut_size(new_crtc_state->gamma_lut) > VKMS_LUT_SIZE)
 			return -EINVAL;
 	}
 
-- 
2.47.3

Re: [PATCH] drm/vkms: fix gamma LUT size check

[Louis Chauvet]

On 2/4/26 15:15, Renjun Wang wrote:
> vkms_atomic_check() computed the gamma LUT entry count using
> sizeof(struct drm_color_lut *), which uses pointer size and
> can incorrectly reject or accept LUT sizes. Use
> drm_color_lut_size() instead to validate against VKMS_LUT_SIZE.
> 
> Signed-off-by: Renjun Wang <renjunw0@foxmail.com>

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

Like for the YUV patch, were you able to reproduce it using a tool?

> ---
>   drivers/gpu/drm/vkms/vkms_drv.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index dd1402f43773..a09589949f48 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -17,6 +17,7 @@
>   #include <drm/drm_gem.h>
>   #include <drm/drm_atomic.h>
>   #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_color_mgmt.h>
>   #include <drm/drm_drv.h>
>   #include <drm/drm_fbdev_shmem.h>
>   #include <drm/drm_file.h>
> @@ -111,8 +112,7 @@ static int vkms_atomic_check(struct drm_device *dev, struct drm_atomic_state *st
>   		if (!new_crtc_state->gamma_lut || !new_crtc_state->color_mgmt_changed)
>   			continue;
>   
> -		if (new_crtc_state->gamma_lut->length / sizeof(struct drm_color_lut *)
> -		    > VKMS_LUT_SIZE)
> +		if (drm_color_lut_size(new_crtc_state->gamma_lut) > VKMS_LUT_SIZE)
>   			return -EINVAL;
>   	}
>   

Re: [PATCH] drm/vkms: fix gamma LUT size check

[Renjun Wang]

Hi Louis,

Actually for a 64-bit machine, the value of sizeof(struct
drm_color_lut) and sizeof(struct drm_color_lut*) is equal. But for
32-bit machine, it is not equal. The definition of struct 
drm_color_lut shown as follows:

struct drm_color_lut {
	__u16 red;
	__u16 green;
	__u16 blue;
	__u16 reserved;
}

There is one test case in igt-gpu-tools for gamma, and the 32-bit
machine test results shown as below:

before patch:

# IGT_FORCE_DRIVER=vkms ./kms_color  --run-subtest gamma
[241590.953610] Console: switching to colour dummy device 80x25
[241590.955637] [IGT] kms_color: executing
IGT-Version: 2.3-NO-GIT (riscv32) (Linux: 6.18.7 riscv32)
Using IGT_SRANDOM=1770864024 for randomisation
Opened device: /dev/dri/card0
[241590.966858] [IGT] kms_color: starting subtest gamma
Starting subtest: gamma
[241590.968030] [IGT] kms_color: starting dynamic subtest pipe-A-
Virtual-1
Starting dynamic subtest: pipe-A-Virtual-1
(kms_color:235) igt_kms-CRITICAL: Test assertion failure function
igt_pipe_commit, file ../lib/igt_kms.c:4212:
(kms_color:235) igt_kms-CRITICAL: Failed assertion: ret == 0
(kms_color:235) igt_kms-CRITICAL: Last errno: 22, Invalid argument
(kms_color:235) igt_kms-CRITICAL: error: -22 != 0
Stack trace: not implemented
Dynamic subtest pipe-A-Virtual-1 failed.

after patch:

# uname -a
Linux buildroot 6.18.7 #2 SMP Mon Feb  9 15:22:27 CST 2026 riscv32
GNU/Linux
# pwd
/usr/libexec/igt-gpu-tools
# lsmod|grep vkms
vkms                   53248  0
drm_shmem_helper       20480  2 vkms
drm_client_lib         12288  1 vkms
drm_kms_helper        122880  3 vkms,drm_shmem_helper,drm_client_lib
drm                   458752  5
vkms,drm_shmem_helper,drm_client_lib,drm_kms_helper
# IGT_FORCE_DRIVER=vkms ./kms_color  --run-subtest gamma
[  111.981381] Console: switching to colour dummy device 80x25
[  111.983078] [IGT] kms_color: executing
IGT-Version: 2.3-NO-GIT (riscv32) (Linux: 6.18.7 riscv32)
Using IGT_SRANDOM=1770864535 for randomisation
Opened device: /dev/dri/card0
[  111.992250] [IGT] kms_color: starting subtest gamma
Starting subtest: gamma
[  111.993166] [IGT] kms_color: starting dynamic subtest pipe-A-
Virtual-1
Starting dynamic subtest: pipe-A-Virtual-1
Dynamic subtest pipe-A-Virtual-1: SUCCESS (7.498s)
[  119.492455] [IGT] kms_color: finished subtest pipe-A-Virtual-1,
SUCCESS
Subtest gamma: SUCCESS (7.501s)
[  119.495286] [IGT] kms_color: finished subtest gamma, SUCCESS
[  119.497233] [IGT] kms_color: exiting, ret=0
[  119.517787] Console: switching to colour frame buffer device 128x48

Best Regards,
Renjun Wang


On Fri, 2026-02-06 at 14:53 +0100, Louis Chauvet wrote:
> 
> 
> On 2/4/26 15:15, Renjun Wang wrote:
> > vkms_atomic_check() computed the gamma LUT entry count using
> > sizeof(struct drm_color_lut *), which uses pointer size and
> > can incorrectly reject or accept LUT sizes. Use
> > drm_color_lut_size() instead to validate against VKMS_LUT_SIZE.
> > 
> > Signed-off-by: Renjun Wang <renjunw0@foxmail.com>
> 
> Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
> 
> Like for the YUV patch, were you able to reproduce it using a tool?
> 
> > ---
> >   drivers/gpu/drm/vkms/vkms_drv.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c
> > b/drivers/gpu/drm/vkms/vkms_drv.c
> > index dd1402f43773..a09589949f48 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > @@ -17,6 +17,7 @@
> >   #include <drm/drm_gem.h>
> >   #include <drm/drm_atomic.h>
> >   #include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_color_mgmt.h>
> >   #include <drm/drm_drv.h>
> >   #include <drm/drm_fbdev_shmem.h>
> >   #include <drm/drm_file.h>
> > @@ -111,8 +112,7 @@ static int vkms_atomic_check(struct drm_device
> > *dev, struct drm_atomic_state *st
> >   		if (!new_crtc_state->gamma_lut || !new_crtc_state-
> > >color_mgmt_changed)
> >   			continue;
> >   
> > -		if (new_crtc_state->gamma_lut->length /
> > sizeof(struct drm_color_lut *)
> > -		    > VKMS_LUT_SIZE)
> > +		if (drm_color_lut_size(new_crtc_state->gamma_lut)
> > > VKMS_LUT_SIZE)
> >   			return -EINVAL;
> >   	}
> >   

[PATCH 1/2] drm/vkms: Cache composer line buffers in vkms_output

[oushixiong1025]

From: Shixiong Ou <oushixiong@kylinos.cn>

compose_active_planes() kvmalloc/kvfree two line buffers every vblank
frame, but their size depends only on hdisplay which changes only on
modeset.  Cache them in vkms_output and reallocate only when the width
changes, avoiding repeated alloc/free overhead.

Register a drmm action to free the buffers when the device is released.

Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
 drivers/gpu/drm/vkms/vkms_composer.c | 44 ++++++++++++++++------------
 drivers/gpu/drm/vkms/vkms_crtc.c     | 12 ++++++++
 drivers/gpu/drm/vkms/vkms_drv.h      |  7 +++++
 3 files changed, 45 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index 83d217085ad0..f3fb203946e8 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -541,11 +541,11 @@ static int check_iosys_map(struct vkms_crtc_state *crtc_state)
 
 static int compose_active_planes(struct vkms_writeback_job *active_wb,
 				 struct vkms_crtc_state *crtc_state,
+				 struct vkms_output *out,
 				 u32 *crc32)
 {
 	size_t line_width, pixel_size = sizeof(struct pixel_argb_u16);
 	struct line_buffer output_buffer, stage_buffer;
-	int ret = 0;
 
 	/*
 	 * This check exists so we can call `crc32_le` for the entire line
@@ -565,27 +565,35 @@ static int compose_active_planes(struct vkms_writeback_job *active_wb,
 	stage_buffer.n_pixels = line_width;
 	output_buffer.n_pixels = line_width;
 
-	stage_buffer.pixels = kvmalloc(line_width * pixel_size, GFP_KERNEL);
-	if (!stage_buffer.pixels) {
-		DRM_ERROR("Cannot allocate memory for the output line buffer");
-		return -ENOMEM;
-	}
+	if (out->composer_buffer_width != line_width) {
+		kvfree(out->composer_stage_buffer);
+		kvfree(out->composer_output_buffer);
+		out->composer_buffer_width = 0;
+
+		out->composer_stage_buffer = kvmalloc(line_width * pixel_size, GFP_KERNEL);
+		if (!out->composer_stage_buffer) {
+			DRM_ERROR("Cannot allocate memory for the output line buffer");
+			return -ENOMEM;
+		}
+
+		out->composer_output_buffer = kvmalloc(line_width * pixel_size, GFP_KERNEL);
+		if (!out->composer_output_buffer) {
+			DRM_ERROR("Cannot allocate memory for intermediate line buffer");
+			kvfree(out->composer_stage_buffer);
+			out->composer_stage_buffer = NULL;
+			return -ENOMEM;
+		}
 
-	output_buffer.pixels = kvmalloc(line_width * pixel_size, GFP_KERNEL);
-	if (!output_buffer.pixels) {
-		DRM_ERROR("Cannot allocate memory for intermediate line buffer");
-		ret = -ENOMEM;
-		goto free_stage_buffer;
+		out->composer_buffer_width = line_width;
 	}
 
+	stage_buffer.pixels = out->composer_stage_buffer;
+	output_buffer.pixels = out->composer_output_buffer;
+
 	blend(active_wb, crtc_state, crc32, &stage_buffer,
 	      &output_buffer, line_width * pixel_size);
 
-	kvfree(output_buffer.pixels);
-free_stage_buffer:
-	kvfree(stage_buffer.pixels);
-
-	return ret;
+	return 0;
 }
 
 /**
@@ -644,9 +652,9 @@ void vkms_composer_worker(struct work_struct *work)
 		return;
 
 	if (wb_pending)
-		ret = compose_active_planes(active_wb, crtc_state, &crc32);
+		ret = compose_active_planes(active_wb, crtc_state, out, &crc32);
 	else
-		ret = compose_active_planes(NULL, crtc_state, &crc32);
+		ret = compose_active_planes(NULL, crtc_state, out, &crc32);
 
 	if (ret)
 		return;
diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index 079abfba427d..6e03c9a1fbd8 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -13,6 +13,14 @@
 
 #include "vkms_drv.h"
 
+static void vkms_composer_buffers_release(struct drm_device *dev, void *data)
+{
+	struct vkms_output *out = data;
+
+	kvfree(out->composer_stage_buffer);
+	kvfree(out->composer_output_buffer);
+}
+
 static bool vkms_crtc_handle_vblank_timeout(struct drm_crtc *crtc)
 {
 	struct vkms_output *output = drm_crtc_to_vkms_output(crtc);
@@ -237,5 +245,9 @@ struct vkms_output *vkms_crtc_init(struct drm_device *dev, struct drm_plane *pri
 	if (IS_ERR(vkms_out->composer_workq))
 		return ERR_CAST(vkms_out->composer_workq);
 
+	ret = drmm_add_action_or_reset(dev, vkms_composer_buffers_release, vkms_out);
+	if (ret)
+		return ERR_PTR(ret);
+
 	return vkms_out;
 }
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 0933e4ce0ff0..e6d58b396451 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -214,6 +214,9 @@ struct vkms_crtc_state {
  *		      writeback)
  * @composer_state: Protected by @lock, current state of this VKMS output
  * @composer_lock: Lock used internally to protect @composer_state members
+ * @composer_stage_buffer: Cached line buffer for plane pixel read and pre-blend transform
+ * @composer_output_buffer: Cached line buffer for blending output
+ * @composer_buffer_width: Current allocated width of the cached buffers (in pixels)
  */
 struct vkms_output {
 	struct drm_crtc crtc;
@@ -226,6 +229,10 @@ struct vkms_output {
 	struct vkms_crtc_state *composer_state;
 
 	spinlock_t composer_lock;
+
+	struct pixel_argb_u16 *composer_stage_buffer;
+	struct pixel_argb_u16 *composer_output_buffer;
+	size_t composer_buffer_width;
 };
 
 struct vkms_config;
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

[PATCH] drm/vkms: Fix gamma_lut size check

[oushixiong1025]

From: Shixiong Ou <oushixiong@kylinos.cn>

The gamma_lut size check in vkms_atomic_check() uses
sizeof(struct drm_color_lut *) instead of sizeof(struct drm_color_lut).
On 64-bit both happen to be 8 bytes, so the check works by accident.
On 32-bit sizeof(pointer) is 4 bytes while sizeof(struct drm_color_lut)
is 8 bytes, which would incorrectly reject valid 256-entry LUTs.

Fixes: db1f254f2cfa ("drm/vkms: Add support to 1D gamma LUT")

Signed-off-by: Shixiong Ou <oushixiong@kylinos.cn>
---
 drivers/gpu/drm/vkms/vkms_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
index 5a640b531d88..2db3971f72c3 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.c
+++ b/drivers/gpu/drm/vkms/vkms_drv.c
@@ -112,7 +112,7 @@ static int vkms_atomic_check(struct drm_device *dev, struct drm_atomic_commit *s
 		if (!new_crtc_state->gamma_lut || !new_crtc_state->color_mgmt_changed)
 			continue;
 
-		if (new_crtc_state->gamma_lut->length / sizeof(struct drm_color_lut *)
+		if (new_crtc_state->gamma_lut->length / sizeof(struct drm_color_lut)
 		    > VKMS_LUT_SIZE)
 			return -EINVAL;
 	}
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus