LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] [net] ipv6: sr: fix NULL pointer dereference in seg6_do_srh_encap()- v4 pkts
@ 2018-04-20 13:58 Ahmed Abdelsalam
  2018-04-20 14:38 ` David Lebrun
  2018-04-23  1:06 ` David Miller
  0 siblings, 2 replies; 5+ messages in thread
From: Ahmed Abdelsalam @ 2018-04-20 13:58 UTC (permalink / raw)
  To: davem, dlebrun, kuznet, yoshfuji, netdev, linux-kernel; +Cc: amsalam20

In case of seg6 in encap mode, seg6_do_srh_encap() calls set_tun_src()
in order to set the src addr of outer IPv6 header.

The net_device is required for set_tun_src(). However calling ip6_dst_idev()
on dst_entry in case of IPv4 traffic results on the following bug.

Using just dst->dev should fix this BUG.

[  196.242461] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
[  196.242975] PGD 800000010f076067 P4D 800000010f076067 PUD 10f060067 PMD 0
[  196.243329] Oops: 0000 [#1] SMP PTI
[  196.243468] Modules linked in: nfsd auth_rpcgss nfs_acl nfs lockd grace fscache sunrpc crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc aesni_intel aes_x86_64 crypto_simd cryptd input_leds glue_helper led_class pcspkr serio_raw mac_hid video autofs4 hid_generic usbhid hid e1000 i2c_piix4 ahci pata_acpi libahci
[  196.244362] CPU: 2 PID: 1089 Comm: ping Not tainted 4.16.0+ #1
[  196.244606] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[  196.244968] RIP: 0010:seg6_do_srh_encap+0x1ac/0x300
[  196.245236] RSP: 0018:ffffb2ce00b23a60 EFLAGS: 00010202
[  196.245464] RAX: 0000000000000000 RBX: ffff8c7f53eea300 RCX: 0000000000000000
[  196.245742] RDX: 0000f10000000000 RSI: ffff8c7f52085a6c RDI: ffff8c7f41166850
[  196.246018] RBP: ffffb2ce00b23aa8 R08: 00000000000261e0 R09: ffff8c7f41166800
[  196.246294] R10: ffffdce5040ac780 R11: ffff8c7f41166828 R12: ffff8c7f41166808
[  196.246570] R13: ffff8c7f52085a44 R14: ffffffffb73211c0 R15: ffff8c7e69e44200
[  196.246846] FS:  00007fc448789700(0000) GS:ffff8c7f59d00000(0000) knlGS:0000000000000000
[  196.247286] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  196.247526] CR2: 0000000000000000 CR3: 000000010f05a000 CR4: 00000000000406e0
[  196.247804] Call Trace:
[  196.247972]  seg6_do_srh+0x15b/0x1c0
[  196.248156]  seg6_output+0x3c/0x220
[  196.248341]  ? prandom_u32+0x14/0x20
[  196.248526]  ? ip_idents_reserve+0x6c/0x80
[  196.248723]  ? __ip_select_ident+0x90/0x100
[  196.248923]  ? ip_append_data.part.50+0x6c/0xd0
[  196.249133]  lwtunnel_output+0x44/0x70
[  196.249328]  ip_send_skb+0x15/0x40
[  196.249515]  raw_sendmsg+0x8c3/0xac0
[  196.249701]  ? _copy_from_user+0x2e/0x60
[  196.249897]  ? rw_copy_check_uvector+0x53/0x110
[  196.250106]  ? _copy_from_user+0x2e/0x60
[  196.250299]  ? copy_msghdr_from_user+0xce/0x140
[  196.250508]  sock_sendmsg+0x36/0x40
[  196.250690]  ___sys_sendmsg+0x292/0x2a0
[  196.250881]  ? _cond_resched+0x15/0x30
[  196.251074]  ? copy_termios+0x1e/0x70
[  196.251261]  ? _copy_to_user+0x22/0x30
[  196.251575]  ? tty_mode_ioctl+0x1c3/0x4e0
[  196.251782]  ? _cond_resched+0x15/0x30
[  196.251972]  ? mutex_lock+0xe/0x30
[  196.252152]  ? vvar_fault+0xd2/0x110
[  196.252337]  ? __do_fault+0x1f/0xc0
[  196.252521]  ? __handle_mm_fault+0xc1f/0x12d0
[  196.252727]  ? __sys_sendmsg+0x63/0xa0
[  196.252919]  __sys_sendmsg+0x63/0xa0
[  196.253107]  do_syscall_64+0x72/0x200
[  196.253305]  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
[  196.253530] RIP: 0033:0x7fc4480b0690
[  196.253715] RSP: 002b:00007ffde9f252f8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[  196.254053] RAX: ffffffffffffffda RBX: 0000000000000040 RCX: 00007fc4480b0690
[  196.254331] RDX: 0000000000000000 RSI: 000000000060a360 RDI: 0000000000000003
[  196.254608] RBP: 00007ffde9f253f0 R08: 00000000002d1e81 R09: 0000000000000002
[  196.254884] R10: 00007ffde9f250c0 R11: 0000000000000246 R12: 0000000000b22070
[  196.255205] R13: 20c49ba5e353f7cf R14: 431bde82d7b634db R15: 00007ffde9f278fe
[  196.255484] Code: a5 0f b6 45 c0 41 88 41 28 41 0f b6 41 2c 48 c1 e0 04 49 8b 54 01 38 49 8b 44 01 30 49 89 51 20 49 89 41 18 48 8b 83 b0 00 00 00 <48> 8b 30 49 8b 86 08 0b 00 00 48 8b 40 20 48 8b 50 08 48 0b 10
[  196.256190] RIP: seg6_do_srh_encap+0x1ac/0x300 RSP: ffffb2ce00b23a60
[  196.256445] CR2: 0000000000000000
[  196.256676] ---[ end trace 71af7d093603885c ]---

Fixes: 8936ef7604c11 ipv6: sr: fix NULL pointer dereference when setting encap source address
Signed-off-by: Ahmed Abdelsalam <amsalam20@gmail.com>
---
I tested the patch for IPv6 and IPv4 traffic 

 net/ipv6/seg6_iptunnel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
index f343e6f..5fe1394 100644
--- a/net/ipv6/seg6_iptunnel.c
+++ b/net/ipv6/seg6_iptunnel.c
@@ -136,7 +136,7 @@ int seg6_do_srh_encap(struct sk_buff *skb, struct ipv6_sr_hdr *osrh, int proto)
 	isrh->nexthdr = proto;
 
 	hdr->daddr = isrh->segments[isrh->first_segment];
-	set_tun_src(net, ip6_dst_idev(dst)->dev, &hdr->daddr, &hdr->saddr);
+	set_tun_src(net, dst->dev, &hdr->daddr, &hdr->saddr);
 
 #ifdef CONFIG_IPV6_SEG6_HMAC
 	if (sr_has_hmac(isrh)) {
-- 
2.1.4

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [net] ipv6: sr: fix NULL pointer dereference in seg6_do_srh_encap()- v4 pkts
  2018-04-20 13:58 [PATCH] [net] ipv6: sr: fix NULL pointer dereference in seg6_do_srh_encap()- v4 pkts Ahmed Abdelsalam
@ 2018-04-20 14:38 ` David Lebrun
  2018-04-20 14:46   ` Ahmed Abdelsalam
  2018-04-23  1:06 ` David Miller
  1 sibling, 1 reply; 5+ messages in thread
From: David Lebrun @ 2018-04-20 14:38 UTC (permalink / raw)
  To: Ahmed Abdelsalam, davem, dlebrun, kuznet, yoshfuji, netdev, linux-kernel

On 04/20/2018 02:58 PM, Ahmed Abdelsalam wrote:
> In case of seg6 in encap mode, seg6_do_srh_encap() calls set_tun_src()
> in order to set the src addr of outer IPv6 header.
> 
> The net_device is required for set_tun_src(). However calling ip6_dst_idev()
> on dst_entry in case of IPv4 traffic results on the following bug.
> 
> Using just dst->dev should fix this BUG.
> 

Good catch, thanks for spotting this. If you actually tested your fix 
with IPv4 and IPv6 traffic, you should mention it in the commit message. 
Your current formulation suggests that you just guessed a fix without 
testing.

> 
> Fixes: 8936ef7604c11 ipv6: sr: fix NULL pointer dereference when setting encap source address
> Signed-off-by: Ahmed Abdelsalam<amsalam20@gmail.com>

Acked-by: David Lebrun <dlebrun@google.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [net] ipv6: sr: fix NULL pointer dereference in seg6_do_srh_encap()- v4 pkts
  2018-04-20 14:38 ` David Lebrun
@ 2018-04-20 14:46   ` Ahmed Abdelsalam
  0 siblings, 0 replies; 5+ messages in thread
From: Ahmed Abdelsalam @ 2018-04-20 14:46 UTC (permalink / raw)
  To: David Lebrun; +Cc: davem, dlebrun, kuznet, yoshfuji, netdev, linux-kernel

On Fri, 20 Apr 2018 15:38:08 +0100
David Lebrun <dav.lebrun@gmail.com> wrote:

> On 04/20/2018 02:58 PM, Ahmed Abdelsalam wrote:
> > In case of seg6 in encap mode, seg6_do_srh_encap() calls set_tun_src()
> > in order to set the src addr of outer IPv6 header.
> > 
> > The net_device is required for set_tun_src(). However calling ip6_dst_idev()
> > on dst_entry in case of IPv4 traffic results on the following bug.
> > 
> > Using just dst->dev should fix this BUG.
> > 
> 
> Good catch, thanks for spotting this. If you actually tested your fix 
> with IPv4 and IPv6 traffic, you should mention it in the commit message. 
> Your current formulation suggests that you just guessed a fix without 
> testing.
> 

Yes, I did two tests for both IPv4 and IPv6.
Sorry for this Language Bug. 

> > 
> > Fixes: 8936ef7604c11 ipv6: sr: fix NULL pointer dereference when setting encap source address
> > Signed-off-by: Ahmed Abdelsalam<amsalam20@gmail.com>
> 
> Acked-by: David Lebrun <dlebrun@google.com>


-- 
Ahmed Abdelsalam <amsalam20@gmail.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [net] ipv6: sr: fix NULL pointer dereference in seg6_do_srh_encap()- v4 pkts
  2018-04-20 13:58 [PATCH] [net] ipv6: sr: fix NULL pointer dereference in seg6_do_srh_encap()- v4 pkts Ahmed Abdelsalam
  2018-04-20 14:38 ` David Lebrun
@ 2018-04-23  1:06 ` David Miller
  2018-04-23  6:09   ` Ahmed Abdelsalam
  1 sibling, 1 reply; 5+ messages in thread
From: David Miller @ 2018-04-23  1:06 UTC (permalink / raw)
  To: amsalam20; +Cc: dlebrun, kuznet, yoshfuji, netdev, linux-kernel

From: Ahmed Abdelsalam <amsalam20@gmail.com>
Date: Fri, 20 Apr 2018 15:58:05 +0200

> In case of seg6 in encap mode, seg6_do_srh_encap() calls set_tun_src()
> in order to set the src addr of outer IPv6 header.
> 
> The net_device is required for set_tun_src(). However calling ip6_dst_idev()
> on dst_entry in case of IPv4 traffic results on the following bug.
> 
> Using just dst->dev should fix this BUG.
 ...
> Fixes: 8936ef7604c11 ipv6: sr: fix NULL pointer dereference when setting encap source address

Please format your Fixes: tag properly next time.  The commit header
text should be enclosed by (" ").  I fixed it up for you this time.

> Signed-off-by: Ahmed Abdelsalam <amsalam20@gmail.com>

Applied and queued up for -stable.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [net] ipv6: sr: fix NULL pointer dereference in seg6_do_srh_encap()- v4 pkts
  2018-04-23  1:06 ` David Miller
@ 2018-04-23  6:09   ` Ahmed Abdelsalam
  0 siblings, 0 replies; 5+ messages in thread
From: Ahmed Abdelsalam @ 2018-04-23  6:09 UTC (permalink / raw)
  To: David Miller; +Cc: dlebrun, kuznet, yoshfuji, netdev, linux-kernel

On Sun, 22 Apr 2018 21:06:04 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:

> From: Ahmed Abdelsalam <amsalam20@gmail.com>
> Date: Fri, 20 Apr 2018 15:58:05 +0200
> 
> > In case of seg6 in encap mode, seg6_do_srh_encap() calls set_tun_src()
> > in order to set the src addr of outer IPv6 header.
> > 
> > The net_device is required for set_tun_src(). However calling ip6_dst_idev()
> > on dst_entry in case of IPv4 traffic results on the following bug.
> > 
> > Using just dst->dev should fix this BUG.
>  ...
> > Fixes: 8936ef7604c11 ipv6: sr: fix NULL pointer dereference when setting encap source address
> 
> Please format your Fixes: tag properly next time.  The commit header
> text should be enclosed by (" ").  I fixed it up for you this time.
> 

Ok! 
Thanks David for your time. 

> > Signed-off-by: Ahmed Abdelsalam <amsalam20@gmail.com>
> 
> Applied and queued up for -stable.


-- 
Ahmed Abdelsalam <amsalam20@gmail.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-04-23  6:09 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-20 13:58 [PATCH] [net] ipv6: sr: fix NULL pointer dereference in seg6_do_srh_encap()- v4 pkts Ahmed Abdelsalam
2018-04-20 14:38 ` David Lebrun
2018-04-20 14:46   ` Ahmed Abdelsalam
2018-04-23  1:06 ` David Miller
2018-04-23  6:09   ` Ahmed Abdelsalam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).