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
