diff mbox series

[ovs-dev,v2,1/4] ip6_gre: Fix a bug that clears address bits

Message ID 1534339490-20505-1-git-send-email-pkusunyifeng@gmail.com
State Accepted
Headers show
Series [ovs-dev,v2,1/4] ip6_gre: Fix a bug that clears address bits | expand

Commit Message

Yifeng Sun Aug. 15, 2018, 1:24 p.m. UTC
In compatable gre module, skb->cb is solely used as ovs_gso_cb.
However, IPCB(skb) also points to skb->cb. IPCB(skb)->flags overlaps
with ovs_gso_cb.tun_dst. As a result, this bug clears the 16-23 bit
in the address of ovs_gso_cb.tun_dst and causes kernel to crash.

Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
---
v1->v2: Improved commit message and fixed __gre6_xmit, thanks Greg!

 datapath/linux/compat/ip6_gre.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Gregory Rose Aug. 16, 2018, 4:14 p.m. UTC | #1
On 8/15/2018 6:24 AM, Yifeng Sun wrote:
> In compatable gre module, skb->cb is solely used as ovs_gso_cb.
> However, IPCB(skb) also points to skb->cb. IPCB(skb)->flags overlaps
> with ovs_gso_cb.tun_dst. As a result, this bug clears the 16-23 bit
> in the address of ovs_gso_cb.tun_dst and causes kernel to crash.
>
> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
> ---
> v1->v2: Improved commit message and fixed __gre6_xmit, thanks Greg!
>
>   datapath/linux/compat/ip6_gre.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/datapath/linux/compat/ip6_gre.c b/datapath/linux/compat/ip6_gre.c
> index 54a76ab..3904455 100644
> --- a/datapath/linux/compat/ip6_gre.c
> +++ b/datapath/linux/compat/ip6_gre.c
> @@ -876,9 +876,6 @@ static netdev_tx_t __gre6_xmit(struct sk_buff *skb,
>   	struct tnl_ptk_info tpi;
>   	__be16 protocol;
>   
> -	if (dev->type == ARPHRD_ETHER)
> -		IPCB(skb)->flags = 0;
> -
>   	if (dev->header_ops && dev->type == ARPHRD_IP6GRE)
>   		fl6->daddr = ((struct ipv6hdr *)skb->data)->daddr;
>   	else
> @@ -1146,7 +1143,6 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb,
>   		goto tx_err;
>   
>   	t->parms.o_flags &= ~TUNNEL_KEY;
> -	IPCB(skb)->flags = 0;
>   
>   	tun_info = ovs_skb_tunnel_info(skb);
>   	if (unlikely(!tun_info ||

Looks good.

Tested-by: Greg Rose <gvrose8192@gmail.com>
Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Yifeng Sun June 7, 2019, 12:55 a.m. UTC | #2
Hi Ben,

Can you please backport this patch to 2.10, thanks.

Yifeng

On Thu, Aug 16, 2018 at 9:14 AM Gregory Rose <gvrose8192@gmail.com> wrote:
>
>
> On 8/15/2018 6:24 AM, Yifeng Sun wrote:
> > In compatable gre module, skb->cb is solely used as ovs_gso_cb.
> > However, IPCB(skb) also points to skb->cb. IPCB(skb)->flags overlaps
> > with ovs_gso_cb.tun_dst. As a result, this bug clears the 16-23 bit
> > in the address of ovs_gso_cb.tun_dst and causes kernel to crash.
> >
> > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
> > ---
> > v1->v2: Improved commit message and fixed __gre6_xmit, thanks Greg!
> >
> >   datapath/linux/compat/ip6_gre.c | 4 ----
> >   1 file changed, 4 deletions(-)
> >
> > diff --git a/datapath/linux/compat/ip6_gre.c b/datapath/linux/compat/ip6_gre.c
> > index 54a76ab..3904455 100644
> > --- a/datapath/linux/compat/ip6_gre.c
> > +++ b/datapath/linux/compat/ip6_gre.c
> > @@ -876,9 +876,6 @@ static netdev_tx_t __gre6_xmit(struct sk_buff *skb,
> >       struct tnl_ptk_info tpi;
> >       __be16 protocol;
> >
> > -     if (dev->type == ARPHRD_ETHER)
> > -             IPCB(skb)->flags = 0;
> > -
> >       if (dev->header_ops && dev->type == ARPHRD_IP6GRE)
> >               fl6->daddr = ((struct ipv6hdr *)skb->data)->daddr;
> >       else
> > @@ -1146,7 +1143,6 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb,
> >               goto tx_err;
> >
> >       t->parms.o_flags &= ~TUNNEL_KEY;
> > -     IPCB(skb)->flags = 0;
> >
> >       tun_info = ovs_skb_tunnel_info(skb);
> >       if (unlikely(!tun_info ||
>
> Looks good.
>
> Tested-by: Greg Rose <gvrose8192@gmail.com>
> Reviewed-by: Greg Rose <gvrose8192@gmail.com>
>
Ben Pfaff June 7, 2019, 3:59 p.m. UTC | #3
Done.

On Thu, Jun 06, 2019 at 05:55:33PM -0700, Yifeng Sun wrote:
> Hi Ben,
> 
> Can you please backport this patch to 2.10, thanks.
> 
> Yifeng
> 
> On Thu, Aug 16, 2018 at 9:14 AM Gregory Rose <gvrose8192@gmail.com> wrote:
> >
> >
> > On 8/15/2018 6:24 AM, Yifeng Sun wrote:
> > > In compatable gre module, skb->cb is solely used as ovs_gso_cb.
> > > However, IPCB(skb) also points to skb->cb. IPCB(skb)->flags overlaps
> > > with ovs_gso_cb.tun_dst. As a result, this bug clears the 16-23 bit
> > > in the address of ovs_gso_cb.tun_dst and causes kernel to crash.
> > >
> > > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
> > > ---
> > > v1->v2: Improved commit message and fixed __gre6_xmit, thanks Greg!
> > >
> > >   datapath/linux/compat/ip6_gre.c | 4 ----
> > >   1 file changed, 4 deletions(-)
> > >
> > > diff --git a/datapath/linux/compat/ip6_gre.c b/datapath/linux/compat/ip6_gre.c
> > > index 54a76ab..3904455 100644
> > > --- a/datapath/linux/compat/ip6_gre.c
> > > +++ b/datapath/linux/compat/ip6_gre.c
> > > @@ -876,9 +876,6 @@ static netdev_tx_t __gre6_xmit(struct sk_buff *skb,
> > >       struct tnl_ptk_info tpi;
> > >       __be16 protocol;
> > >
> > > -     if (dev->type == ARPHRD_ETHER)
> > > -             IPCB(skb)->flags = 0;
> > > -
> > >       if (dev->header_ops && dev->type == ARPHRD_IP6GRE)
> > >               fl6->daddr = ((struct ipv6hdr *)skb->data)->daddr;
> > >       else
> > > @@ -1146,7 +1143,6 @@ static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb,
> > >               goto tx_err;
> > >
> > >       t->parms.o_flags &= ~TUNNEL_KEY;
> > > -     IPCB(skb)->flags = 0;
> > >
> > >       tun_info = ovs_skb_tunnel_info(skb);
> > >       if (unlikely(!tun_info ||
> >
> > Looks good.
> >
> > Tested-by: Greg Rose <gvrose8192@gmail.com>
> > Reviewed-by: Greg Rose <gvrose8192@gmail.com>
> >
diff mbox series

Patch

diff --git a/datapath/linux/compat/ip6_gre.c b/datapath/linux/compat/ip6_gre.c
index 54a76ab..3904455 100644
--- a/datapath/linux/compat/ip6_gre.c
+++ b/datapath/linux/compat/ip6_gre.c
@@ -876,9 +876,6 @@  static netdev_tx_t __gre6_xmit(struct sk_buff *skb,
 	struct tnl_ptk_info tpi;
 	__be16 protocol;
 
-	if (dev->type == ARPHRD_ETHER)
-		IPCB(skb)->flags = 0;
-
 	if (dev->header_ops && dev->type == ARPHRD_IP6GRE)
 		fl6->daddr = ((struct ipv6hdr *)skb->data)->daddr;
 	else
@@ -1146,7 +1143,6 @@  static netdev_tx_t ip6erspan_tunnel_xmit(struct sk_buff *skb,
 		goto tx_err;
 
 	t->parms.o_flags &= ~TUNNEL_KEY;
-	IPCB(skb)->flags = 0;
 
 	tun_info = ovs_skb_tunnel_info(skb);
 	if (unlikely(!tun_info ||