Return-Path: MIME-Version: 1.0 In-Reply-To: References: <20160726000523.11189-1-mathew.j.martineau@linux.intel.com> From: Willem de Bruijn Date: Tue, 26 Jul 2016 14:54:34 -0400 Message-ID: Subject: Re: [PATCH v2] bluetooth, bpf: split sk_filter in l2cap_sock_recv_cb To: Mat Martineau Cc: linux-bluetooth@vger.kernel.org, Daniel Borkmann , Marcel Holtmann , Gustavo Padovan , Alexei Starovoitov Content-Type: text/plain; charset=UTF-8 List-ID: On Tue, Jul 26, 2016 at 2:48 PM, Mat Martineau wrote: > > On Tue, 26 Jul 2016, Willem de Bruijn wrote: > >> On Tue, Jul 26, 2016 at 1:00 PM, Mat Martineau >> wrote: >>> >>> >>> On Tue, 26 Jul 2016, Willem de Bruijn wrote: >>> >>>> sk_filter can also accept, but trim, packets. If the protocol expects >>>> a header that it unconditionally pulls later, use sk_filter_trim to >>> >>> >>> >>> sk_filter_trim_cap? I see that you added that recently. It's not in >>> bluetooth-next and we're aiming for a patch that can be easily backported >>> to >>> stable. >>> >>>> avoid trimming to below header length. I think I saw a path from >>>> >>>> l2cap_data_rcv >>>> l2cap_rx >>>> l2cap_rx_state_recv >>>> l2cap_reassemble_sdu >>>> case L2CAP_SAR_START >>>> skb_pull(skb, L2CAP_SDULEN_SIZE) >>> >>> >>> >>> How about checking skb->len before that skb_pull instead? >> >> >> That works, preferably using pskb_may_pull. The only downside of doing >> that exactly in this path, is that it is not easy to verify that all >> other code paths downstream from sk_filter are also safe. If all >> expect this header, the check is best added right after filter. > > > The SDULEN header is only present for SAR_START frames (handled on this code > path), and there is no other manipulation of skb data/tail/len after > sk_filter. Then this is great. > We know these skbs are linear, or at least they were before > sk_filter was called. Can sk_filter fragment an otherwise linear skb? (Looks > like the answer is "no" but I'd like to confirm) No. It can call ___pskb_trim, but that only happens for non-linear skbs.