Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] drm/client: splash: UAF of drm_framebuffer on hotplug error path - [Medium] drm/client: splash: memory leak of scanout array on hotplug -ENODEV - [Medium] drm/client: splash: firmware blob leak if render thread exits with -EBUSY --
commit 86a0b0be0de60a4b9d33ab49ce7a129fe48448a6 Author: Francesco Valla <[email protected]> Date: Thu Oct 1 19:58:37 2026 +0000 drm: client: add splash client Add a DRM client that draws a simple splash, with possibility to show: - the logo provided by EFI BGRT; - a static BMP image (loaded as firmware); - a colored background. > diff --git a/drivers/gpu/drm/clients/drm_splash.c > b/drivers/gpu/drm/clients/drm_splash.c > new file mode 100644 > index 0000000000000..66c192139d9c4 > --- /dev/null > +++ b/drivers/gpu/drm/clients/drm_splash.c > @@ -0,0 +1,1028 @@ [ ... ] > +static void drm_splash_fw_callback(const struct firmware *fw, void *context) > +{ > + struct drm_splash *splash = context; > + struct drm_client_dev *client = &splash->client; > + > + if (!fw || !fw->data) { > + drm_err(client->dev, "splash: no firmware"); > + release_firmware(fw); > + fw = NULL; > + } > + > + scoped_guard(spinlock, &splash->fw_lock) > + splash->fw = fw; [ ... ] > +static int drm_splash_render_thread(void *data) > +{ > + struct drm_splash *splash = data; > + struct drm_client_dev *client = &splash->client; > + > + while (!kthread_should_stop()) { [ ... ] > + if (draw_count > 0) { > + ret = drm_client_modeset_commit(client); > + /* If commit returns EBUSY, another master showed up. > + * This means that the splash is no more required. > + */ > + if (ret == -EBUSY) { > + drm_info(client->dev, "splash: not master > anymore, exiting"); > + break; > + } [Severity: Medium] Can the firmware blob be leaked if the render thread exits early here? If drm_client_modeset_commit() returns -EBUSY, the thread breaks out of the loop and exits. If the firmware request later completes, drm_splash_fw_callback() stores the firmware in splash->fw. Since the thread is already dead, won't this firmware remain unreleased until the DRM device is completely unregistered? > + } [ ... ] > +static int drm_splash_init_client(struct drm_splash *splash) > +{ [ ... ] > + /* And point every modeset at the buffer of its scanout */ > + i = 0; > + drm_client_for_each_modeset(modeset, client) { > + unsigned int idx = i++; > + > + if (idx >= 32) > + break; > + > + for (k = 0; k < splash->n_scanout; k++) { > + struct drm_splash_scanout *scanout = > &splash->scanout[k]; > + > + if (scanout->buffer && (scanout->modesets & BIT(idx))) { > + modeset->fb = scanout->buffer->fb; > + break; > + } > + } > + } > + > + mutex_unlock(&client->modeset_mutex); > + > + return (fb_count == 0) ? -ENODEV : 0; > +} [ ... ] > +static int drm_splash_client_hotplug(struct drm_client_dev *client) > +{ > + struct drm_splash *splash = client_to_drm_splash(client); > + struct task_struct *thread; > + int ret = 0; > + > + guard(mutex)(&splash->hotplug_lock); > + > + /* The modesets that get a splash are defined at first hotplug event */ > + if (splash->initialized) > + return 0; > + > + ret = drm_splash_init_client(splash); > + if (ret == -ENODEV) { > + drm_info(client->dev, "splash: no modeset found"); > + return 0; > + } else if (ret) { [Severity: Medium] Could this error path leak the splash->scanout array? If drm_splash_init_client() successfully extends the array using krealloc() but fails to create any buffers, it returns -ENODEV. This early return handles the -ENODEV case but does not free the array. Could subsequent hotplug events keep appending to it and leaking memory on every failure? > + drm_err(client->dev, > + "splash: failed to init client: %d", ret); > + return ret; > + } > + > + /* Create the render thread, waken later */ > + thread = kthread_create(drm_splash_render_thread, splash, > + "drm_splash_%s", client->dev->unique); > + if (IS_ERR(thread)) { > + ret = PTR_ERR(thread); > + drm_err(client->dev, > + "splash: failed to create render thread: %d", ret); > + drm_splash_free_scanout(client); > + return ret; > + } [Severity: High] Does this error path leave dangling pointers in the client's modeset array? In drm_splash_init_client(), modeset->fb is assigned the framebuffer of the newly created buffer. If we fail here and call drm_splash_free_scanout(), the buffers and their framebuffers are freed, but modeset->fb is not cleared. Since the client remains registered on dev->clientlist even after a hotplug error, could a subsequent modeset commit or sysrq trigger a use-after-free by dereferencing the freed modeset->fb? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
