Hi Dan,
On Fri, 20 Mar 2020 at 13:23, Dan Carpenter dan.carpenter@oracle.com wrote:
If the "handles" allocation or the copy_from_user() fails then we leak "objs". It's supposed to be freed in panfrost_job_cleanup().
Fixes: c117aa4d8701 ("drm: Add a drm_gem_objects_lookup helper") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com
drivers/gpu/drm/drm_gem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index a9e4a610445a..f28724f2eb69 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -710,6 +710,8 @@ int drm_gem_objects_lookup(struct drm_file *filp, void __user *bo_handles, if (!objs) return -ENOMEM;
*objs_out = objs;handles = kvmalloc_array(count, sizeof(u32), GFP_KERNEL); if (!handles) { ret = -ENOMEM;@@ -723,8 +725,6 @@ int drm_gem_objects_lookup(struct drm_file *filp, void __user *bo_handles, }
ret = objects_lookup(filp, handles, count, objs);
*objs_out = objs;out: kvfree(handles); return ret;
It seems that this will return error to the caller, mangle the output pointer and effectively still leak the objs.
Better option IMHO is to: - move the __user/copy_from_user into the caller Removes a silly kvmalloc_array(1,...) in ~90+ users and drops the "out" label. Extra bonus, this is the only instance in drm_gem with __user - consistency is nice. - add "err" or similar label, where the objs is freed before returning an error.
-Emil