Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/gud: zero max_buffer_size in gud_get_max_buffer_size() for R1/XRGB1111 --
commit 9571a61953c7bcaa943faf581ba726cb4c44a2b4 Author: Ruben Wauters <[email protected]> drm/gud: move format init from gud_probe() This patch factors pixel format initialization and maximum buffer size calculation out of gud_probe() into helper functions gud_get_formats() and gud_get_max_buffer_size(). > diff --git a/drivers/gpu/drm/gud/gud_drv.c b/drivers/gpu/drm/gud/gud_drv.c > index 680447f426f37..36adf2e2b6a2e 100644 > --- a/drivers/gpu/drm/gud/gud_drv.c > +++ b/drivers/gpu/drm/gud/gud_drv.c > @@ -435,11 +435,115 @@ static void gud_free_buffers_and_mutex(void *data) > mutex_destroy(&gdrm->ctrl_lock); > } > > -static int gud_probe(struct usb_interface *intf, const struct usb_device_id > *id) > +static size_t gud_get_max_buffer_size(struct gud_device *gdrm, u32 *formats, > u32 num_formats) > +{ > + struct drm_device *drm = &gdrm->drm; > + size_t max_buffer_size = 0; > + size_t fmt_buf_size; > + int i; > + > + for (i = 0; i < num_formats; i++) { > + const struct drm_format_info *info = > drm_format_info(formats[i]); > + > + /* Skip over emulated XRGB8888 */ > + if (formats[i] == DRM_FORMAT_XRGB8888 && > gdrm->xrgb8888_emulation_format) > + continue; > + > + fmt_buf_size = drm_format_info_min_pitch(info, 0, > drm->mode_config.max_width) * > + > drm->mode_config.max_height; > + max_buffer_size = max(max_buffer_size, fmt_buf_size); > + } > + > + return max_buffer_size; > +} [Severity: High] Could skipping emulated DRM_FORMAT_XRGB8888 in gud_get_max_buffer_size() cause max_buffer_size to be calculated as 0 when a device only supports internal formats (GUD_DRM_FORMAT_R1 or GUD_DRM_FORMAT_XRGB1111)? In gud_get_formats(), GUD_DRM_FORMAT_R1 and GUD_DRM_FORMAT_XRGB1111 set xrgb8888_emulation_format to &gud_drm_format_r1 or &gud_drm_format_xrgb1111, and are then skipped before being added to formats[]: if (format == GUD_DRM_FORMAT_R1 || format == GUD_DRM_FORMAT_XRGB1111) continue; /* Internal not for userspace */ formats[num_formats++] = format; If the device only advertises GUD_PIXEL_FORMAT_R1 and/or GUD_PIXEL_FORMAT_XRGB1111, num_formats remains 0 after the loop and gud_get_formats() appends DRM_FORMAT_XRGB8888 as the sole entry in formats[]: if (!xrgb8888_supported && xrgb8888_emulation_format) { gdrm->xrgb8888_emulation_format = xrgb8888_emulation_format; formats[num_formats++] = DRM_FORMAT_XRGB8888; } Previously, gud_probe() computed fmt_buf_size inside the formats_dev loop before skipping R1 and XRGB1111, so their buffer sizes were accounted for. With this change, gud_get_max_buffer_size() skips the only entry in formats[] and returns 0. When the optional desc.max_buffer_size descriptor field is 0, gud_probe() sets gdrm->bulk_len = 0 and calls gud_alloc_bulk_buffer(): gud_probe() gud_alloc_bulk_buffer() vmalloc_32(gdrm->bulk_len) Passing 0 to vmalloc_32() triggers WARN_ON_ONCE(!size) and returns NULL, causing gud_probe() to fail with -ENOMEM. Should gud_get_max_buffer_size() substitute gdrm->xrgb8888_emulation_format for info when formats[i] == DRM_FORMAT_XRGB8888 and gdrm->xrgb8888_emulation_format is set, instead of skipping it? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
