Re: [PATCH v2 3/3] drm/armada: Make armada_framebuffer_create() an internal interface

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Reference leak of GEM object on error path in `armada_fb_create`
--

commit 6143d70290da395a0c8c1c0addd0695192664072
Author: Thomas Zimmermann <tzimmermann@suse.de>

drm/armada: Make armada_framebuffer_create() an internal interface

The only caller of armada_framebuffer_create() is armada_fb_create()
from the same source file. Declare the former as static.

> diff --git a/drivers/gpu/drm/armada/armada_fb.c b/drivers/gpu/drm/armada/armada_fb.c
> index b828bba419bfb..2f2348d543fd7 100644
> --- a/drivers/gpu/drm/armada/armada_fb.c
> +++ b/drivers/gpu/drm/armada/armada_fb.c
> @@ -18,10 +18,11 @@ static const struct drm_framebuffer_funcs armada_fb_funcs = {
>  	.create_handle	= drm_gem_fb_create_handle,
>  };
>  
> -struct armada_framebuffer *armada_framebuffer_create(struct drm_device *dev,
> -						     const struct drm_format_info *info,
> -						     const struct drm_mode_fb_cmd2 *mode,
> -						     struct armada_gem_object *obj)
> +static struct armada_framebuffer *
> +armada_framebuffer_create(struct drm_device *dev,
> +			  const struct drm_format_info *info,
> +			  const struct drm_mode_fb_cmd2 *mode,
> +			  struct armada_gem_object *obj)
>  {

[Severity: High]
This is a pre-existing issue, but while reviewing the caller of this newly
static function, it appears there might be a resource leak on the error path.

In armada_fb_create(), we get a reference to the GEM object:

	obj = armada_gem_object_lookup(dfile, mode->handles[0]);

But if the call to armada_framebuffer_create() fails, the code jumps to the
'err' label instead of 'err_unref':

	dfb = armada_framebuffer_create(dev, info, mode, obj);
	if (IS_ERR(dfb)) {
		ret = PTR_ERR(dfb);
		goto err;
	}

	drm_gem_object_put(&obj->obj);

	return &dfb->fb;

 err_unref:
	drm_gem_object_put(&obj->obj);
 err:
	DRM_ERROR("failed to initialize framebuffer: %d\n", ret);

Does this bypass the drm_gem_object_put() call and leak the GEM object
reference? Since armada_fb_create() is reachable from userspace via the
DRM_IOCTL_MODE_ADDFB2 ioctl, could a user repeatedly request invalid formats
to leak references and cause memory exhaustion?

>  	struct armada_framebuffer *dfb;
>  	uint8_t format, config;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611072121.71492-1-tzimmermann@suse.de?part=3