Re: PROBLEM: System call 'sendmsg' of process ospfd (quagga) causes kernel oops
From: David Miller <davem@davemloft.net>
Date: 2011-10-19 07:09:29
Also in:
lkml
Subsystem:
networking drivers, networking [general], networking [sockets], the rest · Maintainers:
Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima, Willem de Bruijn, Linus Torvalds
From: Herbert Xu <redacted> Date: Tue, 18 Oct 2011 15:45:37 +0200
On Tue, Oct 18, 2011 at 02:56:00PM +0200, Eric Dumazet wrote:quoted
I am ok by this way, but we might hit another similar problem elsewhere. (igmp.c ip6_output, ...) We effectively want to remove LL_ALLOCATED_SPACE() usage and obfuscate code...Here's another idea, provide a helper to do the skb allocation and the skb_reserve in one go. That way this ugliness would only need to be done once.
Someone please test this: -------------------- net: Fix crashes on devices which dynamically change needed headroom. One such device is IP_GRE. The problem is that we evaluate the device characteristics twice, once to determine the allocation size, and once to do the skb_reserve(). Combine these into one operation using a helper function. With help from Eric Dumazet and Herbert Xu. Reported-by: Reported-by: Elmar Vonlanthen <redacted> Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ddee79b..d4abb400 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h@@ -276,12 +276,22 @@ struct hh_cache { * LL_ALLOCATED_SPACE also takes into account the tailroom the device * may need. */ +#define LL_ALIGN(__len) (((__len)&~(HH_DATA_MOD - 1)) + HH_DATA_MOD) + +#define __LL_RESERVED_SPACE(__hard_hdr_len, __needed_headroom) \ + LL_ALIGN((__hard_hdr_len) + (__needed_headroom)) + #define LL_RESERVED_SPACE(dev) \ - ((((dev)->hard_header_len+(dev)->needed_headroom)&~(HH_DATA_MOD - 1)) + HH_DATA_MOD) + __LL_RESERVED_SPACE((dev)->hard_header_len, (dev)->needed_headroom)) + #define LL_RESERVED_SPACE_EXTRA(dev,extra) \ - ((((dev)->hard_header_len+(dev)->needed_headroom+(extra))&~(HH_DATA_MOD - 1)) + HH_DATA_MOD) + LL_ALIGN((dev)->hard_header_len + (dev)->needed_headroom + (extra)) + +#define __LL_ALLOCATED_SPACE(__hard_hdr_len, __needed_headroom, __needed_tailroom) \ + LL_ALIGN((__hard_hdr_len) + (__needed_headroom) + (__needed_tailroom)) + #define LL_ALLOCATED_SPACE(dev) \ - ((((dev)->hard_header_len+(dev)->needed_headroom+(dev)->needed_tailroom)&~(HH_DATA_MOD - 1)) + HH_DATA_MOD) + __LL_ALLOCATED_SPACE((dev)->hard_header_len, (dev)->needed_headroom, (dev)->needed_tailroom) struct header_ops { int (*create) (struct sk_buff *skb, struct net_device *dev,
diff --git a/include/net/sock.h b/include/net/sock.h
index 8e4062f..9d96995 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h@@ -1103,6 +1103,11 @@ extern struct sk_buff *sock_alloc_send_skb(struct sock *sk, unsigned long size, int noblock, int *errcode); +extern struct sk_buff *sock_alloc_send_skb_reserve(struct sock *sk, + struct net_device *dev, + unsigned long base_size, + int noblock, + int *errcode); extern struct sk_buff *sock_alloc_send_pskb(struct sock *sk, unsigned long header_len, unsigned long data_len,
diff --git a/net/core/sock.c b/net/core/sock.c
index bc745d0..9cefb72 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c@@ -1586,6 +1586,26 @@ struct sk_buff *sock_alloc_send_skb(struct sock *sk, unsigned long size, } EXPORT_SYMBOL(sock_alloc_send_skb); +struct sk_buff *sock_alloc_send_skb_reserve(struct sock *sk, struct net_device *dev, + unsigned long base_size, int noblock, + int *errcode) +{ + unsigned int hh_len = dev->hard_header_len; + unsigned int needed_hroom = dev->needed_headroom; + unsigned int needed_troom = dev->needed_tailroom; + unsigned int reserve, lls; + struct sk_buff *skb; + + lls = __LL_ALLOCATED_SPACE(hh_len, needed_hroom, needed_troom); + skb = sock_alloc_send_skb(sk, (base_size + lls + 15), noblock, errcode); + if (skb) { + reserve = __LL_RESERVED_SPACE(hh_len, needed_hroom); + skb_reserve(skb, reserve); + } + return skb; +} +EXPORT_SYMBOL(sock_alloc_send_skb_reserve); + static void __lock_sock(struct sock *sk) __releases(&sk->sk_lock.slock) __acquires(&sk->sk_lock.slock)
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 61714bd..dcecb97 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c@@ -335,12 +335,11 @@ static int raw_send_hdrinc(struct sock *sk, struct flowi4 *fl4, if (flags&MSG_PROBE) goto out; - skb = sock_alloc_send_skb(sk, - length + LL_ALLOCATED_SPACE(rt->dst.dev) + 15, - flags & MSG_DONTWAIT, &err); + skb = sock_alloc_send_skb_reserve(sk, rt->dst.dev, length, + flags & MSG_DONTWAIT, + &err); if (skb == NULL) goto error; - skb_reserve(skb, LL_RESERVED_SPACE(rt->dst.dev)); skb->priority = sk->sk_priority; skb->mark = sk->sk_mark;
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 343852e..5a8e141 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c@@ -618,12 +618,11 @@ static int rawv6_send_hdrinc(struct sock *sk, void *from, int length, if (flags&MSG_PROBE) goto out; - skb = sock_alloc_send_skb(sk, - length + LL_ALLOCATED_SPACE(rt->dst.dev) + 15, - flags & MSG_DONTWAIT, &err); + skb = sock_alloc_send_skb_reserve(sk, rt->dst.dev, length, + flags & MSG_DONTWAIT, + &err); if (skb == NULL) goto error; - skb_reserve(skb, LL_RESERVED_SPACE(rt->dst.dev)); skb->priority = sk->sk_priority; skb->mark = sk->sk_mark;