[net-next PATCH] udp: Resolve NULL pointer dereference

[net-next PATCH] udp: Resolve NULL pointer dereference

[Alexander Duyck]

While testing an OpenStack configuration using VXLANs I saw the following
call trace:

 RIP: 0010:[<ffffffff815fad49>] udp4_lib_lookup_skb+0x49/0x80
 RSP: 0018:ffff88103867bc50  EFLAGS: 00010286
 RAX: ffff88103269bf00 RBX: ffff88103269bf00 RCX: 00000000ffffffff
 RDX: 0000000000004300 RSI: 0000000000000000 RDI: ffff880f2932e780
 RBP: ffff88103867bc60 R08: 0000000000000000 R09: 000000009001a8c0
 R10: 0000000000004400 R11: ffffffff81333a58 R12: ffff880f2932e794
 R13: 0000000000000014 R14: 0000000000000014 R15: ffffe8efbfd89ca0
 FS:  0000000000000000(0000) GS:ffff88103fd80000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 0000000000000488 CR3: 0000000001c06000 CR4: 00000000001426e0
 Stack:
  ffffffff81576515 ffffffff815733c0 ffff88103867bc98 ffffffff815fcc17
  ffff88103269bf00 ffffe8efbfd89ca0 0000000000000014 0000000000000080
  ffffe8efbfd89ca0 ffff88103867bcc8 ffffffff815fcf8b ffff880f2932e794
 Call Trace:
  [<ffffffff81576515>] ? skb_checksum+0x35/0x50
  [<ffffffff815733c0>] ? skb_push+0x40/0x40
  [<ffffffff815fcc17>] udp_gro_receive+0x57/0x130
  [<ffffffff815fcf8b>] udp4_gro_receive+0x10b/0x2c0
  [<ffffffff81605863>] inet_gro_receive+0x1d3/0x270
  [<ffffffff81589e59>] dev_gro_receive+0x269/0x3b0
  [<ffffffff8158a1b8>] napi_gro_receive+0x38/0x120
  [<ffffffffa0871297>] gro_cell_poll+0x57/0x80 [vxlan]
  [<ffffffff815899d0>] net_rx_action+0x160/0x380
  [<ffffffff816965c7>] __do_softirq+0xd7/0x2c5
  [<ffffffff8107d969>] run_ksoftirqd+0x29/0x50
  [<ffffffff8109a50f>] smpboot_thread_fn+0x10f/0x160
  [<ffffffff8109a400>] ? sort_range+0x30/0x30
  [<ffffffff81096da8>] kthread+0xd8/0xf0
  [<ffffffff81693c82>] ret_from_fork+0x22/0x40
  [<ffffffff81096cd0>] ? kthread_park+0x60/0x60

I believe the trace is pointing to the call to dev_net(dev) in
udp4_lib_lookup_skb.  If I am not mistaken I believe it is possible for us
to have skb_dst(skb)->dev be NULL.  So to resolve that I am adding a check
for this case and skipping the assignment if such an event occurs.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 net/ipv4/udp.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index f67f52ba4809..ff8d9ff3048b 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -613,8 +613,12 @@ struct sock *udp4_lib_lookup_skb(struct sk_buff *skb,
 				 __be16 sport, __be16 dport)
 {
 	const struct iphdr *iph = ip_hdr(skb);
-	const struct net_device *dev =
-	    skb_dst(skb) ? skb_dst(skb)->dev : skb->dev;
+	const struct net_device *dev;
+
+	if (skb_dst(skb) && skb_dst(skb)->dev)
+		dev = skb_dst(skb)->dev;
+	else
+		dev = skb->dev;
 
 	return __udp4_lib_lookup(dev_net(dev), iph->saddr, sport,
 				 iph->daddr, dport, inet_iif(skb),

Re: [net-next PATCH] udp: Resolve NULL pointer dereference

[Eric Dumazet]

On Wed, 2016-05-11 at 19:24 -0700, Alexander Duyck wrote:
> While testing an OpenStack configuration using VXLANs I saw the following
> call trace:

> 
> I believe the trace is pointing to the call to dev_net(dev) in
> udp4_lib_lookup_skb.  If I am not mistaken I believe it is possible for us
> to have skb_dst(skb)->dev be NULL.  So to resolve that I am adding a check
> for this case and skipping the assignment if such an event occurs.

skb_dst(skb)->dev can be NULL ???

Why only UDP ipv4 would need a fix, and not ipv6 ?

Looks the bug is somewhere else maybe ?

Re: [net-next PATCH] udp: Resolve NULL pointer dereference

[Alexander Duyck]

On Wed, May 11, 2016 at 8:43 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2016-05-11 at 19:24 -0700, Alexander Duyck wrote:
>> While testing an OpenStack configuration using VXLANs I saw the following
>> call trace:
>
>>
>> I believe the trace is pointing to the call to dev_net(dev) in
>> udp4_lib_lookup_skb.  If I am not mistaken I believe it is possible for us
>> to have skb_dst(skb)->dev be NULL.  So to resolve that I am adding a check
>> for this case and skipping the assignment if such an event occurs.
>
> skb_dst(skb)->dev can be NULL ???

That is what appears to be happening.  Though I can do some more
research into this.

> Why only UDP ipv4 would need a fix, and not ipv6 ?

Yeah, I should probably fix IPv6 as well if there is an issue.

> Looks the bug is somewhere else maybe ?

I was thinking about it last night and I am not certain we should even
see skb_dst(skb) set as Cong mentioned.  I'm wondering if I should
update the code to just use skb->dev because these functions are
currently only used in the GRO path anyway so it isn't as if skb_dst
should be populated.

- Alex

Re: [net-next PATCH] udp: Resolve NULL pointer dereference

[Cong Wang]

On Wed, May 11, 2016 at 7:24 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> While testing an OpenStack configuration using VXLANs I saw the following
> call trace:
...
>
> I believe the trace is pointing to the call to dev_net(dev) in
> udp4_lib_lookup_skb.  If I am not mistaken I believe it is possible for us
> to have skb_dst(skb)->dev be NULL.  So to resolve that I am adding a check
> for this case and skipping the assignment if such an event occurs.

Isn't in this tunneling case skb_dst() should be NULL?

[net-next PATCH v2] udp: Resolve NULL pointer dereference over flow-based vxlan device

[Alexander Duyck]

While testing an OpenStack configuration using VXLANs I saw the following
call trace:

 RIP: 0010:[<ffffffff815fad49>] udp4_lib_lookup_skb+0x49/0x80
 RSP: 0018:ffff88103867bc50  EFLAGS: 00010286
 RAX: ffff88103269bf00 RBX: ffff88103269bf00 RCX: 00000000ffffffff
 RDX: 0000000000004300 RSI: 0000000000000000 RDI: ffff880f2932e780
 RBP: ffff88103867bc60 R08: 0000000000000000 R09: 000000009001a8c0
 R10: 0000000000004400 R11: ffffffff81333a58 R12: ffff880f2932e794
 R13: 0000000000000014 R14: 0000000000000014 R15: ffffe8efbfd89ca0
 FS:  0000000000000000(0000) GS:ffff88103fd80000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 0000000000000488 CR3: 0000000001c06000 CR4: 00000000001426e0
 Stack:
  ffffffff81576515 ffffffff815733c0 ffff88103867bc98 ffffffff815fcc17
  ffff88103269bf00 ffffe8efbfd89ca0 0000000000000014 0000000000000080
  ffffe8efbfd89ca0 ffff88103867bcc8 ffffffff815fcf8b ffff880f2932e794
 Call Trace:
  [<ffffffff81576515>] ? skb_checksum+0x35/0x50
  [<ffffffff815733c0>] ? skb_push+0x40/0x40
  [<ffffffff815fcc17>] udp_gro_receive+0x57/0x130
  [<ffffffff815fcf8b>] udp4_gro_receive+0x10b/0x2c0
  [<ffffffff81605863>] inet_gro_receive+0x1d3/0x270
  [<ffffffff81589e59>] dev_gro_receive+0x269/0x3b0
  [<ffffffff8158a1b8>] napi_gro_receive+0x38/0x120
  [<ffffffffa0871297>] gro_cell_poll+0x57/0x80 [vxlan]
  [<ffffffff815899d0>] net_rx_action+0x160/0x380
  [<ffffffff816965c7>] __do_softirq+0xd7/0x2c5
  [<ffffffff8107d969>] run_ksoftirqd+0x29/0x50
  [<ffffffff8109a50f>] smpboot_thread_fn+0x10f/0x160
  [<ffffffff8109a400>] ? sort_range+0x30/0x30
  [<ffffffff81096da8>] kthread+0xd8/0xf0
  [<ffffffff81693c82>] ret_from_fork+0x22/0x40
  [<ffffffff81096cd0>] ? kthread_park+0x60/0x60

The following trace is seen when receiving a DHCP request over a flow-based
VXLAN tunnel.  I believe this is caused by the metadata dst having a NULL
dev value and as a result dev_net(dev) is causing a NULL pointer dereference.

To resolve this I am replacing the check for skb_dst() with skb_valid_dst()
so that we do not attempt to use the metadata dst to retrieve a device in
order to determine the network namespace.

Fixes: 63058308cd55 ("udp: Add udp6_lib_lookup_skb and udp4_lib_lookup_skb")
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 net/ipv4/udp.c |    3 ++-
 net/ipv6/udp.c |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index f67f52ba4809..69aa7ab81933 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -114,6 +114,7 @@
 #include <net/busy_poll.h>
 #include "udp_impl.h"
 #include <net/sock_reuseport.h>
+#include <net/dst_metadata.h>
 
 struct udp_table udp_table __read_mostly;
 EXPORT_SYMBOL(udp_table);
@@ -614,7 +615,7 @@ struct sock *udp4_lib_lookup_skb(struct sk_buff *skb,
 {
 	const struct iphdr *iph = ip_hdr(skb);
 	const struct net_device *dev =
-	    skb_dst(skb) ? skb_dst(skb)->dev : skb->dev;
+		skb_valid_dst(skb) ? skb_dst(skb)->dev : skb->dev;
 
 	return __udp4_lib_lookup(dev_net(dev), iph->saddr, sport,
 				 iph->daddr, dport, inet_iif(skb),
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index aca06094110f..12c06deda5ef 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -49,6 +49,7 @@
 #include <net/inet6_hashtables.h>
 #include <net/busy_poll.h>
 #include <net/sock_reuseport.h>
+#include <net/dst_metadata.h>
 
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
@@ -331,7 +332,7 @@ struct sock *udp6_lib_lookup_skb(struct sk_buff *skb,
 {
 	const struct ipv6hdr *iph = ipv6_hdr(skb);
 	const struct net_device *dev =
-	    skb_dst(skb) ? skb_dst(skb)->dev : skb->dev;
+		skb_valid_dst(skb) ? skb_dst(skb)->dev : skb->dev;
 
 	return __udp6_lib_lookup(dev_net(dev), &iph->saddr, sport,
 				 &iph->daddr, dport, inet6_iif(skb),

Re: [net-next PATCH v2] udp: Resolve NULL pointer dereference over flow-based vxlan device

[Eric Dumazet]

On Thu, 2016-05-12 at 12:51 -0700, Alexander Duyck wrote:
> While testing an OpenStack configuration using VXLANs I saw the following
> call trace:

> The following trace is seen when receiving a DHCP request over a flow-based
> VXLAN tunnel.  I believe this is caused by the metadata dst having a NULL
> dev value and as a result dev_net(dev) is causing a NULL pointer dereference.
> 
> To resolve this I am replacing the check for skb_dst() with skb_valid_dst()
> so that we do not attempt to use the metadata dst to retrieve a device in
> order to determine the network namespace.
> 
> Fixes: 63058308cd55 ("udp: Add udp6_lib_lookup_skb and udp4_lib_lookup_skb")
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
>  net/ipv4/udp.c |    3 ++-
>  net/ipv6/udp.c |    3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index f67f52ba4809..69aa7ab81933 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -114,6 +114,7 @@
>  #include <net/busy_poll.h>
>  #include "udp_impl.h"
>  #include <net/sock_reuseport.h>
> +#include <net/dst_metadata.h>
>  
>  struct udp_table udp_table __read_mostly;
>  EXPORT_SYMBOL(udp_table);
> @@ -614,7 +615,7 @@ struct sock *udp4_lib_lookup_skb(struct sk_buff *skb,
>  {
>  	const struct iphdr *iph = ip_hdr(skb);
>  	const struct net_device *dev =
> -	    skb_dst(skb) ? skb_dst(skb)->dev : skb->dev;
> +		skb_valid_dst(skb) ? skb_dst(skb)->dev : skb->dev;

Looks overly complicated to me.

If this is called from GRO, why don't we simply use skb->dev ?

Re: [net-next PATCH v2] udp: Resolve NULL pointer dereference over flow-based vxlan device

[Alexander Duyck]

On Thu, May 12, 2016 at 12:55 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2016-05-12 at 12:51 -0700, Alexander Duyck wrote:
>> While testing an OpenStack configuration using VXLANs I saw the following
>> call trace:
>
>> The following trace is seen when receiving a DHCP request over a flow-based
>> VXLAN tunnel.  I believe this is caused by the metadata dst having a NULL
>> dev value and as a result dev_net(dev) is causing a NULL pointer dereference.
>>
>> To resolve this I am replacing the check for skb_dst() with skb_valid_dst()
>> so that we do not attempt to use the metadata dst to retrieve a device in
>> order to determine the network namespace.
>>
>> Fixes: 63058308cd55 ("udp: Add udp6_lib_lookup_skb and udp4_lib_lookup_skb")
>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>> ---
>>  net/ipv4/udp.c |    3 ++-
>>  net/ipv6/udp.c |    3 ++-
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
>> index f67f52ba4809..69aa7ab81933 100644
>> --- a/net/ipv4/udp.c
>> +++ b/net/ipv4/udp.c
>> @@ -114,6 +114,7 @@
>>  #include <net/busy_poll.h>
>>  #include "udp_impl.h"
>>  #include <net/sock_reuseport.h>
>> +#include <net/dst_metadata.h>
>>
>>  struct udp_table udp_table __read_mostly;
>>  EXPORT_SYMBOL(udp_table);
>> @@ -614,7 +615,7 @@ struct sock *udp4_lib_lookup_skb(struct sk_buff *skb,
>>  {
>>       const struct iphdr *iph = ip_hdr(skb);
>>       const struct net_device *dev =
>> -         skb_dst(skb) ? skb_dst(skb)->dev : skb->dev;
>> +             skb_valid_dst(skb) ? skb_dst(skb)->dev : skb->dev;
>
> Looks overly complicated to me.
>
> If this is called from GRO, why don't we simply use skb->dev ?

I'm assuming this was using skb_dst(skb)->dev in order to allow for
use of this function by other callers since the original function
__udp4_lib_lookup_skb was using that.  If we change this then it
reduces the likelihood of the code being reusable by other callers.
In such a case I would probably want to go through and also rename the
functions to make sure they are tagged as being GRO specific.

- Alex

Re: [net-next PATCH v2] udp: Resolve NULL pointer dereference over flow-based vxlan device

[Eric Dumazet]

On Thu, 2016-05-12 at 13:27 -0700, Alexander Duyck wrote:

> I'm assuming this was using skb_dst(skb)->dev in order to allow for
> use of this function by other callers since the original function
> __udp4_lib_lookup_skb was using that.  If we change this then it
> reduces the likelihood of the code being reusable by other callers.
> In such a case I would probably want to go through and also rename the
> functions to make sure they are tagged as being GRO specific.
> 

__udp4_lib_lookup_skb() should use skb->dev as well,
or get the net pointer from its caller.

It is called from __udp4_lib_rcv() and this one simply does :

struct net *net = dev_net(skb->dev);

So for consistency (and small performance gain) I would suggest :

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index f67f52ba4809..d9006f2d28eb 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -604,7 +604,7 @@ static inline struct sock *__udp4_lib_lookup_skb(struct sk_buff *skb,
 {
        const struct iphdr *iph = ip_hdr(skb);
 
-       return __udp4_lib_lookup(dev_net(skb_dst(skb)->dev), iph->saddr, sport,
+       return __udp4_lib_lookup(dev_net(skb->dev), iph->saddr, sport,
                                 iph->daddr, dport, inet_iif(skb),
                                 udptable, skb);
 }

Re: [net-next PATCH v2] udp: Resolve NULL pointer dereference over flow-based vxlan device

[Cong Wang]

On Thu, May 12, 2016 at 12:51 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> The following trace is seen when receiving a DHCP request over a flow-based
> VXLAN tunnel.  I believe this is caused by the metadata dst having a NULL
> dev value and as a result dev_net(dev) is causing a NULL pointer dereference.
>
> To resolve this I am replacing the check for skb_dst() with skb_valid_dst()
> so that we do not attempt to use the metadata dst to retrieve a device in
> order to determine the network namespace.

Why does UDP layer need to care about tunnel layer things?

If the problem is really what you describe, then the tunnel layer
should reset that dst after finish.

Re: [net-next PATCH v2] udp: Resolve NULL pointer dereference over flow-based vxlan device

[Alexander Duyck]

On Thu, May 12, 2016 at 1:02 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Thu, May 12, 2016 at 12:51 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
>> The following trace is seen when receiving a DHCP request over a flow-based
>> VXLAN tunnel.  I believe this is caused by the metadata dst having a NULL
>> dev value and as a result dev_net(dev) is causing a NULL pointer dereference.
>>
>> To resolve this I am replacing the check for skb_dst() with skb_valid_dst()
>> so that we do not attempt to use the metadata dst to retrieve a device in
>> order to determine the network namespace.
>
> Why does UDP layer need to care about tunnel layer things?
>
> If the problem is really what you describe, then the tunnel layer
> should reset that dst after finish.

A metadata dst is inserted by the tunnel layer after it has released
the regular dst that routed the packet to the tunnel.  My
understanding is it is used by OVS to handle the routing of the packet
after it has come out of the tunnel but before it has gone up through
the IP stack.  All of this was introduced back around the time of the
lightweight tunnels.

- Alex

[net-next PATCH v3] udp: Resolve NULL pointer dereference over flow-based vxlan device

[Alexander Duyck]

While testing an OpenStack configuration using VXLANs I saw the following
call trace:

 RIP: 0010:[<ffffffff815fad49>] udp4_lib_lookup_skb+0x49/0x80
 RSP: 0018:ffff88103867bc50  EFLAGS: 00010286
 RAX: ffff88103269bf00 RBX: ffff88103269bf00 RCX: 00000000ffffffff
 RDX: 0000000000004300 RSI: 0000000000000000 RDI: ffff880f2932e780
 RBP: ffff88103867bc60 R08: 0000000000000000 R09: 000000009001a8c0
 R10: 0000000000004400 R11: ffffffff81333a58 R12: ffff880f2932e794
 R13: 0000000000000014 R14: 0000000000000014 R15: ffffe8efbfd89ca0
 FS:  0000000000000000(0000) GS:ffff88103fd80000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 0000000000000488 CR3: 0000000001c06000 CR4: 00000000001426e0
 Stack:
  ffffffff81576515 ffffffff815733c0 ffff88103867bc98 ffffffff815fcc17
  ffff88103269bf00 ffffe8efbfd89ca0 0000000000000014 0000000000000080
  ffffe8efbfd89ca0 ffff88103867bcc8 ffffffff815fcf8b ffff880f2932e794
 Call Trace:
  [<ffffffff81576515>] ? skb_checksum+0x35/0x50
  [<ffffffff815733c0>] ? skb_push+0x40/0x40
  [<ffffffff815fcc17>] udp_gro_receive+0x57/0x130
  [<ffffffff815fcf8b>] udp4_gro_receive+0x10b/0x2c0
  [<ffffffff81605863>] inet_gro_receive+0x1d3/0x270
  [<ffffffff81589e59>] dev_gro_receive+0x269/0x3b0
  [<ffffffff8158a1b8>] napi_gro_receive+0x38/0x120
  [<ffffffffa0871297>] gro_cell_poll+0x57/0x80 [vxlan]
  [<ffffffff815899d0>] net_rx_action+0x160/0x380
  [<ffffffff816965c7>] __do_softirq+0xd7/0x2c5
  [<ffffffff8107d969>] run_ksoftirqd+0x29/0x50
  [<ffffffff8109a50f>] smpboot_thread_fn+0x10f/0x160
  [<ffffffff8109a400>] ? sort_range+0x30/0x30
  [<ffffffff81096da8>] kthread+0xd8/0xf0
  [<ffffffff81693c82>] ret_from_fork+0x22/0x40
  [<ffffffff81096cd0>] ? kthread_park+0x60/0x60

The following trace is seen when receiving a DHCP request over a flow-based
VXLAN tunnel.  I believe this is caused by the metadata dst having a NULL
dev value and as a result dev_net(dev) is causing a NULL pointer dereference.

To resolve this I am replacing the check for skb_dst(skb)->dev with just
skb->dev.  This makes sense as the callers of this function are usually in
the receive path and as such skb->dev should always be populated.  In
addition other functions in the area where these are called are already
using dev_net(skb->dev) to determine the namespace the UDP packet belongs
in.

Fixes: 63058308cd55 ("udp: Add udp6_lib_lookup_skb and udp4_lib_lookup_skb")
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---

v2: Replaced skb_dst(skb)->dev == NULL test with skb_valid_dst(skb).
    Added update for IPv6 as well since issue would affect both.
v3: Dropped use of skb_dst(skb) entirely and instead just used skb->dev.

 net/ipv4/udp.c |   10 ++--------
 net/ipv6/udp.c |    8 +++-----
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index f67f52ba4809..2e3ebfe5549e 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -604,7 +604,7 @@ static inline struct sock *__udp4_lib_lookup_skb(struct sk_buff *skb,
 {
 	const struct iphdr *iph = ip_hdr(skb);
 
-	return __udp4_lib_lookup(dev_net(skb_dst(skb)->dev), iph->saddr, sport,
+	return __udp4_lib_lookup(dev_net(skb->dev), iph->saddr, sport,
 				 iph->daddr, dport, inet_iif(skb),
 				 udptable, skb);
 }
@@ -612,13 +612,7 @@ static inline struct sock *__udp4_lib_lookup_skb(struct sk_buff *skb,
 struct sock *udp4_lib_lookup_skb(struct sk_buff *skb,
 				 __be16 sport, __be16 dport)
 {
-	const struct iphdr *iph = ip_hdr(skb);
-	const struct net_device *dev =
-	    skb_dst(skb) ? skb_dst(skb)->dev : skb->dev;
-
-	return __udp4_lib_lookup(dev_net(dev), iph->saddr, sport,
-				 iph->daddr, dport, inet_iif(skb),
-				 &udp_table, skb);
+	return __udp4_lib_lookup_skb(skb, sport, dport, &udp_table);
 }
 EXPORT_SYMBOL_GPL(udp4_lib_lookup_skb);
 
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index aca06094110f..2ba6a77a8815 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -315,13 +315,13 @@ static struct sock *__udp6_lib_lookup_skb(struct sk_buff *skb,
 					  __be16 sport, __be16 dport,
 					  struct udp_table *udptable)
 {
-	struct sock *sk;
 	const struct ipv6hdr *iph = ipv6_hdr(skb);
+	struct sock *sk;
 
 	sk = skb_steal_sock(skb);
 	if (unlikely(sk))
 		return sk;
-	return __udp6_lib_lookup(dev_net(skb_dst(skb)->dev), &iph->saddr, sport,
+	return __udp6_lib_lookup(dev_net(skb->dev), &iph->saddr, sport,
 				 &iph->daddr, dport, inet6_iif(skb),
 				 udptable, skb);
 }
@@ -330,10 +330,8 @@ struct sock *udp6_lib_lookup_skb(struct sk_buff *skb,
 				 __be16 sport, __be16 dport)
 {
 	const struct ipv6hdr *iph = ipv6_hdr(skb);
-	const struct net_device *dev =
-	    skb_dst(skb) ? skb_dst(skb)->dev : skb->dev;
 
-	return __udp6_lib_lookup(dev_net(dev), &iph->saddr, sport,
+	return __udp6_lib_lookup(dev_net(skb->dev), &iph->saddr, sport,
 				 &iph->daddr, dport, inet6_iif(skb),
 				 &udp_table, skb);
 }

Re: [net-next PATCH v3] udp: Resolve NULL pointer dereference over flow-based vxlan device

[Eric Dumazet]

On Thu, 2016-05-12 at 16:23 -0700, Alexander Duyck wrote:
> While testing an OpenStack configuration using VXLANs I saw the following
> call trace:

> The following trace is seen when receiving a DHCP request over a flow-based
> VXLAN tunnel.  I believe this is caused by the metadata dst having a NULL
> dev value and as a result dev_net(dev) is causing a NULL pointer dereference.
> 
> To resolve this I am replacing the check for skb_dst(skb)->dev with just
> skb->dev.  This makes sense as the callers of this function are usually in
> the receive path and as such skb->dev should always be populated.  In
> addition other functions in the area where these are called are already
> using dev_net(skb->dev) to determine the namespace the UDP packet belongs
> in.
> 
> Fixes: 63058308cd55 ("udp: Add udp6_lib_lookup_skb and udp4_lib_lookup_skb")
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---

Acked-by: Eric Dumazet <edumazet@google.com>

Thanks ;)

Re: [net-next PATCH v3] udp: Resolve NULL pointer dereference over flow-based vxlan device

[David Miller]

From: Alexander Duyck <aduyck@mirantis.com>
Date: Thu, 12 May 2016 16:23:44 -0700

> While testing an OpenStack configuration using VXLANs I saw the following
> call trace:
 ...
> The following trace is seen when receiving a DHCP request over a flow-based
> VXLAN tunnel.  I believe this is caused by the metadata dst having a NULL
> dev value and as a result dev_net(dev) is causing a NULL pointer dereference.
> 
> To resolve this I am replacing the check for skb_dst(skb)->dev with just
> skb->dev.  This makes sense as the callers of this function are usually in
> the receive path and as such skb->dev should always be populated.  In
> addition other functions in the area where these are called are already
> using dev_net(skb->dev) to determine the namespace the UDP packet belongs
> in.
> 
> Fixes: 63058308cd55 ("udp: Add udp6_lib_lookup_skb and udp4_lib_lookup_skb")
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>

Applied, thanks!