[PATCH 1/3] [media] siano: firmware buffer is too small

[PATCH 1/3] [media] siano: firmware buffer is too small

[Mauro Carvalho Chehab]

As pointed by KASAN:

	BUG: KASAN: slab-out-of-bounds in memcpy+0x1d/0x40 at addr ffff880000038d8c
	Read of size 128 by task systemd-udevd/2536
	page:ffffea0000000800 count:1 mapcount:0 mapping:          (null) index:0x0 compound_mapcount: 0
	flags: 0xffff8000004000(head)
	page dumped because: kasan: bad access detected
	CPU: 1 PID: 2536 Comm: systemd-udevd Not tainted 4.5.0-rc3+ #47
	Hardware name:                  /NUC5i7RYB, BIOS RYBDWi35.86A.0350.2015.0812.1722 08/12/2015
	  ffff880000038d8c ffff8803b0f1f1e8 ffffffff81933901 0000000000000080
	  ffff8803b0f1f280 ffff8803b0f1f270 ffffffff815602c5 ffffffff8284cf93
	  ffffffff822ddc00 0000000000000282 0000000000000001 ffff88009c7c6000
	Call Trace:
	  [<ffffffff81933901>] dump_stack+0x85/0xc4
	  [<ffffffff815602c5>] kasan_report_error+0x525/0x550
	  [<ffffffff815606e9>] kasan_report+0x39/0x40
	  [<ffffffff8155f84d>] memcpy+0x1d/0x40
	  [<ffffffffa120cb90>] smscore_set_device_mode+0xee0/0x2560 [smsmdtv]

Such error happens at the memcpy code below:

0x4bc0 is in smscore_set_device_mode (drivers/media/common/siano/smscoreapi.c:975).
970					      sizeof(u32) + payload_size));
971
972			data_msg->mem_addr = mem_address;
973			memcpy(data_msg->payload, payload, payload_size);
974
975			rc = smscore_sendrequest_and_wait(coredev, data_msg,
976					data_msg->x_msg_header.msg_length,
977					&coredev->data_download_done);
978
979			payload += payload_size;

The problem is that the Siano driver uses a header to store the firmware,
with requires a few more bytes than allocated.

Tested with:
	PCTV 77e (2013:0257)
	Hauppauge WinTV MiniStick (2040:5510)

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
 drivers/media/common/siano/smscoreapi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/common/siano/smscoreapi.c b/drivers/media/common/siano/smscoreapi.c
index 2a8d9a36d6f0..f3a42834d7d6 100644
--- a/drivers/media/common/siano/smscoreapi.c
+++ b/drivers/media/common/siano/smscoreapi.c
@@ -1167,8 +1167,8 @@ static int smscore_load_firmware_from_file(struct smscore_device_t *coredev,
 		return rc;
 	}
 	pr_debug("read fw %s, buffer size=0x%zx\n", fw_filename, fw->size);
-	fw_buf = kmalloc(ALIGN(fw->size, SMS_ALLOC_ALIGNMENT),
-			 GFP_KERNEL | GFP_DMA);
+	fw_buf = kmalloc(ALIGN(fw->size + sizeof(struct sms_firmware),
+			 SMS_ALLOC_ALIGNMENT), GFP_KERNEL | GFP_DMA);
 	if (!fw_buf) {
 		pr_err("failed to allocate firmware buffer\n");
 		rc = -ENOMEM;
-- 
2.5.0

[PATCH 2/3] [media] smsusb: don't sleep while atomic

[Mauro Carvalho Chehab]

smscore_getbuffer() calls internally wait_event(), with can sleep.
As smsusb_onresponse() is called on interrupt context, this causes
the following warning:

	BUG: sleeping function called from invalid context at drivers/media/common/siano/smscoreapi.c:1653
	in_atomic(): 1, irqs_disabled(): 1, pid: 11084, name: systemd-udevd
	INFO: lockdep is turned off.
	irq event stamp: 0
	hardirqs last  enabled at (0): [<          (null)>]           (null)
	hardirqs last disabled at (0): [<ffffffff811480f7>] copy_process.part.7+0x10e7/0x56d0
	softirqs last  enabled at (0): [<ffffffff81148193>] copy_process.part.7+0x1183/0x56d0
	softirqs last disabled at (0): [<          (null)>]           (null)
	CPU: 2 PID: 11084 Comm: systemd-udevd Tainted: G    B   W       4.5.0-rc3+ #47
	Hardware name:                  /NUC5i7RYB, BIOS RYBDWi35.86A.0350.2015.0812.1722 08/12/2015
	 0000000000000000 ffff8803c6907a80 ffffffff81933901 ffff8802bd916000
	 ffff8802bd9165c8 ffff8803c6907aa8 ffffffff811c6af5 ffff8802bd916000
	 ffffffffa0ce9b60 0000000000000675 ffff8803c6907ae8 ffffffff811c6ce5
	Call Trace:
	 <IRQ>  [<ffffffff81933901>] dump_stack+0x85/0xc4
	 [<ffffffff811c6af5>] ___might_sleep+0x245/0x3a0
	 [<ffffffff811c6ce5>] __might_sleep+0x95/0x1a0
	 [<ffffffffa0ce020a>] ? list_add_locked+0xca/0x140 [smsmdtv]
	 [<ffffffffa0ce3b8d>] smscore_getbuffer+0x7d/0x120 [smsmdtv]
	 [<ffffffff8123819d>] ? trace_hardirqs_off+0xd/0x10
	 [<ffffffffa0ce3b10>] ? smscore_sendrequest_and_wait.isra.5+0x120/0x120 [smsmdtv]
	 [<ffffffffa0ce020a>] ? list_add_locked+0xca/0x140 [smsmdtv]
	 [<ffffffffa0ce13ca>] ? smscore_putbuffer+0x3a/0x40 [smsmdtv]
	 [<ffffffffa0d107bc>] smsusb_submit_urb+0x2ec/0x4f0 [smsusb]
	 [<ffffffffa0d10e36>] smsusb_onresponse+0x476/0x720 [smsusb]

Let's add a work queue to handle the bottom half, preventing this
problem.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
 drivers/media/usb/siano/smsusb.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/siano/smsusb.c b/drivers/media/usb/siano/smsusb.c
index 6cb4be6dddbb..8e0c05271a33 100644
--- a/drivers/media/usb/siano/smsusb.c
+++ b/drivers/media/usb/siano/smsusb.c
@@ -52,6 +52,9 @@ struct smsusb_urb_t {
 	struct smsusb_device_t *dev;
 
 	struct urb urb;
+
+	/* For the bottom half */
+	struct work_struct wq;
 };
 
 struct smsusb_device_t {
@@ -72,6 +75,18 @@ static int smsusb_submit_urb(struct smsusb_device_t *dev,
 			     struct smsusb_urb_t *surb);
 
 /**
+ * Completing URB's callback handler - bottom half (proccess context)
+ * submits the URB prepared on smsusb_onresponse()
+ */
+static void do_submit_urb(struct work_struct *work)
+{
+	struct smsusb_urb_t *surb = container_of(work, struct smsusb_urb_t, wq);
+	struct smsusb_device_t *dev = surb->dev;
+
+	smsusb_submit_urb(dev, surb);
+}
+
+/**
  * Completing URB's callback handler - top half (interrupt context)
  * adds completing sms urb to the global surbs list and activtes the worker
  * thread the surb
@@ -139,13 +154,15 @@ static void smsusb_onresponse(struct urb *urb)
 
 
 exit_and_resubmit:
-	smsusb_submit_urb(dev, surb);
+	INIT_WORK(&surb->wq, do_submit_urb);
+	schedule_work(&surb->wq);
 }
 
 static int smsusb_submit_urb(struct smsusb_device_t *dev,
 			     struct smsusb_urb_t *surb)
 {
 	if (!surb->cb) {
+		/* This function can sleep */
 		surb->cb = smscore_getbuffer(dev->coredev);
 		if (!surb->cb) {
 			pr_err("smscore_getbuffer(...) returned NULL\n");
-- 
2.5.0

[PATCH 3/3] [media] siano: use generic function to create MC device

[Mauro Carvalho Chehab]

Currently, it is initializing the driver name using the wrong
name ("usb"). Use the generic function, as its logic works
best, and avoids repeating the very same code everywhere.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
 drivers/media/usb/siano/smsusb.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/media/usb/siano/smsusb.c b/drivers/media/usb/siano/smsusb.c
index 8e0c05271a33..4dac499ed28e 100644
--- a/drivers/media/usb/siano/smsusb.c
+++ b/drivers/media/usb/siano/smsusb.c
@@ -367,20 +367,10 @@ static void *siano_media_device_register(struct smsusb_device_t *dev,
 	struct sms_board *board = sms_get_board(board_id);
 	int ret;
 
-	mdev = kzalloc(sizeof(*mdev), GFP_KERNEL);
+	mdev = v4l2_mc_usb_media_device_init(udev, board->name);
 	if (!mdev)
 		return NULL;
 
-	mdev->dev = &udev->dev;
-	strlcpy(mdev->model, board->name, sizeof(mdev->model));
-	if (udev->serial)
-		strlcpy(mdev->serial, udev->serial, sizeof(mdev->serial));
-	strcpy(mdev->bus_info, udev->devpath);
-	mdev->hw_revision = le16_to_cpu(udev->descriptor.bcdDevice);
-	mdev->driver_version = LINUX_VERSION_CODE;
-
-	media_device_init(mdev);
-
 	ret = media_device_register(mdev);
 	if (ret) {
 		media_device_cleanup(mdev);
-- 
2.5.0