linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv1 1/3] Bluetooth: btmrvl: Correct num_block name
@ 2012-09-27  7:51 Andrei Emeltchenko
  2012-09-27  7:51 ` [PATCHv1 2/3] Bluetooth: btmrvl: Use DIV_ROUND_UP macro Andrei Emeltchenko
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Andrei Emeltchenko @ 2012-09-27  7:51 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Make code readable by correcting name from buf_block_len to num_blocks
since it represent number of blocks; NOT a length of a block buffer.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 drivers/bluetooth/btmrvl_sdio.c |   13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index da4df9c..25dc53b 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -492,7 +492,7 @@ done:
 static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
 {
 	u16 buf_len = 0;
-	int ret, buf_block_len, blksz;
+	int ret, num_blocks, blksz;
 	struct sk_buff *skb = NULL;
 	u32 type;
 	u8 *payload = NULL;
@@ -516,18 +516,17 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
 	BT_DBG("%s buf_len %zu", hdev->name, buf_len);
 
 	blksz = SDIO_BLOCK_SIZE;
-	buf_block_len = (buf_len + blksz - 1) / blksz;
+	num_blocks = (buf_len + blksz - 1) / blksz;
 
 	if (buf_len <= SDIO_HEADER_LEN
-			|| (buf_block_len * blksz) > ALLOC_BUF_SIZE) {
+	    || (num_blocks * blksz) > ALLOC_BUF_SIZE) {
 		BT_ERR("invalid packet length: %d", buf_len);
 		ret = -EINVAL;
 		goto exit;
 	}
 
 	/* Allocate buffer */
-	skb = bt_skb_alloc(buf_block_len * blksz + BTSDIO_DMA_ALIGN,
-								GFP_ATOMIC);
+	skb = bt_skb_alloc(num_blocks * blksz + BTSDIO_DMA_ALIGN, GFP_ATOMIC);
 	if (skb == NULL) {
 		BT_ERR("No free skb");
 		goto exit;
@@ -543,7 +542,7 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
 	payload = skb->data;
 
 	ret = sdio_readsb(card->func, payload, card->ioport,
-			  buf_block_len * blksz);
+			  num_blocks * blksz);
 	if (ret < 0) {
 		BT_ERR("readsb failed: %d", ret);
 		ret = -EIO;
@@ -594,7 +593,7 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
 	default:
 		BT_ERR("Unknown packet type:%d", type);
 		print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, payload,
-						blksz * buf_block_len);
+				     blksz * num_blocks);
 
 		kfree_skb(skb);
 		skb = NULL;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCHv1 2/3] Bluetooth: btmrvl: Use DIV_ROUND_UP macro
  2012-09-27  7:51 [PATCHv1 1/3] Bluetooth: btmrvl: Correct num_block name Andrei Emeltchenko
@ 2012-09-27  7:51 ` Andrei Emeltchenko
  2012-09-27  7:51 ` [PATCHv1 3/3] Bluetooth: btmrvl: Fix skb buffer overflow Andrei Emeltchenko
  2012-09-27 20:48 ` [PATCHv1 1/3] Bluetooth: btmrvl: Correct num_block name Gustavo Padovan
  2 siblings, 0 replies; 9+ messages in thread
From: Andrei Emeltchenko @ 2012-09-27  7:51 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

The kernel.h macro DIV_ROUND_UP performs the computation
(((n) + (d) - 1) / (d))

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 drivers/bluetooth/btmrvl_sdio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index 25dc53b..1457f25 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -516,7 +516,7 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
 	BT_DBG("%s buf_len %zu", hdev->name, buf_len);
 
 	blksz = SDIO_BLOCK_SIZE;
-	num_blocks = (buf_len + blksz - 1) / blksz;
+	num_blocks = DIV_ROUND_UP(buf_len, blksz);
 
 	if (buf_len <= SDIO_HEADER_LEN
 	    || (num_blocks * blksz) > ALLOC_BUF_SIZE) {
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCHv1 3/3] Bluetooth: btmrvl: Fix skb buffer overflow
  2012-09-27  7:51 [PATCHv1 1/3] Bluetooth: btmrvl: Correct num_block name Andrei Emeltchenko
  2012-09-27  7:51 ` [PATCHv1 2/3] Bluetooth: btmrvl: Use DIV_ROUND_UP macro Andrei Emeltchenko
@ 2012-09-27  7:51 ` Andrei Emeltchenko
  2012-09-27 20:51   ` Gustavo Padovan
  2012-09-27 20:48 ` [PATCHv1 1/3] Bluetooth: btmrvl: Correct num_block name Gustavo Padovan
  2 siblings, 1 reply; 9+ messages in thread
From: Andrei Emeltchenko @ 2012-09-27  7:51 UTC (permalink / raw)
  To: linux-bluetooth

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Add extra check to avoid skb buffer overflow. Fixes crash below:

 [  101.030427] ------------[ cut here ]------------
 [  101.030459] kernel BUG at net/core/skbuff.c:127!
 [  101.030486] invalid opcode: 0000 [#1] SMP
...
 [  101.030806] Pid: 2010, comm: btmrvl_main_ser Not tainted 3.5.0+ #80 Laptop
 [  101.030859] EIP: 0060:[<c14f2ba9>] EFLAGS: 00010282 CPU: 0
 [  101.030894] EIP is at skb_put+0x99/0xa0
 [  101.030919] EAX: 00000080 EBX: f129380b ECX: ef923540 EDX: 00000001
 [  101.030956] ESI: f00a4000 EDI: 00001003 EBP: ed4a5efc ESP: ed4a5ecc
 [  101.030992]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
 [  101.031024] CR0: 8005003b CR2: 08fca014 CR3: 30960000 CR4: 000407f0
 [  101.031062] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
 [  101.031100] DR6: ffff0ff0 DR7: 00000400
 [  101.031125] Process btmrvl_main_ser (pid: 2010, ti=ed4a4000 task=ef923540 task.ti=ed4a4000)
 [  101.031174] Stack:
 [  101.031188]  c18126f8 c1651938 f853f8d2 00001003 00001003 f1292800 f1292808 f129380b
 [  101.031250]  f1292940 f00a4000 eddb1280 efc0f9c0 ed4a5f44 f853f8d2 00000040 00000000
 [  101.031312]  ef923540 c15ee096 ef923540 eddb12d4 00000004 f00a4000 00000040 00000000
 [  101.031376] Call Trace:
 [  101.031396]  [<f853f8d2>] ? btmrvl_sdio_process_int_status+0x272/0x3d0 [btmrvl_sdio]
 [  101.031444]  [<f853f8d2>] btmrvl_sdio_process_int_status+0x272/0x3d0 [btmrvl_sdio]
 [  101.031488]  [<c15ee096>] ? _raw_spin_unlock_irqrestore+0x36/0x70
 [  101.031526]  [<f85a46e4>] btmrvl_service_main_thread+0x244/0x300 [btmrvl]
 [  101.031568]  [<f853fb50>] ? btmrvl_sdio_poll_card_status.isra.6.constprop.7+0x90/0x90 [btmrvl_sdio]
 [  101.031619]  [<c107eda0>] ? try_to_wake_up+0x270/0x270
 [  101.031648]  [<f85a44a0>] ? btmrvl_process_event+0x3b0/0x3b0 [btmrvl]
 [  101.031686]  [<c106d19d>] kthread+0x7d/0x90
 [  101.031713]  [<c106d120>] ? flush_kthread_work+0x150/0x150
 [  101.031745]  [<c15f5a82>] kernel_thread_helper+0x6/0x10
...
 [  101.032008] EIP: [<c14f2ba9>] skb_put+0x99/0xa0 SS:ESP 0068:ed4a5ecc
 [  101.056125] ---[ end trace a0bd01d1a9a796c8 ]---

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 drivers/bluetooth/btmrvl_sdio.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index 1457f25..b3b3680 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -554,7 +554,16 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
 	 */
 
 	buf_len = payload[0];
-	buf_len |= (u16) payload[1] << 8;
+	buf_len |= payload[1] << 8;
+	buf_len |= payload[2] << 16;
+
+	if (buf_len > blksz * num_blocks) {
+		BT_ERR("Skip incorrect packet: hdrlen %d buffer %d",
+		       buf_len, blksz * num_blocks);
+		ret = -EIO;
+		goto exit;
+	}
+
 	type = payload[3];
 
 	BT_DBG("%s len %zu type %d", hdev->name, buf_len, type);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCHv1 1/3] Bluetooth: btmrvl: Correct num_block name
  2012-09-27  7:51 [PATCHv1 1/3] Bluetooth: btmrvl: Correct num_block name Andrei Emeltchenko
  2012-09-27  7:51 ` [PATCHv1 2/3] Bluetooth: btmrvl: Use DIV_ROUND_UP macro Andrei Emeltchenko
  2012-09-27  7:51 ` [PATCHv1 3/3] Bluetooth: btmrvl: Fix skb buffer overflow Andrei Emeltchenko
@ 2012-09-27 20:48 ` Gustavo Padovan
  2 siblings, 0 replies; 9+ messages in thread
From: Gustavo Padovan @ 2012-09-27 20:48 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth

Hi Andrei,

* Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> [2012-09-27 10:51:46 +0300]:

> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Make code readable by correcting name from buf_block_len to num_blocks
> since it represent number of blocks; NOT a length of a block buffer.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  drivers/bluetooth/btmrvl_sdio.c |   13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
> index da4df9c..25dc53b 100644
> --- a/drivers/bluetooth/btmrvl_sdio.c
> +++ b/drivers/bluetooth/btmrvl_sdio.c
> @@ -492,7 +492,7 @@ done:
>  static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
>  {
>  	u16 buf_len = 0;
> -	int ret, buf_block_len, blksz;
> +	int ret, num_blocks, blksz;
>  	struct sk_buff *skb = NULL;
>  	u32 type;
>  	u8 *payload = NULL;
> @@ -516,18 +516,17 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
>  	BT_DBG("%s buf_len %zu", hdev->name, buf_len);
>  
>  	blksz = SDIO_BLOCK_SIZE;
> -	buf_block_len = (buf_len + blksz - 1) / blksz;
> +	num_blocks = (buf_len + blksz - 1) / blksz;
>  
>  	if (buf_len <= SDIO_HEADER_LEN
> -			|| (buf_block_len * blksz) > ALLOC_BUF_SIZE) {
> +	    || (num_blocks * blksz) > ALLOC_BUF_SIZE) {
>  		BT_ERR("invalid packet length: %d", buf_len);
>  		ret = -EINVAL;
>  		goto exit;
>  	}
>  
>  	/* Allocate buffer */
> -	skb = bt_skb_alloc(buf_block_len * blksz + BTSDIO_DMA_ALIGN,
> -								GFP_ATOMIC);
> +	skb = bt_skb_alloc(num_blocks * blksz + BTSDIO_DMA_ALIGN, GFP_ATOMIC);
>  	if (skb == NULL) {
>  		BT_ERR("No free skb");
>  		goto exit;
> @@ -543,7 +542,7 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
>  	payload = skb->data;
>  
>  	ret = sdio_readsb(card->func, payload, card->ioport,
> -			  buf_block_len * blksz);
> +			  num_blocks * blksz);
>  	if (ret < 0) {
>  		BT_ERR("readsb failed: %d", ret);
>  		ret = -EIO;
> @@ -594,7 +593,7 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
>  	default:
>  		BT_ERR("Unknown packet type:%d", type);
>  		print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, payload,
> -						blksz * buf_block_len);
> +				     blksz * num_blocks);
>  
>  		kfree_skb(skb);
>  		skb = NULL;

By some reason this patch fails to apply:

Applying: Bluetooth: btmrvl: Correct num_block name
fatal: sha1 information is lacking or useless
(drivers/bluetooth/btmrvl_sdio.c).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bluetooth: btmrvl: Correct num_block name

	Gustavo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCHv1 3/3] Bluetooth: btmrvl: Fix skb buffer overflow
  2012-09-27  7:51 ` [PATCHv1 3/3] Bluetooth: btmrvl: Fix skb buffer overflow Andrei Emeltchenko
@ 2012-09-27 20:51   ` Gustavo Padovan
  2012-09-28 11:36     ` [PATCH 1/3] Bluetooth: btmrvl: Correct num_block name Andrei Emeltchenko
  0 siblings, 1 reply; 9+ messages in thread
From: Gustavo Padovan @ 2012-09-27 20:51 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth

Hi Andrei,

* Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> [2012-09-27 10:51:48 +0300]:

> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Add extra check to avoid skb buffer overflow. Fixes crash below:
> 
>  [  101.030427] ------------[ cut here ]------------
>  [  101.030459] kernel BUG at net/core/skbuff.c:127!
>  [  101.030486] invalid opcode: 0000 [#1] SMP
> ...
>  [  101.030806] Pid: 2010, comm: btmrvl_main_ser Not tainted 3.5.0+ #80 Laptop
>  [  101.030859] EIP: 0060:[<c14f2ba9>] EFLAGS: 00010282 CPU: 0
>  [  101.030894] EIP is at skb_put+0x99/0xa0
>  [  101.030919] EAX: 00000080 EBX: f129380b ECX: ef923540 EDX: 00000001
>  [  101.030956] ESI: f00a4000 EDI: 00001003 EBP: ed4a5efc ESP: ed4a5ecc
>  [  101.030992]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
>  [  101.031024] CR0: 8005003b CR2: 08fca014 CR3: 30960000 CR4: 000407f0
>  [  101.031062] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
>  [  101.031100] DR6: ffff0ff0 DR7: 00000400
>  [  101.031125] Process btmrvl_main_ser (pid: 2010, ti=ed4a4000 task=ef923540 task.ti=ed4a4000)
>  [  101.031174] Stack:
>  [  101.031188]  c18126f8 c1651938 f853f8d2 00001003 00001003 f1292800 f1292808 f129380b
>  [  101.031250]  f1292940 f00a4000 eddb1280 efc0f9c0 ed4a5f44 f853f8d2 00000040 00000000
>  [  101.031312]  ef923540 c15ee096 ef923540 eddb12d4 00000004 f00a4000 00000040 00000000
>  [  101.031376] Call Trace:
>  [  101.031396]  [<f853f8d2>] ? btmrvl_sdio_process_int_status+0x272/0x3d0 [btmrvl_sdio]
>  [  101.031444]  [<f853f8d2>] btmrvl_sdio_process_int_status+0x272/0x3d0 [btmrvl_sdio]
>  [  101.031488]  [<c15ee096>] ? _raw_spin_unlock_irqrestore+0x36/0x70
>  [  101.031526]  [<f85a46e4>] btmrvl_service_main_thread+0x244/0x300 [btmrvl]
>  [  101.031568]  [<f853fb50>] ? btmrvl_sdio_poll_card_status.isra.6.constprop.7+0x90/0x90 [btmrvl_sdio]
>  [  101.031619]  [<c107eda0>] ? try_to_wake_up+0x270/0x270
>  [  101.031648]  [<f85a44a0>] ? btmrvl_process_event+0x3b0/0x3b0 [btmrvl]
>  [  101.031686]  [<c106d19d>] kthread+0x7d/0x90
>  [  101.031713]  [<c106d120>] ? flush_kthread_work+0x150/0x150
>  [  101.031745]  [<c15f5a82>] kernel_thread_helper+0x6/0x10
> ...
>  [  101.032008] EIP: [<c14f2ba9>] skb_put+0x99/0xa0 SS:ESP 0068:ed4a5ecc
>  [  101.056125] ---[ end trace a0bd01d1a9a796c8 ]---
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  drivers/bluetooth/btmrvl_sdio.c |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
> index 1457f25..b3b3680 100644
> --- a/drivers/bluetooth/btmrvl_sdio.c
> +++ b/drivers/bluetooth/btmrvl_sdio.c
> @@ -554,7 +554,16 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
>  	 */
>  
>  	buf_len = payload[0];
> -	buf_len |= (u16) payload[1] << 8;
> +	buf_len |= payload[1] << 8;
> +	buf_len |= payload[2] << 16;
> +
> +	if (buf_len > blksz * num_blocks) {
> +		BT_ERR("Skip incorrect packet: hdrlen %d buffer %d",
> +		       buf_len, blksz * num_blocks);
> +		ret = -EIO;
> +		goto exit;
> +	}
> +
>  	type = payload[3];
>  
>  	BT_DBG("%s len %zu type %d", hdev->name, buf_len, type);

All three patches doesn't apply. Please check them.

	Gustavo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] Bluetooth: btmrvl: Correct num_block name
  2012-09-27 20:51   ` Gustavo Padovan
@ 2012-09-28 11:36     ` Andrei Emeltchenko
  2012-09-28 11:36       ` [PATCH 2/3] Bluetooth: btmrvl: Use DIV_ROUND_UP macro Andrei Emeltchenko
  2012-09-28 11:36       ` [PATCH 3/3] Bluetooth: btmrvl: Fix skb buffer overflow Andrei Emeltchenko
  0 siblings, 2 replies; 9+ messages in thread
From: Andrei Emeltchenko @ 2012-09-28 11:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: bzhao

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Make code readable by correcting name from buf_block_len to num_blocks
since it represent number of blocks; NOT a length of a block buffer.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 drivers/bluetooth/btmrvl_sdio.c |   13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index 3f4bfc8..645b42e 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -492,7 +492,7 @@ done:
 static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
 {
 	u16 buf_len = 0;
-	int ret, buf_block_len, blksz;
+	int ret, num_blocks, blksz;
 	struct sk_buff *skb = NULL;
 	u32 type;
 	u8 *payload = NULL;
@@ -514,18 +514,17 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
 	}
 
 	blksz = SDIO_BLOCK_SIZE;
-	buf_block_len = (buf_len + blksz - 1) / blksz;
+	num_blocks = (buf_len + blksz - 1) / blksz;
 
 	if (buf_len <= SDIO_HEADER_LEN
-			|| (buf_block_len * blksz) > ALLOC_BUF_SIZE) {
+	    || (num_blocks * blksz) > ALLOC_BUF_SIZE) {
 		BT_ERR("invalid packet length: %d", buf_len);
 		ret = -EINVAL;
 		goto exit;
 	}
 
 	/* Allocate buffer */
-	skb = bt_skb_alloc(buf_block_len * blksz + BTSDIO_DMA_ALIGN,
-								GFP_ATOMIC);
+	skb = bt_skb_alloc(num_blocks * blksz + BTSDIO_DMA_ALIGN, GFP_ATOMIC);
 	if (skb == NULL) {
 		BT_ERR("No free skb");
 		goto exit;
@@ -541,7 +540,7 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
 	payload = skb->data;
 
 	ret = sdio_readsb(card->func, payload, card->ioport,
-			  buf_block_len * blksz);
+			  num_blocks * blksz);
 	if (ret < 0) {
 		BT_ERR("readsb failed: %d", ret);
 		ret = -EIO;
@@ -590,7 +589,7 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
 	default:
 		BT_ERR("Unknown packet type:%d", type);
 		print_hex_dump_bytes("", DUMP_PREFIX_OFFSET, payload,
-						blksz * buf_block_len);
+				     blksz * num_blocks);
 
 		kfree_skb(skb);
 		skb = NULL;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] Bluetooth: btmrvl: Use DIV_ROUND_UP macro
  2012-09-28 11:36     ` [PATCH 1/3] Bluetooth: btmrvl: Correct num_block name Andrei Emeltchenko
@ 2012-09-28 11:36       ` Andrei Emeltchenko
  2012-09-28 11:36       ` [PATCH 3/3] Bluetooth: btmrvl: Fix skb buffer overflow Andrei Emeltchenko
  1 sibling, 0 replies; 9+ messages in thread
From: Andrei Emeltchenko @ 2012-09-28 11:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: bzhao

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

The kernel.h macro DIV_ROUND_UP performs the computation
(((n) + (d) - 1) / (d))

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 drivers/bluetooth/btmrvl_sdio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index 645b42e..ec5c456 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -514,7 +514,7 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
 	}
 
 	blksz = SDIO_BLOCK_SIZE;
-	num_blocks = (buf_len + blksz - 1) / blksz;
+	num_blocks = DIV_ROUND_UP(buf_len, blksz);
 
 	if (buf_len <= SDIO_HEADER_LEN
 	    || (num_blocks * blksz) > ALLOC_BUF_SIZE) {
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] Bluetooth: btmrvl: Fix skb buffer overflow
  2012-09-28 11:36     ` [PATCH 1/3] Bluetooth: btmrvl: Correct num_block name Andrei Emeltchenko
  2012-09-28 11:36       ` [PATCH 2/3] Bluetooth: btmrvl: Use DIV_ROUND_UP macro Andrei Emeltchenko
@ 2012-09-28 11:36       ` Andrei Emeltchenko
  2012-09-28 19:52         ` Gustavo Padovan
  1 sibling, 1 reply; 9+ messages in thread
From: Andrei Emeltchenko @ 2012-09-28 11:36 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: bzhao

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Add extra check to avoid skb buffer overflow. Fixes crash below:

 [  101.030427] ------------[ cut here ]------------
 [  101.030459] kernel BUG at net/core/skbuff.c:127!
 [  101.030486] invalid opcode: 0000 [#1] SMP
...
 [  101.030806] Pid: 2010, comm: btmrvl_main_ser Not tainted 3.5.0+ #80 Laptop
 [  101.030859] EIP: 0060:[<c14f2ba9>] EFLAGS: 00010282 CPU: 0
 [  101.030894] EIP is at skb_put+0x99/0xa0
 [  101.030919] EAX: 00000080 EBX: f129380b ECX: ef923540 EDX: 00000001
 [  101.030956] ESI: f00a4000 EDI: 00001003 EBP: ed4a5efc ESP: ed4a5ecc
 [  101.030992]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
 [  101.031024] CR0: 8005003b CR2: 08fca014 CR3: 30960000 CR4: 000407f0
 [  101.031062] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
 [  101.031100] DR6: ffff0ff0 DR7: 00000400
 [  101.031125] Process btmrvl_main_ser (pid: 2010, ti=ed4a4000 task=ef923540 task.ti=ed4a4000)
 [  101.031174] Stack:
 [  101.031188]  c18126f8 c1651938 f853f8d2 00001003 00001003 f1292800 f1292808 f129380b
 [  101.031250]  f1292940 f00a4000 eddb1280 efc0f9c0 ed4a5f44 f853f8d2 00000040 00000000
 [  101.031312]  ef923540 c15ee096 ef923540 eddb12d4 00000004 f00a4000 00000040 00000000
 [  101.031376] Call Trace:
 [  101.031396]  [<f853f8d2>] ? btmrvl_sdio_process_int_status+0x272/0x3d0 [btmrvl_sdio]
 [  101.031444]  [<f853f8d2>] btmrvl_sdio_process_int_status+0x272/0x3d0 [btmrvl_sdio]
 [  101.031488]  [<c15ee096>] ? _raw_spin_unlock_irqrestore+0x36/0x70
 [  101.031526]  [<f85a46e4>] btmrvl_service_main_thread+0x244/0x300 [btmrvl]
 [  101.031568]  [<f853fb50>] ? btmrvl_sdio_poll_card_status.isra.6.constprop.7+0x90/0x90 [btmrvl_sdio]
 [  101.031619]  [<c107eda0>] ? try_to_wake_up+0x270/0x270
 [  101.031648]  [<f85a44a0>] ? btmrvl_process_event+0x3b0/0x3b0 [btmrvl]
 [  101.031686]  [<c106d19d>] kthread+0x7d/0x90
 [  101.031713]  [<c106d120>] ? flush_kthread_work+0x150/0x150
 [  101.031745]  [<c15f5a82>] kernel_thread_helper+0x6/0x10
...
 [  101.032008] EIP: [<c14f2ba9>] skb_put+0x99/0xa0 SS:ESP 0068:ed4a5ecc
 [  101.056125] ---[ end trace a0bd01d1a9a796c8 ]---

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 drivers/bluetooth/btmrvl_sdio.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index ec5c456..1896e91 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -552,7 +552,16 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
 	 */
 
 	buf_len = payload[0];
-	buf_len |= (u16) payload[1] << 8;
+	buf_len |= payload[1] << 8;
+	buf_len |= payload[2] << 16;
+
+	if (buf_len > blksz * num_blocks) {
+		BT_ERR("Skip incorrect packet: hdrlen %d buffer %d",
+		       buf_len, blksz * num_blocks);
+		ret = -EIO;
+		goto exit;
+	}
+
 	type = payload[3];
 
 	switch (type) {
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] Bluetooth: btmrvl: Fix skb buffer overflow
  2012-09-28 11:36       ` [PATCH 3/3] Bluetooth: btmrvl: Fix skb buffer overflow Andrei Emeltchenko
@ 2012-09-28 19:52         ` Gustavo Padovan
  0 siblings, 0 replies; 9+ messages in thread
From: Gustavo Padovan @ 2012-09-28 19:52 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth, bzhao

Hi Andrei,

* Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> [2012-09-28 14:36:10 +0300]:

> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Add extra check to avoid skb buffer overflow. Fixes crash below:
> 
>  [  101.030427] ------------[ cut here ]------------
>  [  101.030459] kernel BUG at net/core/skbuff.c:127!
>  [  101.030486] invalid opcode: 0000 [#1] SMP
> ...
>  [  101.030806] Pid: 2010, comm: btmrvl_main_ser Not tainted 3.5.0+ #80 Laptop
>  [  101.030859] EIP: 0060:[<c14f2ba9>] EFLAGS: 00010282 CPU: 0
>  [  101.030894] EIP is at skb_put+0x99/0xa0
>  [  101.030919] EAX: 00000080 EBX: f129380b ECX: ef923540 EDX: 00000001
>  [  101.030956] ESI: f00a4000 EDI: 00001003 EBP: ed4a5efc ESP: ed4a5ecc
>  [  101.030992]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
>  [  101.031024] CR0: 8005003b CR2: 08fca014 CR3: 30960000 CR4: 000407f0
>  [  101.031062] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
>  [  101.031100] DR6: ffff0ff0 DR7: 00000400
>  [  101.031125] Process btmrvl_main_ser (pid: 2010, ti=ed4a4000 task=ef923540 task.ti=ed4a4000)
>  [  101.031174] Stack:
>  [  101.031188]  c18126f8 c1651938 f853f8d2 00001003 00001003 f1292800 f1292808 f129380b
>  [  101.031250]  f1292940 f00a4000 eddb1280 efc0f9c0 ed4a5f44 f853f8d2 00000040 00000000
>  [  101.031312]  ef923540 c15ee096 ef923540 eddb12d4 00000004 f00a4000 00000040 00000000
>  [  101.031376] Call Trace:
>  [  101.031396]  [<f853f8d2>] ? btmrvl_sdio_process_int_status+0x272/0x3d0 [btmrvl_sdio]
>  [  101.031444]  [<f853f8d2>] btmrvl_sdio_process_int_status+0x272/0x3d0 [btmrvl_sdio]
>  [  101.031488]  [<c15ee096>] ? _raw_spin_unlock_irqrestore+0x36/0x70
>  [  101.031526]  [<f85a46e4>] btmrvl_service_main_thread+0x244/0x300 [btmrvl]
>  [  101.031568]  [<f853fb50>] ? btmrvl_sdio_poll_card_status.isra.6.constprop.7+0x90/0x90 [btmrvl_sdio]
>  [  101.031619]  [<c107eda0>] ? try_to_wake_up+0x270/0x270
>  [  101.031648]  [<f85a44a0>] ? btmrvl_process_event+0x3b0/0x3b0 [btmrvl]
>  [  101.031686]  [<c106d19d>] kthread+0x7d/0x90
>  [  101.031713]  [<c106d120>] ? flush_kthread_work+0x150/0x150
>  [  101.031745]  [<c15f5a82>] kernel_thread_helper+0x6/0x10
> ...
>  [  101.032008] EIP: [<c14f2ba9>] skb_put+0x99/0xa0 SS:ESP 0068:ed4a5ecc
>  [  101.056125] ---[ end trace a0bd01d1a9a796c8 ]---
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  drivers/bluetooth/btmrvl_sdio.c |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)

All 3 patches were applied. Thanks.

	Gustavo

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-09-28 19:52 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-27  7:51 [PATCHv1 1/3] Bluetooth: btmrvl: Correct num_block name Andrei Emeltchenko
2012-09-27  7:51 ` [PATCHv1 2/3] Bluetooth: btmrvl: Use DIV_ROUND_UP macro Andrei Emeltchenko
2012-09-27  7:51 ` [PATCHv1 3/3] Bluetooth: btmrvl: Fix skb buffer overflow Andrei Emeltchenko
2012-09-27 20:51   ` Gustavo Padovan
2012-09-28 11:36     ` [PATCH 1/3] Bluetooth: btmrvl: Correct num_block name Andrei Emeltchenko
2012-09-28 11:36       ` [PATCH 2/3] Bluetooth: btmrvl: Use DIV_ROUND_UP macro Andrei Emeltchenko
2012-09-28 11:36       ` [PATCH 3/3] Bluetooth: btmrvl: Fix skb buffer overflow Andrei Emeltchenko
2012-09-28 19:52         ` Gustavo Padovan
2012-09-27 20:48 ` [PATCHv1 1/3] Bluetooth: btmrvl: Correct num_block name Gustavo Padovan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).