[PATCH 0/5] get rid of on-stack dma buffers (part1)

Hi Mauro!

These are the patches which got tested already and 
should be good to go. [first batch of patches]

I have another batch with updated patches (dib0700, gp8psk, vp702x)
where I did some more extensive changes to use preallocated memory.
And a small update to the vp7045 patch.

Third batch are the patches to opera1, m920x, dw2102, friio,
a800 which I left as is, for the time beeing. 
Regards,
Flo

Florian Mickler (5):
  [media] ec168: get rid of on-stack dma buffers
  [media] ce6230: get rid of on-stack dma buffer
  [media] au6610: get rid of on-stack dma buffer
  [media] lmedm04: correct indentation
  [media] lmedm04: get rid of on-stack dma buffers

 drivers/media/dvb/dvb-usb/au6610.c  |   22 ++++++++++++++++------
 drivers/media/dvb/dvb-usb/ce6230.c  |   11 +++++++++--
 drivers/media/dvb/dvb-usb/ec168.c   |   18 +++++++++++++++---
 drivers/media/dvb/dvb-usb/lmedm04.c |   35 +++++++++++++++++++++++------------
 4 files changed, 63 insertions(+), 23 deletions(-)

-- 
1.7.4.1

This should not change anything except whitespace.

Signed-off-by: Florian Mickler <redacted>
---
 drivers/media/dvb/dvb-usb/lmedm04.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/dvb/dvb-usb/lmedm04.c b/drivers/media/dvb/dvb-usb/lmedm04.c
index 9eea418..0a3e88f 100644
--- a/drivers/media/dvb/dvb-usb/lmedm04.c
+++ b/drivers/media/dvb/dvb-usb/lmedm04.c

@@ -626,15 +626,15 @@ static int lme2510_download_firmware(struct usb_device *dev,
 				data[0] = i | 0x80;
 				dlen = (u8)(end - j)-1;
 			}
-		data[1] = dlen;
-		memcpy(&data[2], fw_data, dlen+1);
-		wlen = (u8) dlen + 4;
-		data[wlen-1] = check_sum(fw_data, dlen+1);
-		deb_info(1, "Data S=%02x:E=%02x CS= %02x", data[3],
+			data[1] = dlen;
+			memcpy(&data[2], fw_data, dlen+1);
+			wlen = (u8) dlen + 4;
+			data[wlen-1] = check_sum(fw_data, dlen+1);
+			deb_info(1, "Data S=%02x:E=%02x CS= %02x", data[3],
 				data[dlen+2], data[dlen+3]);
-		ret |= lme2510_bulk_write(dev, data,  wlen, 1);
-		ret |= lme2510_bulk_read(dev, data, len_in , 1);
-		ret |= (data[0] == 0x88) ? 0 : -1;
+			ret |= lme2510_bulk_write(dev, data,  wlen, 1);
+			ret |= lme2510_bulk_read(dev, data, len_in , 1);
+			ret |= (data[0] == 0x88) ? 0 : -1;
 		}
 	}
 

-- 
1.7.4.1

usb_control_msg initiates (and waits for completion of) a dma transfer using
the supplied buffer. That buffer thus has to be seperately allocated on
the heap.

In lib/dma_debug.c the function check_for_stack even warns about it:
	WARNING: at lib/dma-debug.c:866 check_for_stack

Signed-off-by: Florian Mickler <redacted>
Acked-by: Antti Palosaari <redacted>
Reviewed-by: Antti Palosaari <redacted>
Tested-by: Antti Palosaari <redacted>
---
 drivers/media/dvb/dvb-usb/ce6230.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb/dvb-usb/ce6230.c b/drivers/media/dvb/dvb-usb/ce6230.c
index 3df2045..6d1a304 100644
--- a/drivers/media/dvb/dvb-usb/ce6230.c
+++ b/drivers/media/dvb/dvb-usb/ce6230.c

@@ -39,7 +39,7 @@ static int ce6230_rw_udev(struct usb_device *udev, struct req_t *req)
 	u8 requesttype;
 	u16 value;
 	u16 index;
-	u8 buf[req->data_len];
+	u8 *buf;
 
 	request = req->cmd;
 	value = req->value;

@@ -62,6 +62,12 @@ static int ce6230_rw_udev(struct usb_device *udev, struct req_t *req)
 		goto error;
 	}
 
+	buf = kmalloc(req->data_len, GFP_KERNEL);
+	if (!buf) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
 	if (requesttype == (USB_TYPE_VENDOR | USB_DIR_OUT)) {
 		/* write */
 		memcpy(buf, req->data, req->data_len);

@@ -74,7 +80,7 @@ static int ce6230_rw_udev(struct usb_device *udev, struct req_t *req)
 	msleep(1); /* avoid I2C errors */
 
 	ret = usb_control_msg(udev, pipe, request, requesttype, value, index,
-				buf, sizeof(buf), CE6230_USB_TIMEOUT);
+				buf, req->data_len, CE6230_USB_TIMEOUT);
 
 	ce6230_debug_dump(request, requesttype, value, index, buf,
 		req->data_len, deb_xfer);

@@ -88,6 +94,7 @@ static int ce6230_rw_udev(struct usb_device *udev, struct req_t *req)
 	if (!ret && requesttype == (USB_TYPE_VENDOR | USB_DIR_IN))
 		memcpy(req->data, buf, req->data_len);
 
+	kfree(buf);
 error:
 	return ret;
 }

-- 
1.7.4.1

usb_control_msg initiates (and waits for completion of) a dma transfer using
the supplied buffer. That buffer thus has to be seperately allocated on
the heap.

In lib/dma_debug.c the function check_for_stack even warns about it:
	WARNING: at lib/dma-debug.c:866 check_for_stack

Signed-off-by: Florian Mickler <redacted>
Acked-by: Antti Palosaari <redacted>
Reviewed-by: Antti Palosaari <redacted>
Tested-by: Antti Palosaari <redacted>
---
 drivers/media/dvb/dvb-usb/ec168.c |   18 +++++++++++++++---
 1 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb/dvb-usb/ec168.c b/drivers/media/dvb/dvb-usb/ec168.c
index 52f5d4f..1ba3e5d 100644
--- a/drivers/media/dvb/dvb-usb/ec168.c
+++ b/drivers/media/dvb/dvb-usb/ec168.c

@@ -36,7 +36,9 @@ static int ec168_rw_udev(struct usb_device *udev, struct ec168_req *req)
 	int ret;
 	unsigned int pipe;
 	u8 request, requesttype;
-	u8 buf[req->size];
+	u8 *buf;
+
+
 
 	switch (req->cmd) {
 	case DOWNLOAD_FIRMWARE:

@@ -72,6 +74,12 @@ static int ec168_rw_udev(struct usb_device *udev, struct ec168_req *req)
 		goto error;
 	}
 
+	buf = kmalloc(req->size, GFP_KERNEL);
+	if (!buf) {
+		ret = -ENOMEM;
+		goto error;
+	}
+
 	if (requesttype == (USB_TYPE_VENDOR | USB_DIR_OUT)) {
 		/* write */
 		memcpy(buf, req->data, req->size);

@@ -84,13 +92,13 @@ static int ec168_rw_udev(struct usb_device *udev, struct ec168_req *req)
 	msleep(1); /* avoid I2C errors */
 
 	ret = usb_control_msg(udev, pipe, request, requesttype, req->value,
-		req->index, buf, sizeof(buf), EC168_USB_TIMEOUT);
+		req->index, buf, req->size, EC168_USB_TIMEOUT);
 
 	ec168_debug_dump(request, requesttype, req->value, req->index, buf,
 		req->size, deb_xfer);
 
 	if (ret < 0)
-		goto error;
+		goto err_dealloc;
 	else
 		ret = 0;
 

@@ -98,7 +106,11 @@ static int ec168_rw_udev(struct usb_device *udev, struct ec168_req *req)
 	if (!ret && requesttype == (USB_TYPE_VENDOR | USB_DIR_IN))
 		memcpy(req->data, buf, req->size);
 
+	kfree(buf);
 	return ret;
+
+err_dealloc:
+	kfree(buf);
 error:
 	deb_info("%s: failed:%d\n", __func__, ret);
 	return ret;

-- 
1.7.4.1

usb_control_msg initiates (and waits for completion of) a dma transfer using
the supplied buffer. That buffer thus has to be seperately allocated on
the heap.

In lib/dma_debug.c the function check_for_stack even warns about it:
	WARNING: at lib/dma-debug.c:866 check_for_stack

Tested-By: Malcolm Priestley <tvboxspy@gmail.com>
Signed-off-by: Florian Mickler <redacted>

---

[v2: fix use after free as noted by Malcom]

drivers/media/dvb/dvb-usb/lmedm04.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/media/dvb/dvb-usb/lmedm04.c b/drivers/media/dvb/dvb-usb/lmedm04.c
index 0a3e88f..8a79354 100644
--- a/drivers/media/dvb/dvb-usb/lmedm04.c
+++ b/drivers/media/dvb/dvb-usb/lmedm04.c

@@ -314,13 +314,19 @@ static int lme2510_int_read(struct dvb_usb_adapter *adap)
 static int lme2510_return_status(struct usb_device *dev)
 {
 	int ret = 0;
-	u8 data[10] = {0};
+	u8 *data;
+
+	data = kzalloc(10, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
 
 	ret |= usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
 			0x06, 0x80, 0x0302, 0x00, data, 0x0006, 200);
 	info("Firmware Status: %x (%x)", ret , data[2]);
 
-	return (ret < 0) ? -ENODEV : data[2];
+	ret = (ret < 0) ? -ENODEV : data[2];
+	kfree(data);
+	return ret;
 }
 
 static int lme2510_msg(struct dvb_usb_device *d,

@@ -603,7 +609,7 @@ static int lme2510_download_firmware(struct usb_device *dev,
 					const struct firmware *fw)
 {
 	int ret = 0;
-	u8 data[512] = {0};
+	u8 *data;
 	u16 j, wlen, len_in, start, end;
 	u8 packet_size, dlen, i;
 	u8 *fw_data;

@@ -611,6 +617,11 @@ static int lme2510_download_firmware(struct usb_device *dev,
 	packet_size = 0x31;
 	len_in = 1;
 
+	data = kzalloc(512, GFP_KERNEL);
+	if (!data) {
+		info("FRM Could not start Firmware Download (Buffer allocation failed)");
+		return -ENOMEM;
+	}
 
 	info("FRM Starting Firmware Download");
 

@@ -654,7 +665,7 @@ static int lme2510_download_firmware(struct usb_device *dev,
 	else
 		info("FRM Firmware Download Completed - Resetting Device");
 
-
+	kfree(data);
 	return (ret < 0) ? -ENODEV : 0;
 }
 

-- 
1.7.4.1

usb_control_msg initiates (and waits for completion of) a dma transfer using
the supplied buffer. That buffer thus has to be seperately allocated on
the heap.

In lib/dma_debug.c the function check_for_stack even warns about it:
	WARNING: at lib/dma-debug.c:866 check_for_stack

Signed-off-by: Florian Mickler <redacted>
Acked-by: Antti Palosaari <redacted>
Reviewed-by: Antti Palosaari <redacted>
Tested-by: Antti Palosaari <redacted>
---
 drivers/media/dvb/dvb-usb/au6610.c |   22 ++++++++++++++++------
 1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/media/dvb/dvb-usb/au6610.c b/drivers/media/dvb/dvb-usb/au6610.c
index eb34cc3..2351077 100644
--- a/drivers/media/dvb/dvb-usb/au6610.c
+++ b/drivers/media/dvb/dvb-usb/au6610.c

@@ -33,8 +33,16 @@ static int au6610_usb_msg(struct dvb_usb_device *d, u8 operation, u8 addr,
 {
 	int ret;
 	u16 index;
-	u8 usb_buf[6]; /* enough for all known requests,
-			  read returns 5 and write 6 bytes */
+	u8 *usb_buf;
+
+	/*
+	 * allocate enough for all known requests,
+	 * read returns 5 and write 6 bytes
+	 */
+	usb_buf = kmalloc(6, GFP_KERNEL);
+	if (!usb_buf)
+		return -ENOMEM;
+
 	switch (wlen) {
 	case 1:
 		index = wbuf[0] << 8;

@@ -45,14 +53,15 @@ static int au6610_usb_msg(struct dvb_usb_device *d, u8 operation, u8 addr,
 		break;
 	default:
 		warn("wlen = %x, aborting.", wlen);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto error;
 	}
 
 	ret = usb_control_msg(d->udev, usb_rcvctrlpipe(d->udev, 0), operation,
 			      USB_TYPE_VENDOR|USB_DIR_IN, addr << 1, index,
-			      usb_buf, sizeof(usb_buf), AU6610_USB_TIMEOUT);
+			      usb_buf, 6, AU6610_USB_TIMEOUT);
 	if (ret < 0)
-		return ret;
+		goto error;
 
 	switch (operation) {
 	case AU6610_REQ_I2C_READ:

@@ -60,7 +69,8 @@ static int au6610_usb_msg(struct dvb_usb_device *d, u8 operation, u8 addr,
 		/* requested value is always 5th byte in buffer */
 		rbuf[0] = usb_buf[4];
 	}
-
+error:
+	kfree(usb_buf);
 	return ret;
 }
 

-- 
1.7.4.1

Hi Mauro!

I just saw that you picked up some patches of mine. What about these?
These are actually tested...

Regards,
Flo

 On Sun, 20 Mar 2011 22:50:47 +0100
Florian Mickler [off-list ref] wrote:

Hi Mauro!

These are the patches which got tested already and 
should be good to go. [first batch of patches]

I have another batch with updated patches (dib0700, gp8psk, vp702x)
where I did some more extensive changes to use preallocated memory.
And a small update to the vp7045 patch.

Third batch are the patches to opera1, m920x, dw2102, friio,
a800 which I left as is, for the time beeing. 
Regards,
Flo

Florian Mickler (5):
  [media] ec168: get rid of on-stack dma buffers
  [media] ce6230: get rid of on-stack dma buffer
  [media] au6610: get rid of on-stack dma buffer
  [media] lmedm04: correct indentation
  [media] lmedm04: get rid of on-stack dma buffers

 drivers/media/dvb/dvb-usb/au6610.c  |   22 ++++++++++++++++------
 drivers/media/dvb/dvb-usb/ce6230.c  |   11 +++++++++--
 drivers/media/dvb/dvb-usb/ec168.c   |   18 +++++++++++++++---
 drivers/media/dvb/dvb-usb/lmedm04.c |   35 +++++++++++++++++++++++------------
 4 files changed, 63 insertions(+), 23 deletions(-)

Hi Florian,

Em 30-04-2011 15:54, Florian Mickler escreveu:

Hi Mauro!

I just saw that you picked up some patches of mine. What about these?
These are actually tested...

I'm still in process of applying the pending patches. Due to patchwork.kernel.org
troubles (including the loss of about 270 patches from its SQL database only 
recovered yesterday[1]), I have a long backlog. So, I'm gradually applying the remaing
stuff. It will take some time though, and it will depend on patchwork mood, but I intend
to spend some time during this weekend to minimize the backlog.


Cheers,
Mauro

[1] The recover lost the email's body/SOB, so I've wrote a script to use my email
queue to get the data, using patchwork just to mark what patches were already
processed. This increses the time I have to spend on each patch, as I need to run
a script to match the patchwork patch with the patch ID inside my email queue.

Regards,
Flo

 On Sun, 20 Mar 2011 22:50:47 +0100
Florian Mickler [off-list ref] wrote:

quoted

Hi Mauro!

These are the patches which got tested already and 
should be good to go. [first batch of patches]

I have another batch with updated patches (dib0700, gp8psk, vp702x)
where I did some more extensive changes to use preallocated memory.
And a small update to the vp7045 patch.

Third batch are the patches to opera1, m920x, dw2102, friio,
a800 which I left as is, for the time beeing. 
Regards,
Flo

Florian Mickler (5):
  [media] ec168: get rid of on-stack dma buffers
  [media] ce6230: get rid of on-stack dma buffer
  [media] au6610: get rid of on-stack dma buffer
  [media] lmedm04: correct indentation
  [media] lmedm04: get rid of on-stack dma buffers

 drivers/media/dvb/dvb-usb/au6610.c  |   22 ++++++++++++++++------
 drivers/media/dvb/dvb-usb/ce6230.c  |   11 +++++++++--
 drivers/media/dvb/dvb-usb/ec168.c   |   18 +++++++++++++++---
 drivers/media/dvb/dvb-usb/lmedm04.c |   35 +++++++++++++++++++++++------------
 4 files changed, 63 insertions(+), 23 deletions(-)

On Sat, 30 Apr 2011 19:30:52 -0300
Mauro Carvalho Chehab [off-list ref] wrote:

Hi Florian,

Em 30-04-2011 15:54, Florian Mickler escreveu:

quoted

Hi Mauro!

I just saw that you picked up some patches of mine. What about these?
These are actually tested...

I'm still in process of applying the pending patches. Due to patchwork.kernel.org
troubles (including the loss of about 270 patches from its SQL database only 
recovered yesterday[1]), I have a long backlog. So, I'm gradually applying the remaing
stuff. It will take some time though, and it will depend on patchwork mood, but I intend
to spend some time during this weekend to minimize the backlog.


Cheers,
Mauro

[1] The recover lost the email's body/SOB, so I've wrote a script to use my email
queue to get the data, using patchwork just to mark what patches were already
processed. This increses the time I have to spend on each patch, as I need to run
a script to match the patchwork patch with the patch ID inside my email queue.

Ah ok, no time pressure over here.. just wanted to make sure
that these don't get lost.

Regards and thanks,
Flo

Em 01-05-2011 07:38, Florian Mickler escreveu:

On Sat, 30 Apr 2011 19:30:52 -0300
Mauro Carvalho Chehab [off-list ref] wrote:

quoted

Hi Florian,

Em 30-04-2011 15:54, Florian Mickler escreveu:

quoted

Hi Mauro!

I just saw that you picked up some patches of mine. What about these?
These are actually tested...

I'm still in process of applying the pending patches. Due to patchwork.kernel.org
troubles (including the loss of about 270 patches from its SQL database only 
recovered yesterday[1]), I have a long backlog. So, I'm gradually applying the remaing
stuff. It will take some time though, and it will depend on patchwork mood, but I intend
to spend some time during this weekend to minimize the backlog.


Cheers,
Mauro

[1] The recover lost the email's body/SOB, so I've wrote a script to use my email
queue to get the data, using patchwork just to mark what patches were already
processed. This increses the time I have to spend on each patch, as I need to run
a script to match the patchwork patch with the patch ID inside my email queue.

Ah ok, no time pressure over here.. just wanted to make sure
that these don't get lost.

I think I've applied your series yesterday. Yet, patchwork is currently at bad mood, and it is
not allowing to mark your patches as applied (and the previous series as superseded).

I suspect that I'll need to completely abandon patchwork and work on a new way for handling it
that doesn't depend (nor will update) patchwork.kernel.org.

Mauro.