diff mbox

ip_tunnel:multicast process cause panic due to skb->_skb_refdst NULL pointer

Message ID 1393216519-9066-1-git-send-email-lucien.xin@gmail.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Xin Long Feb. 24, 2014, 4:35 a.m. UTC
when ip_tunnel process multicast packets, it may check if the packet is looped
back packet though 'rt_is_output_route(skb_rtable(skb))' in ip_tunnel_rcv(),
but before that , skb->_skb_refdst has been dropped in iptunnel_pull_header(),
so which leads to a panic.

fix the bug: https://bugzilla.kernel.org/show_bug.cgi?id=70681

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/ipv4/ip_tunnel_core.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Hannes Frederic Sowa Feb. 24, 2014, 2:58 p.m. UTC | #1
On Mon, Feb 24, 2014 at 12:35:19PM +0800, Xin Long wrote:
> when ip_tunnel process multicast packets, it may check if the packet is looped
> back packet though 'rt_is_output_route(skb_rtable(skb))' in ip_tunnel_rcv(),
> but before that , skb->_skb_refdst has been dropped in iptunnel_pull_header(),
> so which leads to a panic.
> 
> fix the bug: https://bugzilla.kernel.org/show_bug.cgi?id=70681

If the packet is locally forwarded we can test for IPCB(skb)->flags &
IPSKB_FORWARDED.

I am not sure if we actually have the correct dst at that point.

Greetings,

  Hannes

--
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
Xin Long Feb. 28, 2014, 1:27 a.m. UTC | #2
On Mon, Feb 24, 2014 at 10:58 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
>
> If the packet is locally forwarded we can test for IPCB(skb)->flags &
> IPSKB_FORWARDED.
>

thanks for  your reply,  hannes

IPSKB_FORWARDED is locally forwarded flag, but in ip_tunnel_rcv(), it
check if the packet
is a *looped back packet* (not about locally forwarded), the path is
__ip_route_output_key()-->
__mkroute_output()(rth->dst.output = ip_mc_output), then,
ip_mc_output()-->dev_loopback_xmit(),
 which happen when inet_sk(sk)->mc_loop =1, and in __mkroute_output(),
rth->rt_is_input = 0.
 In that path, IPCB(skb)->flags &IPSKB_FORWARDED always 0.

so I think 'IPCB(skb)->flags &IPSKB_FORWARDED' cannot meet the goal.
--
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
Hannes Frederic Sowa March 1, 2014, 5:33 p.m. UTC | #3
On Fri, Feb 28, 2014 at 09:27:35AM +0800, lucien xin wrote:
> On Mon, Feb 24, 2014 at 10:58 PM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> >
> > If the packet is locally forwarded we can test for IPCB(skb)->flags &
> > IPSKB_FORWARDED.
> >
> 
> thanks for  your reply,  hannes
> 
> IPSKB_FORWARDED is locally forwarded flag, but in ip_tunnel_rcv(), it
> check if the packet
> is a *looped back packet* (not about locally forwarded), the path is
> __ip_route_output_key()-->
> __mkroute_output()(rth->dst.output = ip_mc_output), then,
> ip_mc_output()-->dev_loopback_xmit(),
>  which happen when inet_sk(sk)->mc_loop =1, and in __mkroute_output(),
> rth->rt_is_input = 0.
>  In that path, IPCB(skb)->flags &IPSKB_FORWARDED always 0.
> 
> so I think 'IPCB(skb)->flags &IPSKB_FORWARDED' cannot meet the goal.

Ok, I see, sorry for the wrong hint, you're absolutely correct.

It seems to me that your original proposal is fine for me.

You would have to repost so it gets back into patchworks.

Thanks,

  Hannes

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

Patch

diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index 6156f4e..88b08aa 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -108,7 +108,6 @@  int iptunnel_pull_header(struct sk_buff *skb, int hdr_len, __be16 inner_proto)
 	nf_reset(skb);
 	secpath_reset(skb);
 	skb_clear_hash_if_not_l4(skb);
-	skb_dst_drop(skb);
 	skb->vlan_tci = 0;
 	skb_set_queue_mapping(skb, 0);
 	skb->pkt_type = PACKET_HOST;