| Message ID | 1559235580-31747-1-git-send-email-u9012063@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [PATCHv2,net] net: ip6_gre: access skb data after skb_cow_head() | expand |
On 5/30/2019 9:59 AM, William Tu wrote: > When increases the headroom, skb's data pointer might get re-allocated. > As a result, the skb->data before the skb_cow_head becomes a dangling pointer, > and dereferences to daddr causes general protection fault at the following > line in __gre6_xmit(): > > if (dev->header_ops && dev->type == ARPHRD_IP6GRE) > fl6->daddr = ((struct ipv6hdr *)skb->data)->daddr; > > general protection fault: 0000 [#1] SMP PTI > OE 4.15.0-43-generic #146-Ubuntu > Hardware name: VMware, Inc. VMware Virtual Platform 440BX Desktop Reference > Platform, BIOS 6.00 07/03/2018 > RIP: 0010: __gre6_xmit+0x11f/0x2c0 [openvswitch] > RSP: 0018:ffffb8d5c44df6a8 EFLAGS: 00010286 > RAX: 00000000ffffffea RBX: ffff8b1528a0000 RCX: 0000000000000036 > RDX: ffff000000000000 RSI: 0000000000000000 RDI: ffff8db267829200 > RBP: ffffb8d5c44df 700 R08: 0000000000005865 RØ9: ffffb8d5c44df724 > R10: 0000000000000002 R11: 0000000000000000 R12: ffff8db267829200 > R13: 0000000000000000 R14: ffffb8d5c44df 728 R15: 00000000ffffffff > FS: 00007f8744df 2700(0000) GS:ffff8db27fc0000000000) knlGS:0000000000000000 > CS: 0910 DS: 0000 ES: 9000 CRO: 0000000080050033 > CR2: 00007f893ef92148 CR3: 0000000400462003 CR4: 00000000001626f8 > Call Trace: > ip6gre_tunnel_xmit+0x1cc/0x530 [openvswitch] > ? skb_clone+0x58/0xc0 > __ip6gre_tunnel_xmit+0x12/0x20 [openvswitch] > ovs_vport_send +0xd4/0x170 [openvswitch] > do_output+0x53/0x160 [openvswitch] > do_execute_actions+0x9a1/0x1880 [openvswitch] > > Fix it by moving skb_cow_head before accessing the skb->data pointer. > > Fixes: 01b8d064d58b4 ("net: ip6_gre: Request headroom in __gre6_xmit()") > Reported-by: Haichao Ma <haichaom@vmware.com> > Signed-off-by: William Tu <u9012063@gmail.com> > --- > v1-v2: add more details in commit message. > --- > net/ipv6/ip6_gre.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c > index 655e46b227f9..90b2b129b105 100644 > --- a/net/ipv6/ip6_gre.c > +++ b/net/ipv6/ip6_gre.c > @@ -714,6 +714,9 @@ static netdev_tx_t __gre6_xmit(struct sk_buff *skb, > struct ip6_tnl *tunnel = netdev_priv(dev); > __be16 protocol; > > + if (skb_cow_head(skb, dev->needed_headroom ?: tunnel->hlen)) > + return -ENOMEM; > + > if (dev->type == ARPHRD_ETHER) > IPCB(skb)->flags = 0; > > @@ -722,9 +725,6 @@ static netdev_tx_t __gre6_xmit(struct sk_buff *skb, > else > fl6->daddr = tunnel->parms.raddr; > > - if (skb_cow_head(skb, dev->needed_headroom ?: tunnel->hlen)) > - return -ENOMEM; > - > /* Push GRE header. */ > protocol = (dev->type == ARPHRD_ETHER) ? htons(ETH_P_TEB) : proto; > Tested-by: Greg Rose <gvrose8192@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Please ignore this patch and allow me to investigate more. On Thu, May 30, 2019 at 10:23 AM Gregory Rose <gvrose8192@gmail.com> wrote: > > > On 5/30/2019 9:59 AM, William Tu wrote: > > When increases the headroom, skb's data pointer might get re-allocated. > > As a result, the skb->data before the skb_cow_head becomes a dangling pointer, > > and dereferences to daddr causes general protection fault at the following > > line in __gre6_xmit(): > > > > if (dev->header_ops && dev->type == ARPHRD_IP6GRE) > > fl6->daddr = ((struct ipv6hdr *)skb->data)->daddr; > > Look again Dave's comment that The fl6->daddr assignments are object copies, not pointer assignments. So this shouldn't cause any problem after skb_cow_head. I will work on the right fix. Thanks, William > > general protection fault: 0000 [#1] SMP PTI > > OE 4.15.0-43-generic #146-Ubuntu > > Hardware name: VMware, Inc. VMware Virtual Platform 440BX Desktop Reference > > Platform, BIOS 6.00 07/03/2018 > > RIP: 0010: __gre6_xmit+0x11f/0x2c0 [openvswitch] > > RSP: 0018:ffffb8d5c44df6a8 EFLAGS: 00010286 > > RAX: 00000000ffffffea RBX: ffff8b1528a0000 RCX: 0000000000000036 > > RDX: ffff000000000000 RSI: 0000000000000000 RDI: ffff8db267829200 > > RBP: ffffb8d5c44df 700 R08: 0000000000005865 RØ9: ffffb8d5c44df724 > > R10: 0000000000000002 R11: 0000000000000000 R12: ffff8db267829200 > > R13: 0000000000000000 R14: ffffb8d5c44df 728 R15: 00000000ffffffff > > FS: 00007f8744df 2700(0000) GS:ffff8db27fc0000000000) knlGS:0000000000000000 > > CS: 0910 DS: 0000 ES: 9000 CRO: 0000000080050033 > > CR2: 00007f893ef92148 CR3: 0000000400462003 CR4: 00000000001626f8 > > Call Trace: > > ip6gre_tunnel_xmit+0x1cc/0x530 [openvswitch] > > ? skb_clone+0x58/0xc0 > > __ip6gre_tunnel_xmit+0x12/0x20 [openvswitch] > > ovs_vport_send +0xd4/0x170 [openvswitch] > > do_output+0x53/0x160 [openvswitch] > > do_execute_actions+0x9a1/0x1880 [openvswitch] > > > > Fix it by moving skb_cow_head before accessing the skb->data pointer. > > > > Fixes: 01b8d064d58b4 ("net: ip6_gre: Request headroom in __gre6_xmit()") > > Reported-by: Haichao Ma <haichaom@vmware.com> > > Signed-off-by: William Tu <u9012063@gmail.com> > > --- > > v1-v2: add more details in commit message. > > --- > > net/ipv6/ip6_gre.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c > > index 655e46b227f9..90b2b129b105 100644 > > --- a/net/ipv6/ip6_gre.c > > +++ b/net/ipv6/ip6_gre.c > > @@ -714,6 +714,9 @@ static netdev_tx_t __gre6_xmit(struct sk_buff *skb, > > struct ip6_tnl *tunnel = netdev_priv(dev); > > __be16 protocol; > > > > + if (skb_cow_head(skb, dev->needed_headroom ?: tunnel->hlen)) > > + return -ENOMEM; > > + > > if (dev->type == ARPHRD_ETHER) > > IPCB(skb)->flags = 0; > > > > @@ -722,9 +725,6 @@ static netdev_tx_t __gre6_xmit(struct sk_buff *skb, > > else > > fl6->daddr = tunnel->parms.raddr; > > > > - if (skb_cow_head(skb, dev->needed_headroom ?: tunnel->hlen)) > > - return -ENOMEM; > > - > > /* Push GRE header. */ > > protocol = (dev->type == ARPHRD_ETHER) ? htons(ETH_P_TEB) : proto; > > > > Tested-by: Greg Rose <gvrose8192@gmail.com> > Reviewed-by: Greg Rose <gvrose8192@gmail.com> >
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c index 655e46b227f9..90b2b129b105 100644 --- a/net/ipv6/ip6_gre.c +++ b/net/ipv6/ip6_gre.c @@ -714,6 +714,9 @@ static netdev_tx_t __gre6_xmit(struct sk_buff *skb, struct ip6_tnl *tunnel = netdev_priv(dev); __be16 protocol; + if (skb_cow_head(skb, dev->needed_headroom ?: tunnel->hlen)) + return -ENOMEM; + if (dev->type == ARPHRD_ETHER) IPCB(skb)->flags = 0; @@ -722,9 +725,6 @@ static netdev_tx_t __gre6_xmit(struct sk_buff *skb, else fl6->daddr = tunnel->parms.raddr; - if (skb_cow_head(skb, dev->needed_headroom ?: tunnel->hlen)) - return -ENOMEM; - /* Push GRE header. */ protocol = (dev->type == ARPHRD_ETHER) ? htons(ETH_P_TEB) : proto;
When increases the headroom, skb's data pointer might get re-allocated. As a result, the skb->data before the skb_cow_head becomes a dangling pointer, and dereferences to daddr causes general protection fault at the following line in __gre6_xmit(): if (dev->header_ops && dev->type == ARPHRD_IP6GRE) fl6->daddr = ((struct ipv6hdr *)skb->data)->daddr; general protection fault: 0000 [#1] SMP PTI OE 4.15.0-43-generic #146-Ubuntu Hardware name: VMware, Inc. VMware Virtual Platform 440BX Desktop Reference Platform, BIOS 6.00 07/03/2018 RIP: 0010: __gre6_xmit+0x11f/0x2c0 [openvswitch] RSP: 0018:ffffb8d5c44df6a8 EFLAGS: 00010286 RAX: 00000000ffffffea RBX: ffff8b1528a0000 RCX: 0000000000000036 RDX: ffff000000000000 RSI: 0000000000000000 RDI: ffff8db267829200 RBP: ffffb8d5c44df 700 R08: 0000000000005865 RØ9: ffffb8d5c44df724 R10: 0000000000000002 R11: 0000000000000000 R12: ffff8db267829200 R13: 0000000000000000 R14: ffffb8d5c44df 728 R15: 00000000ffffffff FS: 00007f8744df 2700(0000) GS:ffff8db27fc0000000000) knlGS:0000000000000000 CS: 0910 DS: 0000 ES: 9000 CRO: 0000000080050033 CR2: 00007f893ef92148 CR3: 0000000400462003 CR4: 00000000001626f8 Call Trace: ip6gre_tunnel_xmit+0x1cc/0x530 [openvswitch] ? skb_clone+0x58/0xc0 __ip6gre_tunnel_xmit+0x12/0x20 [openvswitch] ovs_vport_send +0xd4/0x170 [openvswitch] do_output+0x53/0x160 [openvswitch] do_execute_actions+0x9a1/0x1880 [openvswitch] Fix it by moving skb_cow_head before accessing the skb->data pointer. Fixes: 01b8d064d58b4 ("net: ip6_gre: Request headroom in __gre6_xmit()") Reported-by: Haichao Ma <haichaom@vmware.com> Signed-off-by: William Tu <u9012063@gmail.com> --- v1-v2: add more details in commit message. --- net/ipv6/ip6_gre.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)