* [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 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
* 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
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).