| Message ID | 1584329768-16119-1-git-send-email-hqjagain@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/lease: fix WARNING in idr_destroy | expand |
On Mon, Mar 16, 2020 at 11:36:08AM +0800, Qiujun Huang wrote: > leases has been destroyed: > drm_master_put > ->drm_master_destroy > ->idr_destroy > > so the "out_lessee" needn't to call idr_destroy again. > > Reported-and-tested-by: syzbot+05835159fe322770fe3d@syzkaller.appspotmail.com > Signed-off-by: Qiujun Huang <hqjagain@gmail.com> > --- > drivers/gpu/drm/drm_lease.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c > index b481caf..54506c2 100644 > --- a/drivers/gpu/drm/drm_lease.c > +++ b/drivers/gpu/drm/drm_lease.c > @@ -577,11 +577,14 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev, > > out_lessee: > drm_master_put(&lessee); > + goto err_exit; I think this is correct, but also confusioning and inconsistent with the existing style. Most error cases before drm_lease_create explicit destroy the idr, except the one for drm_lease_create. Instead of your patch I'd - remove the idr_destry from the error unrolling here - add it the to drm_lease_create error case - add a comment above the call to drm_lease_crete that it takes ownership of the lease idr Can you pls respin and retest to make sure that patch is still correct? Thanks, Daniel > > out_leases: > - put_unused_fd(fd); > idr_destroy(&leases); > > +err_exit: > + put_unused_fd(fd); > + > DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl failed: %d\n", ret); > return ret; > } > -- > 1.8.3.1 >
On Wed, Mar 18, 2020 at 12:57 AM Daniel Vetter <daniel@ffwll.ch> wrote: > > On Mon, Mar 16, 2020 at 11:36:08AM +0800, Qiujun Huang wrote: > > leases has been destroyed: > > drm_master_put > > ->drm_master_destroy > > ->idr_destroy > > > > so the "out_lessee" needn't to call idr_destroy again. > > > > Reported-and-tested-by: syzbot+05835159fe322770fe3d@syzkaller.appspotmail.com > > Signed-off-by: Qiujun Huang <hqjagain@gmail.com> > > --- > > drivers/gpu/drm/drm_lease.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c > > index b481caf..54506c2 100644 > > --- a/drivers/gpu/drm/drm_lease.c > > +++ b/drivers/gpu/drm/drm_lease.c > > @@ -577,11 +577,14 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev, > > > > out_lessee: > > drm_master_put(&lessee); > > + goto err_exit; > > I think this is correct, but also confusioning and inconsistent with the > existing style. Most error cases before drm_lease_create explicit destroy > the idr, except the one for drm_lease_create. Yeah, it is. > > Instead of your patch I'd > - remove the idr_destry from the error unrolling here > - add it the to drm_lease_create error case > - add a comment above the call to drm_lease_crete that it takes ownership > of the lease idr > > Can you pls respin and retest to make sure that patch is still correct? I get that, thanks. > > Thanks, Daniel > > > > > out_leases: > > - put_unused_fd(fd); > > idr_destroy(&leases); > > > > +err_exit: > > + put_unused_fd(fd); > > + > > DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl failed: %d\n", ret); > > return ret; > > } > > -- > > 1.8.3.1 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c index b481caf..54506c2 100644 --- a/drivers/gpu/drm/drm_lease.c +++ b/drivers/gpu/drm/drm_lease.c @@ -577,11 +577,14 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev, out_lessee: drm_master_put(&lessee); + goto err_exit; out_leases: - put_unused_fd(fd); idr_destroy(&leases); +err_exit: + put_unused_fd(fd); + DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl failed: %d\n", ret); return ret; }
leases has been destroyed: drm_master_put ->drm_master_destroy ->idr_destroy so the "out_lessee" needn't to call idr_destroy again. Reported-and-tested-by: syzbot+05835159fe322770fe3d@syzkaller.appspotmail.com Signed-off-by: Qiujun Huang <hqjagain@gmail.com> --- drivers/gpu/drm/drm_lease.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)