Re: [igt-dev] [PATCH i-g-t] lib/igt_kms: Fix memory corruption

[Vignesh Raman <vignesh.raman@collabora.com>]

Hi Bhanu,

On 26/10/23 10:18, Modem, Bhanuprakash wrote:

> Why don't we use n_pipes = count_crtcs (before allocating the memory)?
> 
> -       display->n_pipes = IGT_MAX_PIPES;
> +       display->n_pipes = resources->count_crtcs;
> 

If we dynamically allocate memory for display->pipes using count_crtcs 
instead of IGT_MAX_PIPES, subtest all-pipes is failing in the below tests,
kms_cursor_legacy@forked-bo
kms_cursor_legacy@forked-move
kms_cursor_legacy@single-bo
kms_cursor_legacy@single-move
kms_cursor_legacy@torture-bo
kms_cursor_legacy@torture-move

Starting dynamic subtest: all-pipes
Stack trace:
   #0 ../lib/igt_core.c:1988 __igt_fail_assert()
   #1 ../tests/kms_cursor_legacy.c:138 stress.constprop.0()
   #2 ../tests/kms_cursor_legacy.c:1942 __igt_unique____real_main1887()
   #3 ../tests/kms_cursor_legacy.c:1887 main()
   #4 [__libc_init_first+0x8a]
   #5 [__libc_start_main+0x85]
   #6 [_start+0x21]
child 0 failed with exit status 98
Dynamic subtest all-pipes: FAIL (0.021s)
Subtest forked-bo: FAIL (43.029s)
Test requirement not met in function igt_require_pipe_crc, file 
../lib/igt_pipe_crc.c:211:
Test requirement: fstatat(dir, "crtc-0/crc/control", &stat, 0) == 0
CRCs not supported on this platform
Last errno: 2, No such file or directory
Test requirement not met in function igt_require_intel, file 
../lib/drmtest.c:787:
Test requirement: is_intel_device(fd)
Last errno: 2, No such file or directory
stderr:
-------
(kms_cursor_legacy:721) CRITICAL: Test assertion failure function 
stress, file ../tests/kms_cursor_legacy.c:158:
(kms_cursor_legacy:721) CRITICAL: Failed assertion: 
igt_ioctl((display->drm_fd), ((((2U|1U) << (((0+8)+8)+14)) | ((('d')) << 
(0+8)) | (((0xA3)) << 0) | ((((sizeof(struct drm_mode_cursor)))) << 
((0+8)+8)))), (&arg)) == 0
(kms_cursor_legacy:721) CRITICAL: Last errno: 2, No such file or directory
(kms_cursor_legacy:721) CRITICAL: error: -1 != 0
(kms_cursor_legacy:722) CRITICAL: Test assertion failure function 
stress, file ../tests/kms_cursor_legacy.c:158:
(kms_cursor_legacy:722) CRITICAL: Failed assertion: 
igt_ioctl((display->drm_fd), ((((2U|1U) << (((0+8)+8)+14)) | ((('d')) << 
(0+8)) | (((0xA3)) << 0) | ((((sizeof(struct drm_mode_cursor)))) << 
((0+8)+8)))), (&arg)) == 0
(kms_cursor_legacy:722) CRITICAL: Last errno: 2, No such file or directory
(kms_cursor_legacy:722) CRITICAL: error: -1 != 0
Dynamic subtest all-pipes failed.

With the initial patch all-pipes subtest was passing.


>> -    for (i = 0; i < resources->count_crtcs; i++) {
>> +    for (i = 0; i < min(resources->count_crtcs, IGT_MAX_PIPES); i++) {
> 
> With this change, we are missing the information of crtc index 7 to 15 
> in the display structure, aren't we?

Yes agree. Do we need to fix the all-pipes subtest failure with your 
proposed solution?

Regards,
Vignesh