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 <[email protected]> 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/[email protected]?part=3
