diff mbox series

[ovs-dev] ip_tunnel: Fix bugs that could crash kernel

Message ID 1532109882-10881-1-git-send-email-pkusunyifeng@gmail.com
State Accepted
Headers show
Series [ovs-dev] ip_tunnel: Fix bugs that could crash kernel | expand

Commit Message

Yifeng Sun July 20, 2018, 6:04 p.m. UTC 
Without this patch, OVS kernel module can delete itn->fb_tunnel_dev
one more time than necessary, which causes kernel crash.

On kernel 4.4.0-116-generic, the crash can be reproduced by running
the simple test provided below through check-kernel.

  make & make modules_install
  rmmod ip_gre gre ip_tunnel
  modprobe openvswitch
  make check-kernel TESTSUITEFLAGS=x
  dmesg

Simple test:

AT_SETUP([datapath - crash test])                                              
OVS_CHECK_GRE()                                                                
ip link del gre0                                                               
OVS_TRAFFIC_VSWITCHD_START()                                                    
AT_CHECK([ovs-vsctl -- set bridge br0])                                        
ADD_BR([br-underlay], [set bridge br-underlay])                                
AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])                            
AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])                    
ADD_NAMESPACES(at_ns0)                                                         
ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24")                             
AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])                      
AT_CHECK([ip link set dev br-underlay up])                                     
ADD_OVS_TUNNEL([gre], [br0], [at_gre0], [172.31.1.1], [10.1.1.100/24])         
tcpdump -U -i br-underlay -w underlay.pcap &                                   
sleep 1                                                                         
OVS_TRAFFIC_VSWITCHD_STOP                                                      
AT_CLEANUP

Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
---
 datapath/linux/compat/ip_tunnel.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

William Tu July 20, 2018, 11:13 p.m. UTC | #1 
On Fri, Jul 20, 2018 at 11:04 AM, Yifeng Sun <pkusunyifeng@gmail.com> wrote:
> Without this patch, OVS kernel module can delete itn->fb_tunnel_dev
> one more time than necessary, which causes kernel crash.
>
> On kernel 4.4.0-116-generic, the crash can be reproduced by running
> the simple test provided below through check-kernel.
>
>   make & make modules_install
>   rmmod ip_gre gre ip_tunnel
>   modprobe openvswitch
>   make check-kernel TESTSUITEFLAGS=x
>   dmesg
>
> Simple test:
>
> AT_SETUP([datapath - crash test])
> OVS_CHECK_GRE()
> ip link del gre0
> OVS_TRAFFIC_VSWITCHD_START()
> AT_CHECK([ovs-vsctl -- set bridge br0])
> ADD_BR([br-underlay], [set bridge br-underlay])
> AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
> AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
> ADD_NAMESPACES(at_ns0)
> ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24")
> AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
> AT_CHECK([ip link set dev br-underlay up])
> ADD_OVS_TUNNEL([gre], [br0], [at_gre0], [172.31.1.1], [10.1.1.100/24])
> tcpdump -U -i br-underlay -w underlay.pcap &
> sleep 1
> OVS_TRAFFIC_VSWITCHD_STOP
> AT_CLEANUP
>
> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
> ---

LGTM, Thanks for the fix.
Acked-by: William Tu <u9012063@gmail.com>
Ben Pfaff Aug. 15, 2018, 8:28 p.m. UTC | #2 
On Fri, Jul 20, 2018 at 04:13:14PM -0700, William Tu wrote:
> On Fri, Jul 20, 2018 at 11:04 AM, Yifeng Sun <pkusunyifeng@gmail.com> wrote:
> > Without this patch, OVS kernel module can delete itn->fb_tunnel_dev
> > one more time than necessary, which causes kernel crash.
> >
> > On kernel 4.4.0-116-generic, the crash can be reproduced by running
> > the simple test provided below through check-kernel.
> >
> >   make & make modules_install
> >   rmmod ip_gre gre ip_tunnel
> >   modprobe openvswitch
> >   make check-kernel TESTSUITEFLAGS=x
> >   dmesg
> >
> > Simple test:
> >
> > AT_SETUP([datapath - crash test])
> > OVS_CHECK_GRE()
> > ip link del gre0
> > OVS_TRAFFIC_VSWITCHD_START()
> > AT_CHECK([ovs-vsctl -- set bridge br0])
> > ADD_BR([br-underlay], [set bridge br-underlay])
> > AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
> > AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
> > ADD_NAMESPACES(at_ns0)
> > ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24")
> > AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
> > AT_CHECK([ip link set dev br-underlay up])
> > ADD_OVS_TUNNEL([gre], [br0], [at_gre0], [172.31.1.1], [10.1.1.100/24])
> > tcpdump -U -i br-underlay -w underlay.pcap &
> > sleep 1
> > OVS_TRAFFIC_VSWITCHD_STOP
> > AT_CLEANUP
> >
> > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
> > ---
> 
> LGTM, Thanks for the fix.
> Acked-by: William Tu <u9012063@gmail.com>

Thanks Yifeng (and William).

I applied this to master and branch-2.10.  If it should be backported
farther, please let me know.
Yifeng Sun June 6, 2019, 11:47 p.m. UTC | #3 
Hi Ben,

Could you check and backport this patch to branch-2.10.
I checked and branch-2.10 doesn't contain this fix.

Thanks Greg for the checking!

Thanks,
Yifeng

On Wed, Aug 15, 2018 at 1:28 PM Ben Pfaff <blp@ovn.org> wrote:
>
> On Fri, Jul 20, 2018 at 04:13:14PM -0700, William Tu wrote:
> > On Fri, Jul 20, 2018 at 11:04 AM, Yifeng Sun <pkusunyifeng@gmail.com> wrote:
> > > Without this patch, OVS kernel module can delete itn->fb_tunnel_dev
> > > one more time than necessary, which causes kernel crash.
> > >
> > > On kernel 4.4.0-116-generic, the crash can be reproduced by running
> > > the simple test provided below through check-kernel.
> > >
> > >   make & make modules_install
> > >   rmmod ip_gre gre ip_tunnel
> > >   modprobe openvswitch
> > >   make check-kernel TESTSUITEFLAGS=x
> > >   dmesg
> > >
> > > Simple test:
> > >
> > > AT_SETUP([datapath - crash test])
> > > OVS_CHECK_GRE()
> > > ip link del gre0
> > > OVS_TRAFFIC_VSWITCHD_START()
> > > AT_CHECK([ovs-vsctl -- set bridge br0])
> > > ADD_BR([br-underlay], [set bridge br-underlay])
> > > AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
> > > AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
> > > ADD_NAMESPACES(at_ns0)
> > > ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24")
> > > AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
> > > AT_CHECK([ip link set dev br-underlay up])
> > > ADD_OVS_TUNNEL([gre], [br0], [at_gre0], [172.31.1.1], [10.1.1.100/24])
> > > tcpdump -U -i br-underlay -w underlay.pcap &
> > > sleep 1
> > > OVS_TRAFFIC_VSWITCHD_STOP
> > > AT_CLEANUP
> > >
> > > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
> > > ---
> >
> > LGTM, Thanks for the fix.
> > Acked-by: William Tu <u9012063@gmail.com>
>
> Thanks Yifeng (and William).
>
> I applied this to master and branch-2.10.  If it should be backported
> farther, please let me know.
Ben Pfaff June 7, 2019, 12:39 a.m. UTC | #4 
Would you mind looking again?  For me,
        git log origin/branch-2.10 --grep 'ip_tunnel: Fix bugs'
does show this commit.

On Thu, Jun 06, 2019 at 04:47:59PM -0700, Yifeng Sun wrote:
> Hi Ben,
> 
> Could you check and backport this patch to branch-2.10.
> I checked and branch-2.10 doesn't contain this fix.
> 
> Thanks Greg for the checking!
> 
> Thanks,
> Yifeng
> 
> On Wed, Aug 15, 2018 at 1:28 PM Ben Pfaff <blp@ovn.org> wrote:
> >
> > On Fri, Jul 20, 2018 at 04:13:14PM -0700, William Tu wrote:
> > > On Fri, Jul 20, 2018 at 11:04 AM, Yifeng Sun <pkusunyifeng@gmail.com> wrote:
> > > > Without this patch, OVS kernel module can delete itn->fb_tunnel_dev
> > > > one more time than necessary, which causes kernel crash.
> > > >
> > > > On kernel 4.4.0-116-generic, the crash can be reproduced by running
> > > > the simple test provided below through check-kernel.
> > > >
> > > >   make & make modules_install
> > > >   rmmod ip_gre gre ip_tunnel
> > > >   modprobe openvswitch
> > > >   make check-kernel TESTSUITEFLAGS=x
> > > >   dmesg
> > > >
> > > > Simple test:
> > > >
> > > > AT_SETUP([datapath - crash test])
> > > > OVS_CHECK_GRE()
> > > > ip link del gre0
> > > > OVS_TRAFFIC_VSWITCHD_START()
> > > > AT_CHECK([ovs-vsctl -- set bridge br0])
> > > > ADD_BR([br-underlay], [set bridge br-underlay])
> > > > AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
> > > > AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
> > > > ADD_NAMESPACES(at_ns0)
> > > > ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24")
> > > > AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
> > > > AT_CHECK([ip link set dev br-underlay up])
> > > > ADD_OVS_TUNNEL([gre], [br0], [at_gre0], [172.31.1.1], [10.1.1.100/24])
> > > > tcpdump -U -i br-underlay -w underlay.pcap &
> > > > sleep 1
> > > > OVS_TRAFFIC_VSWITCHD_STOP
> > > > AT_CLEANUP
> > > >
> > > > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
> > > > ---
> > >
> > > LGTM, Thanks for the fix.
> > > Acked-by: William Tu <u9012063@gmail.com>
> >
> > Thanks Yifeng (and William).
> >
> > I applied this to master and branch-2.10.  If it should be backported
> > farther, please let me know.
Yifeng Sun June 7, 2019, 12:56 a.m. UTC | #5 
I am so sorry, replied in the wrong patch.
I send another email in the correct patch.

Thanks,
Yifeng

On Thu, Jun 6, 2019 at 5:39 PM Ben Pfaff <blp@ovn.org> wrote:
>
> Would you mind looking again?  For me,
>         git log origin/branch-2.10 --grep 'ip_tunnel: Fix bugs'
> does show this commit.
>
> On Thu, Jun 06, 2019 at 04:47:59PM -0700, Yifeng Sun wrote:
> > Hi Ben,
> >
> > Could you check and backport this patch to branch-2.10.
> > I checked and branch-2.10 doesn't contain this fix.
> >
> > Thanks Greg for the checking!
> >
> > Thanks,
> > Yifeng
> >
> > On Wed, Aug 15, 2018 at 1:28 PM Ben Pfaff <blp@ovn.org> wrote:
> > >
> > > On Fri, Jul 20, 2018 at 04:13:14PM -0700, William Tu wrote:
> > > > On Fri, Jul 20, 2018 at 11:04 AM, Yifeng Sun <pkusunyifeng@gmail.com> wrote:
> > > > > Without this patch, OVS kernel module can delete itn->fb_tunnel_dev
> > > > > one more time than necessary, which causes kernel crash.
> > > > >
> > > > > On kernel 4.4.0-116-generic, the crash can be reproduced by running
> > > > > the simple test provided below through check-kernel.
> > > > >
> > > > >   make & make modules_install
> > > > >   rmmod ip_gre gre ip_tunnel
> > > > >   modprobe openvswitch
> > > > >   make check-kernel TESTSUITEFLAGS=x
> > > > >   dmesg
> > > > >
> > > > > Simple test:
> > > > >
> > > > > AT_SETUP([datapath - crash test])
> > > > > OVS_CHECK_GRE()
> > > > > ip link del gre0
> > > > > OVS_TRAFFIC_VSWITCHD_START()
> > > > > AT_CHECK([ovs-vsctl -- set bridge br0])
> > > > > ADD_BR([br-underlay], [set bridge br-underlay])
> > > > > AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
> > > > > AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"])
> > > > > ADD_NAMESPACES(at_ns0)
> > > > > ADD_VETH(p0, at_ns0, br-underlay, "172.31.1.1/24")
> > > > > AT_CHECK([ip addr add dev br-underlay "172.31.1.100/24"])
> > > > > AT_CHECK([ip link set dev br-underlay up])
> > > > > ADD_OVS_TUNNEL([gre], [br0], [at_gre0], [172.31.1.1], [10.1.1.100/24])
> > > > > tcpdump -U -i br-underlay -w underlay.pcap &
> > > > > sleep 1
> > > > > OVS_TRAFFIC_VSWITCHD_STOP
> > > > > AT_CLEANUP
> > > > >
> > > > > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
> > > > > ---
> > > >
> > > > LGTM, Thanks for the fix.
> > > > Acked-by: William Tu <u9012063@gmail.com>
> > >
> > > Thanks Yifeng (and William).
> > >
> > > I applied this to master and branch-2.10.  If it should be backported
> > > farther, please let me know.
diff mbox series

Patch

diff --git a/datapath/linux/compat/ip_tunnel.c b/datapath/linux/compat/ip_tunnel.c
index 54af1f178d36..d16e60fbfef0 100644
--- a/datapath/linux/compat/ip_tunnel.c
+++ b/datapath/linux/compat/ip_tunnel.c
@@ -482,8 +482,10 @@  void rpl_ip_tunnel_dellink(struct net_device *dev, struct list_head *head)
 
 	itn = net_generic(tunnel->net, tunnel->ip_tnl_net_id);
 
-	ip_tunnel_del(itn, netdev_priv(dev));
-        unregister_netdevice_queue(dev, head);
+	if (itn->fb_tunnel_dev != dev) {
+		ip_tunnel_del(itn, netdev_priv(dev));
+		unregister_netdevice_queue(dev, head);
+	}
 }
 
 int rpl_ip_tunnel_init_net(struct net *net, int ip_tnl_net_id,
@@ -642,7 +644,8 @@  void rpl_ip_tunnel_uninit(struct net_device *dev)
 	struct ip_tunnel_net *itn;
 
 	itn = net_generic(net, tunnel->ip_tnl_net_id);
-	ip_tunnel_del(itn, netdev_priv(dev));
+	if (itn->fb_tunnel_dev != dev)
+		ip_tunnel_del(itn, netdev_priv(dev));
 }
 
 /* Do least required initialization, rest of init is done in tunnel_init call */