Re: [PATCH 03/12] drm/panic: Return -EINVAL if font is not available

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing NULL check for the result of `kmap_local_page_try_from_panic()` in `drm_panic_write_pixel24_xpage()` leads to a NULL pointer dereference.
--

--- Patch [3]: [PATCH 03/12] drm/panic: Return -EINVAL if font is not available ---
commit 5ad93562bf6ffaf36ef771a5c8eebc2aa43f5621
Author: Thomas Zimmermann <[email protected]>

drm/panic: Return -EINVAL if font is not available

Font lookup does not allocate memory. Hence reporting ENOMEM on
failed lookups is misleading. Report EINVAL instead.

> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
> index 705b72bf0d839..96d238bfceee9 100644
> --- a/drivers/gpu/drm/drm_panic.c
> +++ b/drivers/gpu/drm/drm_panic.c

[ ... ]

> static void drm_panic_write_pixel24_xpage(void *vaddr, struct page *next_page,
> 					  unsigned int offset, u32 color)
> {
> 	u8 *vaddr2;
> 	u8 *p = vaddr + offset;
> 
> 	vaddr2 = kmap_local_page_try_from_panic(next_page);
> 
> 	*p++ = color & 0xff;
> 	color >>= 8;
> 
> 	if (offset == PAGE_SIZE - 1)
> 		p = vaddr2;
> 
> 	*p++ = color & 0xff;

[Severity: High]
This isn't a bug introduced by this patch, but does 
drm_panic_write_pixel24_xpage() lack a NULL check for vaddr2?

If the next page is located in HighMem, kmap_local_page_try_from_panic()
returns NULL since it is unsafe to map HighMem during a panic.

When vaddr2 is NULL and the offset crosses the page boundary
(PAGE_SIZE - 1 or PAGE_SIZE - 2), the write pointer p is set to vaddr2.
Would the subsequent write operation then dereference NULL, causing a
recursive panic that aborts the DRM panic handler?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3