netdev - Re: [PATCH net-next] net: bail on sock_wfree, sock_rfree when we have a TCP

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:	Fri, 3 Jul 2015 11:32:39 +0300 (EEST)
From:	Julian Anastasov <ja@....bg>
To:	Alex Gartrell <alexgartrell@...il.com>
cc:	Eric Dumazet <edumazet@...gle.com>,
	David Miller <davem@...emloft.net>,
	"agartrell@...com" <agartrell@...com>,
	netdev <netdev@...r.kernel.org>, kernel-team <kernel-team@...com>
Subject: Re: [PATCH net-next] net: bail on sock_wfree, sock_rfree when we
 have a TCP_TIMEWAIT sk


	Hello,

On Thu, 2 Jul 2015, Alex Gartrell wrote:

> On Thu, Jul 2, 2015 at 2:18 PM, Alex Gartrell <alexgartrell@...il.com> wrote:
> > If early demux was enabled, we'd use the route from the socket
> 
> Actually now that I think about it, this is probably broken, because
> we don't reply to the packet but instead silently drop it.

	I think, the problem is that input packet takes
the output path, not a problem with its sk_state.

	Here is how I understand the situation:

Input packet:

ip_rcv_finish:
	- early_demux:
		- attach skb->sk, skb->destructor, skb->dst
		- skb size is accounted later to sk

LOCAL_IN:
	- IPVS Remote Client mode: ip_vs_in
	- Case 1: IPVS forward to local real server (NF_ACCEPT case):
		- we are going to hit local server, so we should keep
		skb->sk, etc
		- skb->dst not changed
		- return NF_ACCEPT
		- continue to TCP stack
	- Case 2: IPVS forward TUN for example:
		- skb_orphan
		- we should not work with any skb->sk from input path
		- attach new skb->dst for the new real server
		- ip_local_out

TCP:
	- if IPVS does not grab the packet above, we have
	a call to skb_set_owner_r to account the skb size to sk


Locally generated TCP packet:

TCP tcp_transmit_skb:
	- attach skb->sk, skb->destructor, increase sk_wmem_alloc

LOCAL_OUT:
	- IPVS Local Client mode: again ip_vs_in
	- skb->dev is NULL means locally generated packet, skb->sk can
	be set by TCP
	- Case 1: IPVS forward to local real server (NF_ACCEPT case):
		- we are going to hit local server, so we should keep
		skb->sk, etc
		- skb->dst not changed
		- return NF_ACCEPT
		- continue to TCP stack in LOCAL_IN
	- Case 2: IPVS forward TUN for example:
		- no skb_orphan because skb->sk is for
		output path on skb->dev == NULL
		- realloc headroom (call skb_set_owner_w): should happen
		only for skb->sk from output path. This code in
		ip_vs_prepare_tunneled_skb comes from old days from
		ipip.c where skb->sk is present for locally generated
		packets and IPIP's xmit routine reallocs headroom
		- attach new skb->dst for the new real server
		- ip_local_out

To summarize:
	- we should call skb_orphan as soon as possible after
	deciding if packets goes to local or remote real server
	but only for skb->sk set by early_demux, not for packets
	sent by TCP
	- if packets go to local server IPVS should not touch
	skb->dst, skb->sk, etc (NF_ACCEPT case)
	- for skb->sk set by early_demux, skb_orphan should happen before
	skb_set_owner_w in ip_vs_prepare_tunneled_skb because
	skb_set_owner_w will try to increase sk_wmem_alloc which is
	wrong for early_demux phase
	- reaching skb_set_owner_w code for skb->sk set by eraly_demux
	looks wrong to me, it can happen on:
		- redirect (DNAT), if somehow we have socket too
		- IPVS redirect: if we forward both to local and remote
		real servers
		- not likely for forward, nobody forwards traffic
		destined to local IP to remote host

Regards

--
Julian Anastasov <ja@....bg>
--
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