From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 14 Sep 2010 15:21:47 -0300 From: "Gustavo F. Padovan" To: Emeltchenko Andrei Cc: linux-bluetooth@vger.kernel.org Subject: Re: [PATCHv3 3/3] Bluetooth: check L2CAP length in first ACL fragment Message-ID: <20100914182147.GB5398@vigoh> References: <1284018181-29928-1-git-send-email-Andrei.Emeltchenko.news@gmail.com> <1284018181-29928-4-git-send-email-Andrei.Emeltchenko.news@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <1284018181-29928-4-git-send-email-Andrei.Emeltchenko.news@gmail.com> Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi Andrei, * Emeltchenko Andrei [2010-09-09 10:43:01 +0300]: > From: Andrei Emeltchenko > > Current Bluetooth code assembles fragments of big L2CAP packets > in l2cap_recv_acldata and then checks allowed L2CAP size in > assemled L2CAP packet (pi->imtu < skb->len). > > The patch moves allowed L2CAP size check to the early stage when > we receive the first fragment of L2CAP packet. We do not need to > reserve and keep L2CAP fragments for bad packets. > > Updated version after comments from Mat Martineau > > Trace below is received when using stress tools sending big > fragmented L2CAP packets. > ... > [ 1712.798492] swapper: page allocation failure. order:4, mode:0x4020 > [ 1712.804809] [] (unwind_backtrace+0x0/0xdc) from [] > (__alloc_pages_nodemask+0x4) > [ 1712.814666] [] (__alloc_pages_nodemask+0x47c/0x4d4) from > [] (__get_free_pages+) > [ 1712.824645] [] (__get_free_pages+0x10/0x3c) from [] > (__alloc_skb+0x4c/0xfc) > [ 1712.833465] [] (__alloc_skb+0x4c/0xfc) from [] > (l2cap_recv_acldata+0xf0/0x1f8 ) > [ 1712.843322] [] (l2cap_recv_acldata+0xf0/0x1f8 [l2cap]) from > [] (hci_rx_task+0x) > ... > > Signed-off-by: Andrei Emeltchenko > --- > net/bluetooth/l2cap.c | 17 +++++++++++++++++ > 1 files changed, 17 insertions(+), 0 deletions(-) > > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c > index ce8f5e4..c95fbd8 100644 > --- a/net/bluetooth/l2cap.c > +++ b/net/bluetooth/l2cap.c > @@ -4654,6 +4654,8 @@ static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 fl > > if (flags & ACL_START) { > struct l2cap_hdr *hdr; > + struct sock *sk; > + u16 cid; > int len; > > if (conn->rx_len) { > @@ -4673,6 +4675,7 @@ static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 fl > > hdr = (struct l2cap_hdr *) skb->data; > len = __le16_to_cpu(hdr->len) + L2CAP_HDR_SIZE; > + cid = __le16_to_cpu(hdr->cid); > > if (len == skb->len) { > /* Complete frame received */ > @@ -4689,6 +4692,20 @@ static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 fl > goto drop; > } > > + sk = l2cap_get_chan_by_scid(&conn->chan_list, cid); > + > + if (sk && l2cap_pi(sk)->imtu < len) { > + BT_ERR("Frame exceeding recv MTU (len %d, MTU %d)", > + len, l2cap_pi(sk)->imtu); I think you have to check if the imtu is less than (len - L2CAP_HDR_SIZE) because we don't count the header in the MTU. > + conn->rx_len = 0; /* needed? */ It's not needed. rx_len is always zero here. -- Gustavo F. Padovan ProFUSION embedded systems - http://profusion.mobi