Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] This problem wasn't introduced by this patch, but using the `CONFIG_DRM_PANIC_SCREEN_QR_CODE_URL` Kconfig macro directly as part of the format string in `snprintf` creates a format string vulnerability. If the configured URL contains URL-encoded characters that form valid format specifiers (e.g., `?reason=kernel%20panic`, where `%20p` is a format specifier for a padded pointer), `snprintf` will consume unintended arguments. This shifts the argument mapping for the explicit `%s` specifiers, causing them to read and dereference uninitialized stack memory, leading to a nested kernel oops during a panic. -- --- Patch [1]: [PATCH 01/12] drm/panic: Allocate QR-code buffers statically --- commit 8f4057e44664878e84dd3de301552d418b19254c Author: Thomas Zimmermann <[email protected]> drm/panic: Allocate QR-code buffers statically Declare qrbuf1 and qrbuf2 as static arrays so that the module loader allocates them for us. Avoids the kmalloc later on. Access the arrays once with memset, so that the physical pages are available on a panic. > diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c > --- a/drivers/gpu/drm/drm_panic.c > +++ b/drivers/gpu/drm/drm_panic.c [ ... ] > @@ -656,13 +655,17 @@ static int drm_panic_get_qr_code_url(u8 **qr_image) > char url[256]; > size_t kmsg_len, max_kmsg_size; > char *kmsg; > - int max_qr_data_size, url_len; > + int ret; > + size_t max_qr_data_size, url_len; > > - url_len = snprintf(url, sizeof(url), > CONFIG_DRM_PANIC_SCREEN_QR_CODE_URL "?a=%s&v=%s&z=", > - utsname()->machine, utsname()->release); > + ret = snprintf(url, sizeof(url), CONFIG_DRM_PANIC_SCREEN_QR_CODE_URL > "?a=%s&v=%s&z=", > + utsname()->machine, utsname()->release); [Severity: High] This isn't a bug introduced by this patch, but does constructing the format string directly from CONFIG_DRM_PANIC_SCREEN_QR_CODE_URL create a format string vulnerability in drm_panic_get_qr_code_url()? If a system builder configures the url string with standard url-encoded characters that happen to match valid format specifiers (such as %20p or %20d), snprintf() will try to consume unintended arguments. Could this shift the argument mapping for the explicit %s specifiers, causing them to dereference uninitialized stack memory and trigger a nested oops that prevents the panic screen from displaying? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
