Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/msm: uninitialized return value in msm_ioctl_gem_info_get_metadata() --
commit 26d3997a404e96c83bc421df7eb395c717c9751f Author: Rob Clark <[email protected]> drm/msm/gem: Add some cleanup.h This commit converts msm_gem_lock() and related locking helpers to use scope-based cleanup from cleanup.h, while also fixing a lock leak in msm_ioctl_gem_info_set_metadata(). > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c > index 5e8f3ce26b32c..42381db0c4686 100644 > --- a/drivers/gpu/drm/msm/msm_drv.c > +++ b/drivers/gpu/drm/msm/msm_drv.c [ ... ] > @@ -607,21 +600,14 @@ static int msm_ioctl_gem_info_get_metadata(struct > drm_gem_object *obj, > return 0; > } > > - ret = msm_gem_lock_interruptible(obj); > - if (ret) > - return ret; > - > - /* Avoid copy_to_user() under gem obj lock: */ > - len = msm_obj->metadata_size; > - buf = kmemdup(msm_obj->metadata, len, GFP_KERNEL); > - > - if (!buf) { > - msm_gem_unlock(obj); > - return -ENOMEM; > + scoped_cond_guard (gem_lock_intr, return -EINTR, obj) { > + /* Avoid copy_to_user() under gem obj lock by making a copy: */ > + len = msm_obj->metadata_size; > + buf = kmemdup(msm_obj->metadata, len, GFP_KERNEL); > + if (!buf) > + return -ENOMEM; > } > > - msm_gem_unlock(obj); > - > if (*metadata_size < len) { > ret = -ETOOSMALL; > } else if (copy_to_user(metadata, buf, len)) { [Severity: High] Does msm_ioctl_gem_info_get_metadata() return an uninitialized ret on the success path? Previously, ret was initialized to 0 by: ret = msm_gem_lock_interruptible(obj); With that replaced by scoped_cond_guard, when *metadata_size >= len and copy_to_user() succeeds, execution takes the final else branch without assigning ret: drivers/gpu/drm/msm/msm_drv.c:msm_ioctl_gem_info_get_metadata() { ... int ret, len; ... if (*metadata_size < len) { ret = -ETOOSMALL; } else if (copy_to_user(metadata, buf, len)) { ret = -EFAULT; } else { *metadata_size = len; } kfree(buf); return ret; } Should ret be initialized to 0 (or set to 0 in the else branch)? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
