netdev - Re: [PATCH v2 net-next 2/2] tc: make ingress and egress qdiscs consistent

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 07 Apr 2015 22:35:49 -0400 (EDT)
From:	David Miller <davem@...emloft.net>
To:	ast@...mgrid.com
Cc:	daniel@...earbox.net, jiri@...nulli.us, jhs@...atatu.com,
	netdev@...r.kernel.org
Subject: Re: [PATCH v2 net-next 2/2] tc: make ingress and egress qdiscs
 consistent

From: Alexei Starovoitov <ast@...mgrid.com>
Date: Tue,  7 Apr 2015 18:03:45 -0700

> +
> +	/* don't recompute skb->csum back and forth while pushing/pulling L2 */
> +	__skb_push(skb, hard_header_len);
>  	result = tc_classify(skb, fl, &res);
> +	__skb_pull(skb, hard_header_len);

This is not legal.

This SKB can be referenced by other entities, such as AF_PACKET
sockets and other network taps on input.  You absolutely do not
have private access to this SKB object, and any modification you
make to it will be seen by others.

Therefore you cannot push and pull the headers, because that
modification will be seen by the other entities referencing this SKB.

This is why every network protocol's input path must do something
like:

	skb = skb_share_check(skb, GFP_ATOMIC);

before pulling headers.

And you do not want to add this expensive operation here.

That would be really stupid overhead just to accomodate BPF things.

Instead just propagate some offset into the locations that care about
the skb data location.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html