netdev
[Top] [All Lists]

Re: [PATCH] Conntrack leak with raw sockets

To: Phil Oester <kernel@xxxxxxxxxxxx>
Subject: Re: [PATCH] Conntrack leak with raw sockets
From: Patrick McHardy <kaber@xxxxxxxxx>
Date: Fri, 25 Mar 2005 21:37:01 +0100
Cc: netdev@xxxxxxxxxxx, netfilter-devel@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20050325201127.GA3400@linuxace.com>
References: <20050325201127.GA3400@linuxace.com>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.7.6) Gecko/20050324 Debian/1.7.6-1
Phil Oester wrote:
In the event a raw socket is created for sending purposes only, the creator
never bothers to check the socket's receive queue.  But we continue to
add skbs to its queue until it fills up.

Unfortunately, if ip_conntrack is loaded on the box, each skb we add to the
queue potentially holds a reference to a conntrack.  If the user attempts
to unload ip_conntrack, we will spin around forever since the queued skbs
are pinned.

Great work tracking this down. But I fear the problem will come back haunt us with this patch. The are more places where a packet can be queued indefinitely, for example stopped qdiscs. IMO the best fix is to drop the conntrack reference once the packet leaves IP, so we don't have to make any assumptions about what will happen to the packet - this would be in ip_finish_output2(). Could you send a patch that does this? While you're at it, you could also remove this part from ip_conntrack_standalone:

#if !defined(CONFIG_IP_NF_NAT) && !defined(CONFIG_IP_NF_NAT_MODULE)
        /* Previously seen (loopback)?  Ignore.  Do this before
           fragment check. */
        if ((*pskb)->nfct)
                return NF_ACCEPT;
#endif

Regards
Patrick

<Prev in Thread] Current Thread [Next in Thread>