* Re: [PATCH] drm/vkms: fix gamma LUT size check
2026-02-06 13:53 ` Louis Chauvet
@ 2026-02-12 15:04 ` Renjun Wang
0 siblings, 0 replies; 3+ messages in thread
From: Renjun Wang @ 2026-02-12 15:04 UTC (permalink / raw)
To: Louis Chauvet, hamohammed.sa, simona, melissa.srw,
maarten.lankhorst, mripard, tzimmermann, airlied
Cc: dri-devel, linux-kernel
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;
> > }
> >
^ permalink raw reply [flat|nested] 3+ messages in thread