| Message ID | 20171111205527.g5dach2rmhlxmr5x@gofer.mess.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sean, Thank you for the changes, It's better like this, I will test it. Don't you think that a much better way would be to make the kalloc directly inside dvb_usb_generic_rw instead of changing each call of it? Are you sure there are no other mistake somewhere else ? Laurent 2017-11-11 21:55 GMT+01:00 Sean Young <sean@mess.org>: > Hi Laurant, > > On Sat, Nov 11, 2017 at 08:06:38PM +0100, Laurent Caumont wrote: >> Hi Sean, >> >> I hope this one will be okay. > > There is a memory leak in there, and there is no reason to have to kmallocs > for this function. Please would you mind testing this version? > > Please note that there were other issues like whitespace which I've fixed > up. > > Thanks, > Sean > ---- > From 8362bc3e95016944b173c3866c103fcbc2587b6d Mon Sep 17 00:00:00 2001 > From: Laurent Caumont <lcaumont2@gmail.com> > Date: Sat, 11 Nov 2017 18:44:46 +0100 > Subject: [PATCH] media: dvb: i2c transfers over usb cannot be done from stack > > Cc: stable@vger.kernel.org > Signed-off-by: Laurent Caumont <lcaumont2@gmail.com> > Signed-off-by: Sean Young <sean@mess.org> > --- > drivers/media/usb/dvb-usb/dibusb-common.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/usb/dvb-usb/dibusb-common.c b/drivers/media/usb/dvb-usb/dibusb-common.c > index 8207e6900656..bcacb0f22028 100644 > --- a/drivers/media/usb/dvb-usb/dibusb-common.c > +++ b/drivers/media/usb/dvb-usb/dibusb-common.c > @@ -223,8 +223,20 @@ EXPORT_SYMBOL(dibusb_i2c_algo); > > int dibusb_read_eeprom_byte(struct dvb_usb_device *d, u8 offs, u8 *val) > { > - u8 wbuf[1] = { offs }; > - return dibusb_i2c_msg(d, 0x50, wbuf, 1, val, 1); > + u8 *buf; > + int rc; > + > + buf = kmalloc(2, GFP_KERNEL); > + if (!buf) > + return -ENOMEM; > + > + buf[0] = offs; > + > + rc = dibusb_i2c_msg(d, 0x50, &buf[0], 1, &buf[1], 1); > + *val = buf[1]; > + kfree(buf); > + > + return rc; > } > EXPORT_SYMBOL(dibusb_read_eeprom_byte); > > -- > 2.13.6 >
On Sun, Nov 12, 2017 at 09:38:47AM +0100, Laurent Caumont wrote: > Hi Sean, > > Thank you for the changes, It's better like this, I will test it. Great, thanks. Please let us know if it works, then it can be included. > Don't you think that a much better way would be to make the kalloc > directly inside dvb_usb_generic_rw instead of changing each call of > it? Are you sure there are no other mistake somewhere else ? That would introduce an extra kmalloc and memcpy in that function, and for many call sites that would be unnecesary, so it would be inefficient. Sean
Hi Laurent,
On Sun, Nov 12, 2017 at 09:38:47AM +0100, Laurent Caumont wrote:
> Thank you for the changes, It's better like this, I will test it.
Just wondering if you have had the time to test this patch. It would be
great if it could be included after being tested.
Thanks,
Sean
Hi Sean, Yes, it is working well. Laurent 2017-11-21 22:03 UTC+01:00, Sean Young <sean@mess.org>: > Hi Laurent, > > On Sun, Nov 12, 2017 at 09:38:47AM +0100, Laurent Caumont wrote: >> Thank you for the changes, It's better like this, I will test it. > > Just wondering if you have had the time to test this patch. It would be > great if it could be included after being tested. > > Thanks, > > Sean >
diff --git a/drivers/media/usb/dvb-usb/dibusb-common.c b/drivers/media/usb/dvb-usb/dibusb-common.c index 8207e6900656..bcacb0f22028 100644 --- a/drivers/media/usb/dvb-usb/dibusb-common.c +++ b/drivers/media/usb/dvb-usb/dibusb-common.c @@ -223,8 +223,20 @@ EXPORT_SYMBOL(dibusb_i2c_algo); int dibusb_read_eeprom_byte(struct dvb_usb_device *d, u8 offs, u8 *val) { - u8 wbuf[1] = { offs }; - return dibusb_i2c_msg(d, 0x50, wbuf, 1, val, 1); + u8 *buf; + int rc; + + buf = kmalloc(2, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + buf[0] = offs; + + rc = dibusb_i2c_msg(d, 0x50, &buf[0], 1, &buf[1], 1); + *val = buf[1]; + kfree(buf); + + return rc; } EXPORT_SYMBOL(dibusb_read_eeprom_byte);