Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1313276 > unrolled thread
| Started by | Jacob Siverskog <jacob@teenage.engineering> |
|---|---|
| First post | 2016-01-20 16:10 +0100 |
| Last post | 2016-01-20 18:10 +0100 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram Jacob Siverskog <jacob@teenage.engineering> - 2016-01-20 16:10 +0100
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram Eric Dumazet <eric.dumazet@gmail.com> - 2016-01-20 16:50 +0100
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram Jacob Siverskog <jacob@teenage.engineering> - 2016-01-20 17:20 +0100
Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram Eric Dumazet <eric.dumazet@gmail.com> - 2016-01-20 18:10 +0100
| From | Jacob Siverskog <jacob@teenage.engineering> |
|---|---|
| Date | 2016-01-20 16:10 +0100 |
| Subject | Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram |
| Message-ID | <qT2yu-8hL-7@gated-at.bofh.it> |
On Tue, Jan 5, 2016 at 3:39 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Tue, 2016-01-05 at 15:34 +0100, Jacob Siverskog wrote: >> On Tue, Jan 5, 2016 at 3:14 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > >> > >> > You might build a kernel with KASAN support to get maybe more chances to >> > trigger the bug. >> > >> > ( https://www.kernel.org/doc/Documentation/kasan.txt ) >> > >> >> Ah. Doesn't seem to be supported on arm(32) unfortunately. > > Then you could at least use standard debugging features : > > CONFIG_SLAB=y > CONFIG_SLABINFO=y > CONFIG_DEBUG_SLAB=y > CONFIG_DEBUG_SLAB_LEAK=y > > (Or equivalent SLUB options) > > and > > CONFIG_DEBUG_PAGEALLOC=y > > (If arm(32) has CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y) I tried with those enabled and while toggling power on the Bluetooth interface I usually get this after a few iterations: kernel: Bluetooth: Unable to push skb to HCI core(-6) kernel: (stc): proto stack 4's ->recv failed kernel: Slab corruption (Not tainted): skbuff_head_cache start=c08a8a00, len=176 kernel: 0a0: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6a 6b 6b a5 kkkkkkkkkkkkjkk. kernel: Prev obj: start=c08a8940, len=176 kernel: 000: 00 00 00 00 00 00 00 00 31 73 52 00 43 17 2b 14 ........1sR.C.+. kernel: 010: 00 00 00 00 00 00 00 00 04 00 00 00 01 00 00 00 ................ kernel: Next obj: start=c08a8ac0, len=176 kernel: 000: 00 00 00 00 00 00 00 00 01 42 f6 50 36 17 2b 14 .........B.P6.+. kernel: 010: 00 00 00 00 00 00 00 00 04 00 00 00 01 00 00 00 ................ The "Unable to push skb" and "recv failed" lines always appear before the corruption. Unfortunately, the corruptions occur also with your patch.
[toc] | [next] | [standalone]
| From | Eric Dumazet <eric.dumazet@gmail.com> |
|---|---|
| Date | 2016-01-20 16:50 +0100 |
| Subject | Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram |
| Message-ID | <qT3bb-7a-11@gated-at.bofh.it> |
| In reply to | #1313276 |
On Wed, 2016-01-20 at 16:06 +0100, Jacob Siverskog wrote:
> On Tue, Jan 5, 2016 at 3:39 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > On Tue, 2016-01-05 at 15:34 +0100, Jacob Siverskog wrote:
> >> On Tue, Jan 5, 2016 at 3:14 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> >> >
> >> > You might build a kernel with KASAN support to get maybe more chances to
> >> > trigger the bug.
> >> >
> >> > ( https://www.kernel.org/doc/Documentation/kasan.txt )
> >> >
> >>
> >> Ah. Doesn't seem to be supported on arm(32) unfortunately.
> >
> > Then you could at least use standard debugging features :
> >
> > CONFIG_SLAB=y
> > CONFIG_SLABINFO=y
> > CONFIG_DEBUG_SLAB=y
> > CONFIG_DEBUG_SLAB_LEAK=y
> >
> > (Or equivalent SLUB options)
> >
> > and
> >
> > CONFIG_DEBUG_PAGEALLOC=y
> >
> > (If arm(32) has CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y)
>
> I tried with those enabled and while toggling power on the Bluetooth
> interface I usually get this after a few iterations:
> kernel: Bluetooth: Unable to push skb to HCI core(-6)
Well, this code seems to be quite buggy.
I do not have time to audit it, but 5 minutes are enough to spot 2
issues.
skb, once given to another queue/layer should not be accessed anymore.
diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
index 24a652f9252b..2d3092aa6cfe 100644
--- a/drivers/bluetooth/btwilink.c
+++ b/drivers/bluetooth/btwilink.c
@@ -98,6 +98,7 @@ static void st_reg_completion_cb(void *priv_data, char data)
static long st_receive(void *priv_data, struct sk_buff *skb)
{
struct ti_st *lhst = priv_data;
+ unsigned int len;
int err;
if (!skb)
@@ -109,13 +110,14 @@ static long st_receive(void *priv_data, struct sk_buff *skb)
}
/* Forward skb to HCI core layer */
+ len = skb->len;
err = hci_recv_frame(lhst->hdev, skb);
if (err < 0) {
BT_ERR("Unable to push skb to HCI core(%d)", err);
return err;
}
- lhst->hdev->stat.byte_rx += skb->len;
+ lhst->hdev->stat.byte_rx += len;
return 0;
}
@@ -245,6 +247,7 @@ static int ti_st_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
{
struct ti_st *hst;
long len;
+ u8 pkt_type;
hst = hci_get_drvdata(hdev);
@@ -258,6 +261,7 @@ static int ti_st_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
* Freeing skb memory is taken care in shared transport layer,
* so don't free skb memory here.
*/
+ pkt_type = hci_skb_pkt_type(skb);
len = hst->st_write(skb);
if (len < 0) {
kfree_skb(skb);
@@ -268,7 +272,7 @@ static int ti_st_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
/* ST accepted our skb. So, Go ahead and do rest */
hdev->stat.byte_tx += len;
- ti_st_tx_complete(hst, hci_skb_pkt_type(skb));
+ ti_st_tx_complete(hst, pkt_type);
return 0;
}
[toc] | [prev] | [next] | [standalone]
| From | Jacob Siverskog <jacob@teenage.engineering> |
|---|---|
| Date | 2016-01-20 17:20 +0100 |
| Message-ID | <qT3Ee-yP-11@gated-at.bofh.it> |
| In reply to | #1313310 |
On Wed, Jan 20, 2016 at 4:48 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Wed, 2016-01-20 at 16:06 +0100, Jacob Siverskog wrote: >> On Tue, Jan 5, 2016 at 3:39 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: >> > On Tue, 2016-01-05 at 15:34 +0100, Jacob Siverskog wrote: >> >> On Tue, Jan 5, 2016 at 3:14 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: >> > >> >> > >> >> > You might build a kernel with KASAN support to get maybe more chances to >> >> > trigger the bug. >> >> > >> >> > ( https://www.kernel.org/doc/Documentation/kasan.txt ) >> >> > >> >> >> >> Ah. Doesn't seem to be supported on arm(32) unfortunately. >> > >> > Then you could at least use standard debugging features : >> > >> > CONFIG_SLAB=y >> > CONFIG_SLABINFO=y >> > CONFIG_DEBUG_SLAB=y >> > CONFIG_DEBUG_SLAB_LEAK=y >> > >> > (Or equivalent SLUB options) >> > >> > and >> > >> > CONFIG_DEBUG_PAGEALLOC=y >> > >> > (If arm(32) has CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y) >> >> I tried with those enabled and while toggling power on the Bluetooth >> interface I usually get this after a few iterations: >> kernel: Bluetooth: Unable to push skb to HCI core(-6) > > Well, this code seems to be quite buggy. > > I do not have time to audit it, but 5 minutes are enough to spot 2 > issues. > > skb, once given to another queue/layer should not be accessed anymore. > Ok. Unfortunately I still see the slab corruption even with your changes.
[toc] | [prev] | [next] | [standalone]
| From | Eric Dumazet <eric.dumazet@gmail.com> |
|---|---|
| Date | 2016-01-20 18:10 +0100 |
| Subject | Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram |
| Message-ID | <qT4qC-1bh-29@gated-at.bofh.it> |
| In reply to | #1313328 |
On Wed, 2016-01-20 at 17:17 +0100, Jacob Siverskog wrote: > On Wed, Jan 20, 2016 at 4:48 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > > On Wed, 2016-01-20 at 16:06 +0100, Jacob Siverskog wrote: > >> On Tue, Jan 5, 2016 at 3:39 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > >> > On Tue, 2016-01-05 at 15:34 +0100, Jacob Siverskog wrote: > >> >> On Tue, Jan 5, 2016 at 3:14 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > >> > > >> >> > > >> >> > You might build a kernel with KASAN support to get maybe more chances to > >> >> > trigger the bug. > >> >> > > >> >> > ( https://www.kernel.org/doc/Documentation/kasan.txt ) > >> >> > > >> >> > >> >> Ah. Doesn't seem to be supported on arm(32) unfortunately. > >> > > >> > Then you could at least use standard debugging features : > >> > > >> > CONFIG_SLAB=y > >> > CONFIG_SLABINFO=y > >> > CONFIG_DEBUG_SLAB=y > >> > CONFIG_DEBUG_SLAB_LEAK=y > >> > > >> > (Or equivalent SLUB options) > >> > > >> > and > >> > > >> > CONFIG_DEBUG_PAGEALLOC=y > >> > > >> > (If arm(32) has CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y) > >> > >> I tried with those enabled and while toggling power on the Bluetooth > >> interface I usually get this after a few iterations: > >> kernel: Bluetooth: Unable to push skb to HCI core(-6) > > > > Well, this code seems to be quite buggy. > > > > I do not have time to audit it, but 5 minutes are enough to spot 2 > > issues. > > > > skb, once given to another queue/layer should not be accessed anymore. > > > > Ok. Unfortunately I still see the slab corruption even with your changes. Patch was only showing potential _reads_ after free, which do not generally corrupt memory. As I said, a full audit is needed, and I don't have time for this.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web