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]

Message-ID: <alpine.LFD.2.11.1507021135180.2615@ja.home.ssi.bg>
Date:	Thu, 2 Jul 2015 11:44:54 +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 Wed, 1 Jul 2015, Alex Gartrell wrote:

> On Wed, Jul 1, 2015 at 4:26 PM, Eric Dumazet <edumazet@...gle.com> wrote:
> > I think you are mistaken Alex.
> 
> Indeed, I was!  Should be unsurpising.
> 
> >
> > socket early demux cannot possibly set skb->destructor to sock_rfree()
> 
> Yeah I will admit adding the code to sock_rfree reflexively out of paranoia.
> 
> > If skb->destructor is set by early demux, it correctly points to sock_edemux()
> >
> > And this one correctly handles all socket variants.
> 
> Yes, the problem appears to be in ip_vs_prepare_tunneled_skb
> (ip_vs_xmit.c:859 in 4.0)
> 
>         if (skb_headroom(skb) < max_headroom || skb_cloned(skb)) {
>                 new_skb = skb_realloc_headroom(skb, max_headroom);
>                 if (!new_skb)
>                         goto error;
>                 if (skb->sk)
>                         skb_set_owner_w(new_skb, skb->sk);
>                 consume_skb(skb);
>                 skb = new_skb;
>         }
> 
> skb_set_owner_w sets sock_wfree.
> 
> I'll figure out how to ensure that we're using an appropriate destructor here.

	Alex, in our discussion on January I thought
we can skip calling skb_orphan for some cases but as
input and output path use different skb->destructor
we should call skb_orphan for every method, in every
case when skb->dev != NULL, even when we do not call
LOCAL_OUT, i.e. when NF_ACCEPT is returned for traffic
to local real server. We should not call it only for
local socket (skb->dev == NULL).

	I think, your patch from January is almost
good:

http://archive.linuxvirtualserver.org/html/lvs-devel/2015-01/msg00014.html

	Just add skb->dev check and we should be fine.
And the patch from Eric for IPVS looks good too.

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