Re: [PATCH 1/2] drm: Fix memory leak at error path of drm_read()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 04, 2014 at 11:51:14AM +0000, Chris Wilson wrote:
> On Thu, Dec 04, 2014 at 11:56:42AM +0100, Takashi Iwai wrote:
> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> > ---
> >  drivers/gpu/drm/drm_fops.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> > index ed7bc68f7e87..a82dc28d54f3 100644
> > --- a/drivers/gpu/drm/drm_fops.c
> > +++ b/drivers/gpu/drm/drm_fops.c
> > @@ -525,6 +525,7 @@ ssize_t drm_read(struct file *filp, char __user *buffer,
> >  		if (copy_to_user(buffer + total,
> >  				 e->event, e->event->length)) {
> >  			total = -EFAULT;
> > +			e->destroy(e);
> 
> We shouldn't just be throwing away the event here, but put the event
> back at the front of the queue. Poses an interesting race issue. Seems
> like we want to hold the spinlock until the copy is complete so that we
> can fix up the failure correctly.

I've read the manpage for read and it explicitly states that when you get
an error it's undefined what happens to the read position. Since -EFAULT
is really just a userspace bug I think we can happily drop the event on
the floor, no reason to bend over in the kernel.

I'll add a note to the commit message.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux