From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dominique Martinet Subject: Re: KCM - recvmsg() mangles packets? Date: Mon, 6 Aug 2018 01:39:21 +0200 Message-ID: <20180805233921.GA5773@nautica> References: <20180803182830.GB29193@nautica> <20180803232057.GB7583@nautica> <20180804014132.GA26606@nautica> <20180804020806.GA32338@nautica> <20180805064410.GA26807@nautica> <20180805141253.GA22780@nautica> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: Linux Kernel Network Developers To: Tom Herbert Return-path: Received: from nautica.notk.org ([91.121.71.147]:37570 "EHLO nautica.notk.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726673AbeHFBp7 (ORCPT ); Sun, 5 Aug 2018 21:45:59 -0400 Content-Disposition: inline In-Reply-To: <20180805141253.GA22780@nautica> Sender: netdev-owner@vger.kernel.org List-ID: Dominique Martinet wrote on Sun, Aug 05, 2018: > It's getting late but I'll try adding a pskb_pull in there tomorrow, it > would be better to make the bpf program start with an offset but I don't > think that'll be easy to change... I can confirm the following patch fixes the issue for me: -----8<--------------------- diff --git a/net/strparser/strparser.c b/net/strparser/strparser.c index 625acb27efcc..348ff5945591 100644 --- a/net/strparser/strparser.c +++ b/net/strparser/strparser.c @@ -222,6 +222,16 @@ static int __strp_recv(read_descriptor_t *desc, struct sk_buff *orig_skb, if (!stm->strp.full_len) { ssize_t len; + /* Can only parse if there is no offset */ + if (unlikely(stm->strp.offset)) { + if (!pskb_pull(skb, stm->strp.offset)) { + STRP_STATS_INCR(strp->stats.mem_fail); + strp_parser_err(strp, -ENOMEM, desc); + break; + } + stm->strp.offset = 0; + } + len = (*strp->cb.parse_msg)(strp, head); if (!len) { ----------------8<---------------------- Now, I was looking at other users of strparser (I see sockmap, kcm and tls) and it looks like sockmap does not handle offsets either but tls does by using skb_copy_bits -- they're copying the tls header to a buffer on the stack. kcm cannot do that because we do not know how much data the user expects to read, and I'm not comfortable doing pskb_pull in the kcm callback either, but the cost of this pull is probably non-negligible if some user can make do without it... On the other hand, I do not see how to make the bpf program handle an offset in the skb as that offset is strparser-specific. Maybe add a flag in the cb that specifies wether the callback allows non-zero offset? I'll let you see if you can reproduce this and will wait for advices on how to solve this properly so we can work on a proper fix. Thanks, -- Dominique