netdev - [RFC] net: skb_head_is_locked() should use skb_header

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: <1337666034.3361.50.camel@edumazet-glaptop>
Date:	Tue, 22 May 2012 07:53:54 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	David Miller <davem@...emloft.net>, alexander.h.duyck@...el.com
Cc:	netdev@...r.kernel.org
Subject: [RFC] net: skb_head_is_locked() should use skb_header_cloned()

Hi David and Alexander

There is no hurry since net-next is closed, but I hit the following
problem :

When IPv6 conntracking is enabled, code from
net/ipv6/netfilter/nf_conntrack_reasm.c does a cloning of all skbs to
build a shadow.

Then we run : (skb here is the head of the 'shadow skb' )

void nf_ct_frag6_output(unsigned int hooknum, struct sk_buff *skb,
                        struct net_device *in, struct net_device *out,
                        int (*okfn)(struct sk_buff *))
{
        struct sk_buff *s, *s2;

        for (s = NFCT_FRAG6_CB(skb)->orig; s;) {
                nf_conntrack_put_reasm(s->nfct_reasm);
                nf_conntrack_get_reasm(skb);
                s->nfct_reasm = skb;

                s2 = s->next;
                s->next = NULL;

                NF_HOOK_THRESH(NFPROTO_IPV6, hooknum, s, in, out, okfn,
                               NF_IP6_PRI_CONNTRACK_DEFRAG + 1);
                s = s2;
        }
        nf_conntrack_put_reasm(skb);
}

So when all original skbs are fed to real IPv6 reassembly code, their
clones are still alive and we hit the condition in skb_try_coalesce() :

if (skb_head_is_locked(from))
	return false;

I was wondering if skb_head_is_locked() should be changed to :

if (!skb->head_frag || skb_header_cloned(skb))
	return false;

Then we could add skb_header_release() calls on the clones of course in
net/ipv6/netfilter/nf_conntrack_reasm.c 

Not-Yet-Signed-off-by: Eric Dumazet <edumazet@...gle.com>
---
 include/linux/skbuff.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 0e50171..6509ee1 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2587,7 +2587,7 @@ static inline bool skb_is_recycleable(const struct sk_buff *skb, int skb_size)
  */
 static inline bool skb_head_is_locked(const struct sk_buff *skb)
 {
-	return !skb->head_frag || skb_cloned(skb);
+	return !skb->head_frag || skb_header_cloned(skb);
 }
 #endif	/* __KERNEL__ */
 #endif	/* _LINUX_SKBUFF_H */


--
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