Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Missing IS_ERR check on idr_replace() return value causes an invalid pointer dereference. --
commit f7209a31976a5023734386f397179e3835aa2048 Author: Simona Vetter <[email protected]> drm/gem: Try to fix change_handle ioctl, attempt 4 This commit modifies the change_handle ioctl to resolve a race condition by merging the GEM object lookup and two-stage idr_replace. It also temporarily disables the ioctl in drm_ioctls to prevent further abuse until the implementation is fully verified. > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index ac751a42db281..66cfd52c7cff9 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c [ ... ] > @@ -1028,52 +1028,36 @@ int drm_gem_change_handle_ioctl(struct drm_device > *dev, void *data, [ ... ] > + obj = idr_replace(&file_priv->object_idr, NULL, args->handle); > + if (!obj) { [Severity: High] Will this result in dereferencing an error pointer if args->handle is invalid? If idr_replace() fails to find the handle, it returns ERR_PTR(-ENOENT) rather than NULL. The check here only tests for NULL, allowing an error pointer to fall through. > + idr_remove(&file_priv->object_idr, new_handle); > + spin_unlock(&file_priv->table_lock); > + ret = -ENOENT; > + goto out_unlock; > + } > spin_unlock(&file_priv->table_lock); > > if (obj->dma_buf) { If an error pointer falls through the check above, wouldn't evaluating obj->dma_buf here cause an invalid pointer dereference? Should the error handling for idr_replace() be updated to use IS_ERR(obj)? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
