Re: [PATCH v2] drm/panic: allow overriding panic QR code URL

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/panic: OOB read due to missing null-termination on panic_qr_url
--

commit 4020ffafa052ada60c700fa2d64b78e9eb01e839
Author: 纳西妲 · Nahida <me@nahida.im>

drm/panic: allow overriding panic QR code URL

This patch adds a module parameter to override the DRM panic QR code base URL
at boot or module load time. It exposes a getter function for the DRM panic
helper to retrieve and format the configured URL when generating the QR code.

> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
> index 9fe97c4eda6c8..463544f71c537 100644
> --- a/drivers/gpu/drm/drm_panic.c
> +++ b/drivers/gpu/drm/drm_panic.c
[ ... ]
> @@ -49,6 +49,15 @@ MODULE_LICENSE("GPL");
>  static uint panic_qr_version = CONFIG_DRM_PANIC_SCREEN_QR_VERSION;
>  module_param(panic_qr_version, uint, 0644);
>  MODULE_PARM_DESC(panic_qr_version, "maximum version (size) of the QR code");
> +static char panic_qr_url[256] = CONFIG_DRM_PANIC_SCREEN_QR_CODE_URL;

[Severity: Medium]
Will this array be null-terminated if CONFIG_DRM_PANIC_SCREEN_QR_CODE_URL is
exactly 256 bytes or longer?

In C, initializing a fixed-size character array with a string literal of equal
or greater length does not append a null terminator.

[ ... ]
> diff --git a/drivers/gpu/drm/drm_panic_helper.c b/drivers/gpu/drm/drm_panic_helper.c
> index 8913c3d22e8d3..b0ea0b4e8edef 100644
> --- a/drivers/gpu/drm/drm_panic_helper.c
> +++ b/drivers/gpu/drm/drm_panic_helper.c
[ ... ]
> @@ -609,17 +609,20 @@ static void drm_panic_helper_qr_exit(void)
>  	stream.workspace = NULL;
>  }
>  
> -static int drm_panic_helper_get_qr_code_url(u8 **qr_image, unsigned int qr_version)
> +static int drm_panic_helper_get_qr_code_url(u8 **qr_image, unsigned int qr_version,
> +					    const char *qr_url)
>  {
>  	struct kmsg_dump_iter iter;
> -	char url[256];
> +	char url[512];
>  	size_t kmsg_len, max_kmsg_size;
>  	char *kmsg;
>  	int max_qr_data_size, url_len;
>  
>  	url_len = snprintf(url, sizeof(url), "%s?a=%s&v=%s&z=",
> -			   CONFIG_DRM_PANIC_SCREEN_QR_CODE_URL,
> -			   utsname()->machine, utsname()->release);
> +			   qr_url, utsname()->machine, utsname()->release);

[Severity: Medium]
Could this lead to an out-of-bounds memory read during a kernel panic?

If qr_url points to a panic_qr_url array that lacks a null terminator,
snprintf() will read past the end of the array until it encounters a null
byte in adjacent memory. This could potentially leak adjacent memory into the
QR code URL or cause a secondary crash that prevents the panic screen from
displaying.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002222021.149450-1-me@nahida.im?part=1