| Message ID | 1330007786.15610.26.camel@edumazet-laptop |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
Le jeudi 23 février 2012 à 15:36 +0100, Eric Dumazet a écrit : > [PATCH] ipsec: be careful of non existing mac headers > > Nicollo Belli reported ipsec crashes in case we handle a frame without > mac header (atm in his case) Oops sorry for your name being mangled in changelog, its Niccolò as correctly spelled in the "Reported-by" and "Tested-by" tags -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Il 23/02/2012 15:39, Eric Dumazet ha scritto: > Oops sorry for your name being mangled in changelog, its Niccolò as > correctly spelled in the "Reported-by" and "Tested-by" tags Don't worry and thanks for your help. I'm currently running the official patch you submitted for review. Niccolò -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Thu, 23 Feb 2012 15:36:26 +0100 > [PATCH] ipsec: be careful of non existing mac headers > > Nicollo Belli reported ipsec crashes in case we handle a frame without > mac header (atm in his case) > > Before copying mac header, better make sure it is present. > > Bugzilla reference: https://bugzilla.kernel.org/show_bug.cgi?id=42809 > > Reported-by: Niccolò Belli <darkbasic@linuxsystems.it> > Tested-by: Niccolò Belli <darkbasic@linuxsystems.it> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Three instances of the same piece of code, maybe a helper function is appropriate at that point? :-) You might even get ambitious and add a big comment to that helper function explaining the situation. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le jeudi 23 février 2012 à 15:11 -0500, David Miller a écrit : > Three instances of the same piece of code, maybe a helper function is > appropriate at that point? :-) You might even get ambitious and add a > big comment to that helper function explaining the situation. I did have this idea but refrained because this kind of things is harder to backport to stable kernels. I'll make a cleanup in net-next if you agree ? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Thu, 23 Feb 2012 21:17:23 +0100 > Le jeudi 23 février 2012 à 15:11 -0500, David Miller a écrit : >> Three instances of the same piece of code, maybe a helper function is >> appropriate at that point? :-) You might even get ambitious and add a >> big comment to that helper function explaining the situation. > > I did have this idea but refrained because this kind of things is harder > to backport to stable kernels. > > I'll make a cleanup in net-next if you agree ? I think the work to backport is equal whether you use a helper function or not. Heck, use an inline function so you don't have to worry about module exports or any details like that. Besides, I'm the one who likely has to backport this thing, so... :-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le jeudi 23 février 2012 à 15:23 -0500, David Miller a écrit : > I think the work to backport is equal whether you use a helper function > or not. Heck, use an inline function so you don't have to worry about > module exports or any details like that. > > Besides, I'm the one who likely has to backport this thing, so... :-) Great, I'll send an updated version in a couple of minutes ;) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/xfrm4_mode_beet.c b/net/ipv4/xfrm4_mode_beet.c index 6341818..d3451f6 100644 --- a/net/ipv4/xfrm4_mode_beet.c +++ b/net/ipv4/xfrm4_mode_beet.c @@ -111,10 +111,11 @@ static int xfrm4_beet_input(struct xfrm_state *x, struct sk_buff *skb) skb_push(skb, sizeof(*iph)); skb_reset_network_header(skb); - memmove(skb->data - skb->mac_len, skb_mac_header(skb), - skb->mac_len); - skb_set_mac_header(skb, -skb->mac_len); - + if (skb_mac_header_was_set(skb)) { + memmove(skb->data - skb->mac_len, skb_mac_header(skb), + skb->mac_len); + skb_set_mac_header(skb, -skb->mac_len); + } xfrm4_beet_make_header(skb); iph = ip_hdr(skb); diff --git a/net/ipv4/xfrm4_mode_tunnel.c b/net/ipv4/xfrm4_mode_tunnel.c index 534972e..a646f30 100644 --- a/net/ipv4/xfrm4_mode_tunnel.c +++ b/net/ipv4/xfrm4_mode_tunnel.c @@ -66,7 +66,6 @@ static int xfrm4_mode_tunnel_output(struct xfrm_state *x, struct sk_buff *skb) static int xfrm4_mode_tunnel_input(struct xfrm_state *x, struct sk_buff *skb) { - const unsigned char *old_mac; int err = -EINVAL; if (XFRM_MODE_SKB_CB(skb)->protocol != IPPROTO_IPIP) @@ -84,9 +83,12 @@ static int xfrm4_mode_tunnel_input(struct xfrm_state *x, struct sk_buff *skb) if (!(x->props.flags & XFRM_STATE_NOECN)) ipip_ecn_decapsulate(skb); - old_mac = skb_mac_header(skb); - skb_set_mac_header(skb, -skb->mac_len); - memmove(skb_mac_header(skb), old_mac, skb->mac_len); + if (skb_mac_header_was_set(skb)) { + const unsigned char *old_mac = skb_mac_header(skb); + + skb_set_mac_header(skb, -skb->mac_len); + memmove(skb_mac_header(skb), old_mac, skb->mac_len); + } skb_reset_network_header(skb); err = 0; diff --git a/net/ipv6/xfrm6_mode_beet.c b/net/ipv6/xfrm6_mode_beet.c index a81ce94..74c4b92 100644 --- a/net/ipv6/xfrm6_mode_beet.c +++ b/net/ipv6/xfrm6_mode_beet.c @@ -80,7 +80,6 @@ static int xfrm6_beet_output(struct xfrm_state *x, struct sk_buff *skb) static int xfrm6_beet_input(struct xfrm_state *x, struct sk_buff *skb) { struct ipv6hdr *ip6h; - const unsigned char *old_mac; int size = sizeof(struct ipv6hdr); int err; @@ -91,10 +90,12 @@ static int xfrm6_beet_input(struct xfrm_state *x, struct sk_buff *skb) __skb_push(skb, size); skb_reset_network_header(skb); - old_mac = skb_mac_header(skb); - skb_set_mac_header(skb, -skb->mac_len); - memmove(skb_mac_header(skb), old_mac, skb->mac_len); + if (skb_mac_header_was_set(skb)) { + const unsigned char *old_mac = skb_mac_header(skb); + skb_set_mac_header(skb, -skb->mac_len); + memmove(skb_mac_header(skb), old_mac, skb->mac_len); + } xfrm6_beet_make_header(skb); ip6h = ipv6_hdr(skb); diff --git a/net/ipv6/xfrm6_mode_tunnel.c b/net/ipv6/xfrm6_mode_tunnel.c index 261e6e6..edb7091 100644 --- a/net/ipv6/xfrm6_mode_tunnel.c +++ b/net/ipv6/xfrm6_mode_tunnel.c @@ -63,7 +63,6 @@ static int xfrm6_mode_tunnel_output(struct xfrm_state *x, struct sk_buff *skb) static int xfrm6_mode_tunnel_input(struct xfrm_state *x, struct sk_buff *skb) { int err = -EINVAL; - const unsigned char *old_mac; if (XFRM_MODE_SKB_CB(skb)->protocol != IPPROTO_IPV6) goto out; @@ -80,9 +79,12 @@ static int xfrm6_mode_tunnel_input(struct xfrm_state *x, struct sk_buff *skb) if (!(x->props.flags & XFRM_STATE_NOECN)) ipip6_ecn_decapsulate(skb); - old_mac = skb_mac_header(skb); - skb_set_mac_header(skb, -skb->mac_len); - memmove(skb_mac_header(skb), old_mac, skb->mac_len); + if (skb_mac_header_was_set(skb)) { + const unsigned char *old_mac = skb_mac_header(skb); + + skb_set_mac_header(skb, -skb->mac_len); + memmove(skb_mac_header(skb), old_mac, skb->mac_len); + } skb_reset_network_header(skb); err = 0;