Re: [PATCH] drm/i915/fbdev: Preserve visible size when reusing BIOS fb

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] drm/i915/fbdev: framebuffer leak on error in
  intel_fbdev_driver_fbdev_probe() [1]
--

commit 89c135a5c0c574b132cf58f5439280c84473a688
Author: Zheyu Shen <arsdragonfly@gmail.com>

drm/i915/fbdev: Preserve visible size when reusing BIOS fb

This commit stops intel_fbdev_driver_fbdev_probe() from overwriting the
requested fb_width and fb_height with the BIOS framebuffer's dimensions
when reusing a preallocated BIOS framebuffer.

> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index 219ee17de3f68..5c043172a35c7 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -324,8 +324,6 @@ int intel_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
>  	} else {
>  		drm_dbg_kms(display->drm, "re-using BIOS fb\n");
>  		prealloc = true;
> -		sizes->fb_width = fb->base.width;
> -		sizes->fb_height = fb->base.height;
>  	}
>  
>  	obj = intel_fb_bo(&fb->base);

[Severity: Medium]
This problem wasn't introduced by this patch, but does
intel_fbdev_driver_fbdev_probe() leak &fb->base [1] if pinning or
filling the fbdev info fails right after this block?

drivers/gpu/drm/i915/display/intel_fbdev.c:intel_fbdev_driver_fbdev_probe() {
	...
	ifbdev->fb = NULL;
	...
	ret = intel_parent_fb_pin_ggtt_pin(display, obj, &pin_params,
					   &vma, &offset, NULL);
	if (ret)
		goto out_unlock;

	helper->funcs = &intel_fb_helper_funcs;
	helper->fb = &fb->base;

	info->fbops = &intelfb_ops;

	ret = intel_bo_fbdev_fill_info(obj, info, vma);
	if (ret)
		goto out_unpin;
	...
out_unpin:
	intel_parent_fb_pin_ggtt_unpin(display, vma, -1);
out_unlock:
	intel_display_rpm_put(display, wakeref);

	return ret;
}

ifbdev->fb is cleared at the start of the function, so the local fb holds
the only reference to the reused BIOS framebuffer (or the newly allocated
one).  If intel_parent_fb_pin_ggtt_pin() or intel_bo_fbdev_fill_info()
fails, drm_fb_helper_single_fb_probe() aborts before
register_framebuffer(), so intelfb_ops.fb_destroy is never called and the
drm_framebuffer and its backing GEM object are leaked, triggering a
WARN_ON(!list_empty(&dev->mode_config.fb_list)) in
drm_mode_config_cleanup() on driver teardown.

[1] https://sashiko.dev/#/bug/linux-c63f5649-e8ad-40e0-abcd-b5a3b7668026

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/e1823bda-ff83-4e9e-8e81-b2d0ea5c05ac@gmail.com?part=1