* [PATCHv2 net] net: ip6_gre: access skb data after skb_cow_head()
@ 2019-05-30 16:59 William Tu
2019-05-30 17:23 ` Gregory Rose
0 siblings, 1 reply; 3+ messages in thread
From: William Tu @ 2019-05-30 16:59 UTC (permalink / raw)
To: netdev
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;
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCHv2 net] net: ip6_gre: access skb data after skb_cow_head()
2019-05-30 16:59 [PATCHv2 net] net: ip6_gre: access skb data after skb_cow_head() William Tu
@ 2019-05-30 17:23 ` Gregory Rose
2019-05-30 17:29 ` William Tu
0 siblings, 1 reply; 3+ messages in thread
From: Gregory Rose @ 2019-05-30 17:23 UTC (permalink / raw)
To: William Tu, netdev
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>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCHv2 net] net: ip6_gre: access skb data after skb_cow_head()
2019-05-30 17:23 ` Gregory Rose
@ 2019-05-30 17:29 ` William Tu
0 siblings, 0 replies; 3+ messages in thread
From: William Tu @ 2019-05-30 17:29 UTC (permalink / raw)
To: Gregory Rose; +Cc: Linux Kernel Network Developers
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>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-05-30 17:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-30 16:59 [PATCHv2 net] net: ip6_gre: access skb data after skb_cow_head() William Tu
2019-05-30 17:23 ` Gregory Rose
2019-05-30 17:29 ` William Tu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.