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

Pre-existing issues:
- [High] Potential NULL pointer dereference in 
`kmap_local_page_try_from_panic()` due to missing NULL checks for page pointers 
in `drm_panic_fill_page()` and `drm_panic_blit_page()`.
--

commit ddaf01e3dc4138e21af0a9818e11a3a8b5244eae
Author: Thomas Zimmermann <[email protected]>

drm/panic: Test address from kmap op for NULL

This patch adds a NULL check for the return value of
kmap_local_page_try_from_panic() to avoid dereferencing a failed mapping
during kernel panic screen rendering.

> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
> index ba21df4be3380..d28ce8878e055 100644
> --- a/drivers/gpu/drm/drm_panic.c
> +++ b/drivers/gpu/drm/drm_panic.c
> @@ -178,6 +178,8 @@ static void drm_panic_write_pixel24_xpage(void *vaddr, 
> struct page *next_page,
>       u8 *p = vaddr + offset;
>  
>       vaddr2 = kmap_local_page_try_from_panic(next_page);

[Severity: High]
This isn't a bug introduced by this patch, but could next_page itself 
be NULL here?

If a 24-bit pixel crosses a page boundary and the next page in the pages 
array is NULL, passing it into kmap_local_page_try_from_panic() can cause 
a NULL pointer dereference via PageHighMem(). If CONFIG_HIGHMEM is disabled, 
page_to_virt(NULL) yields a garbage pointer which also crashes upon 
dereference. This would cause a double fault during a kernel panic, 
preventing the panic screen from rendering.

It looks like similar unchecked page pointers are passed elsewhere in 
drivers/gpu/drm/drm_panic.c, such as in drm_panic_fill_page():

drm_panic_fill_page() {
    ...
    vaddr = kmap_local_page_try_from_panic(pages[page]);
    ...
}

And in drm_panic_blit_page(), pages[page + 1] is passed without a NULL check 
even though pages[new_page] is explicitly checked earlier in the function:

drm_panic_blit_page() {
    ...
    drm_panic_write_pixel24_xpage(vaddr, pages[page + 1], offset, fg32);
    ...
}

Would it be safer to verify that next_page and pages[page] are not NULL 
before attempting to map them, in order to safely handle unmapped or sparse 
pages in the display buffer?

> +     if (!vaddr2)
> +             return;
>  
>       *p++ = color & 0xff;
>       color >>= 8;

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=2

Reply via email to