Re: [PATCH] [net]ip_tunnel: the lack of vti_link_ops' dellink() cause kernel panic

[Nicolas Dichtel <nicolas.dichtel@6wind.com>]

Le 18/11/2014 11:37, Xin Long a écrit :
> Now the vti_link_ops do not point the .dellink, rtnl_newlink will invoke
> the default function, unregister_netdevice_queue, which will cause the
> dev to unregister later. so when we delete a vti device, the net_device
> will be removed, but the tunnel still in the tunnel list. then, we add
> a new vti, in ip_tunnel_find():
>
>          hlist_for_each_entry_rcu(t, head, hash_node) {
>                  if (local == t->parms.iph.saddr &&
>                      remote == t->parms.iph.daddr &&
>                      link == t->parms.link &&
>                      type == t->dev->type &&
>                      ip_tunnel_key_match(&t->parms, flags, key))
>                          break;
>          }
>
> the dev of ip_tunnel *t may be null because of unregister_netdevice_queue
> motioned above. so the panic will happen:
>
> [ 3835.072977] IP: [<ffffffffa04103fd>] ip_tunnel_find+0x9d/0xc0 [ip_tunnel]
> [ 3835.073008] PGD b2c21067 PUD b7277067 PMD 0
> [ 3835.073008] Oops: 0000 [#1] SMP
> .....
> [ 3835.073008] Stack:
> [ 3835.073008]  ffff8800b72d77f0 ffffffffa0411924 ffff8800bb956000 ffff8800b72d78e0
> [ 3835.073008]  ffff8800b72d78a0 0000000000000000 ffffffffa040d100 ffff8800b72d7858
> [ 3835.073008]  ffffffffa040b2e3 0000000000000000 0000000000000000 0000000000000000
> [ 3835.073008] Call Trace:
> [ 3835.073008]  [<ffffffffa0411924>] ip_tunnel_newlink+0x64/0x160 [ip_tunnel]
> [ 3835.073008]  [<ffffffffa040b2e3>] vti_newlink+0x43/0x70 [ip_vti]
> [ 3835.073008]  [<ffffffff8150d4da>] rtnl_newlink+0x4fa/0x5f0
> [ 3835.073008]  [<ffffffff812f68bb>] ? nla_strlcpy+0x5b/0x70
> [ 3835.073008]  [<ffffffff81508fb0>] ? rtnl_link_ops_get+0x40/0x60
> [ 3835.073008]  [<ffffffff8150d11f>] ? rtnl_newlink+0x13f/0x5f0
> [ 3835.073008]  [<ffffffff81509cf4>] rtnetlink_rcv_msg+0xa4/0x270
> [ 3835.073008]  [<ffffffff8126adf5>] ? sock_has_perm+0x75/0x90
> [ 3835.073008]  [<ffffffff81509c50>] ? rtnetlink_rcv+0x30/0x30
> [ 3835.073008]  [<ffffffff81529e39>] netlink_rcv_skb+0xa9/0xc0
> [ 3835.073008]  [<ffffffff81509c48>] rtnetlink_rcv+0x28/0x30
> ....
>
> the reproduction can be like this:
>
> modprobe ip_vti
> ip link del ip_vti0 type vti
> ip link add ip_vti0 type vti
> rmmod ip_vti
>
> do that one or more time, kernel will panic.
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
A quick look at the ipv6 side seems to show that there is the same problem. Can
you provide the IPv6 patch too?

Note also that the maintainer of this module is Steffen Klassert, please don't
forget to CC him.

> ---
>   net/ipv4/ip_vti.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
> index 3e86101..1a7e979 100644
> --- a/net/ipv4/ip_vti.c
> +++ b/net/ipv4/ip_vti.c
> @@ -528,6 +528,7 @@ static struct rtnl_link_ops vti_link_ops __read_mostly = {
>   	.validate	= vti_tunnel_validate,
>   	.newlink	= vti_newlink,
>   	.changelink	= vti_changelink,
> +	.dellink        = ip_tunnel_dellink,
Nitpicking: other lines into this struct uses tabs to align the '=', but
the one you add uses spaces.


Thank you,
Nicolas