Commit 072222b4 authored Sep 11, 2018 by Dominique Martinet Committed by David S. Miller Sep 17, 2018

kcm: remove any offset before parsing messages



The current code assumes kcm users know they need to look for the
strparser offset within their bpf program, which is not documented
anywhere and examples laying around do not do.

The actual recv function does handle the offset well, so we can create a
temporary clone of the skb and pull that one up as required for parsing.

The pull itself has a cost if we are pulling beyond the head data,
measured to 2-3% latency in a noisy VM with a local client stressing
that path. The clone's impact seemed too small to measure.

This bug can be exhibited easily by implementing a "trivial" kcm parser
taking the first bytes as size, and on the client sending at least two
such packets in a single write().

Note that bpf sockmap has the same problem, both for parse and for recv,
so it would pulling twice or a real pull within the strparser logic if
anyone cares about that.

Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

parent c7348091

net/kcm/kcmsock.c

+25 −1

Original line number	Diff line number	Diff line
		@@ -381,8 +381,32 @@ static int kcm_parse_func_strparser(struct strparser strp, struct sk_buff skb)
		{
		struct kcm_psock *psock = container_of(strp, struct kcm_psock, strp);
		struct bpf_prog *prog = psock->bpf_prog;
		struct sk_buff *clone_skb = NULL;
		struct strp_msg *stm;
		int rc;

		stm = strp_msg(skb);
		if (stm->offset) {
		skb = clone_skb = skb_clone(skb, GFP_ATOMIC);
		if (!clone_skb)
		return -ENOMEM;

		if (!pskb_pull(clone_skb, stm->offset)) {
		rc = -ENOMEM;
		goto out;
		}

		/* reset cloned skb's offset for bpf programs using it */
		stm = strp_msg(clone_skb);
		stm->offset = 0;
		}

		rc = (*prog->bpf_func)(skb, prog->insnsi);
		out:
		if (clone_skb)
		kfree_skb(clone_skb);

		return (*prog->bpf_func)(skb, prog->insnsi);
		return rc;
		}

		static int kcm_read_sock_done(struct strparser *strp, int err)

Admin message