Thread (9 messages) flat view 9 messages, 2 authors, 2018-03-24
STALE3098d

Revision v1 of 84 in this series.

Revisions (84)
  1. v1 [diff vs current]
  2. v1 [diff vs current]
  3. v1 [diff vs current]
  4. v1 [diff vs current]
  5. v1 [diff vs current]
  6. v1 [diff vs current]
  7. v1 [diff vs current]
  8. v1 [diff vs current]
  9. v1 [diff vs current]
  10. v1 [diff vs current]
  11. v1 [diff vs current]
  12. v1 [diff vs current]
  13. v1 [diff vs current]
  14. v1 [diff vs current]
  15. v1 [diff vs current]
  16. v1 [diff vs current]
  17. v1 [diff vs current]
  18. v1 [diff vs current]
  19. v1 [diff vs current]
  20. v1 [diff vs current]
  21. v1 current
  22. v1 [diff vs current]
  23. v1 [diff vs current]
  24. v1 [diff vs current]
  25. v1 [diff vs current]
  26. v1 [diff vs current]
  27. v1 [diff vs current]
  28. v1 [diff vs current]
  29. v1 [diff vs current]
  30. v1 [diff vs current]
  31. v1 [diff vs current]
  32. v1 [diff vs current]
  33. v1 [diff vs current]
  34. v1 [diff vs current]
  35. v1 [diff vs current]
  36. v1 [diff vs current]
  37. v1 [diff vs current]
  38. v1 [diff vs current]
  39. v1 [diff vs current]
  40. v1 [diff vs current]
  41. v1 [diff vs current]
  42. v1 [diff vs current]
  43. v1 [diff vs current]
  44. v1 [diff vs current]
  45. v1 [diff vs current]
  46. v1 [diff vs current]
  47. v1 [diff vs current]
  48. v1 [diff vs current]
  49. v1 [diff vs current]
  50. v1 [diff vs current]
  51. v1 [diff vs current]
  52. v1 [diff vs current]
  53. v2 [diff vs current]
  54. v1 [diff vs current]
  55. v1 [diff vs current]
  56. v1 [diff vs current]
  57. v1 [diff vs current]
  58. v1 [diff vs current]
  59. v1 [diff vs current]
  60. v1 [diff vs current]
  61. v1 [diff vs current]
  62. v1 [diff vs current]
  63. v1 [diff vs current]
  64. v1 [diff vs current]
  65. v2 [diff vs current]
  66. v1 [diff vs current]
  67. v2 [diff vs current]
  68. v1 [diff vs current]
  69. v1 [diff vs current]
  70. v1 [diff vs current]
  71. v2 [diff vs current]
  72. v1 [diff vs current]
  73. v1 [diff vs current]
  74. v2 [diff vs current]
  75. v1 [diff vs current]
  76. v2 [diff vs current]
  77. v1 [diff vs current]
  78. v1 [diff vs current]
  79. v1 [diff vs current]
  80. v1 [diff vs current]
  81. v2 [diff vs current]
  82. v1 [diff vs current]
  83. v1 [diff vs current]
  84. v2 [diff vs current]

[PATCH 7/7] netfilter: nf_socket: Fix out of bounds access in nf_sk_lookup_slow_v{4,6}

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: 2018-03-24 20:34:35
Subsystem: networking [general], the rest · Maintainers: "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds

From: Subash Abhinov Kasiviswanathan <redacted>

skb_header_pointer will copy data into a buffer if data is non linear,
otherwise it will return a pointer in the linear section of the data.
nf_sk_lookup_slow_v{4,6} always copies data of size udphdr but later
accesses memory within the size of tcphdr (th->doff) in case of TCP
packets. This causes a crash when running with KASAN with the following
call stack -

BUG: KASAN: stack-out-of-bounds in xt_socket_lookup_slow_v4+0x524/0x718
net/netfilter/xt_socket.c:178
Read of size 2 at addr ffffffe3d417a87c by task syz-executor/28971
CPU: 2 PID: 28971 Comm: syz-executor Tainted: G    B   W  O    4.9.65+ #1
Call trace:
[<ffffff9467e8d390>] dump_backtrace+0x0/0x428 arch/arm64/kernel/traps.c:76
[<ffffff9467e8d7e0>] show_stack+0x28/0x38 arch/arm64/kernel/traps.c:226
[<ffffff946842d9b8>] __dump_stack lib/dump_stack.c:15 [inline]
[<ffffff946842d9b8>] dump_stack+0xd4/0x124 lib/dump_stack.c:51
[<ffffff946811d4b0>] print_address_description+0x68/0x258 mm/kasan/report.c:248
[<ffffff946811d8c8>] kasan_report_error mm/kasan/report.c:347 [inline]
[<ffffff946811d8c8>] kasan_report.part.2+0x228/0x2f0 mm/kasan/report.c:371
[<ffffff946811df44>] kasan_report+0x5c/0x70 mm/kasan/report.c:372
[<ffffff946811bebc>] check_memory_region_inline mm/kasan/kasan.c:308 [inline]
[<ffffff946811bebc>] __asan_load2+0x84/0x98 mm/kasan/kasan.c:739
[<ffffff94694d6f04>] __tcp_hdrlen include/linux/tcp.h:35 [inline]
[<ffffff94694d6f04>] xt_socket_lookup_slow_v4+0x524/0x718 net/netfilter/xt_socket.c:178

Fix this by copying data into appropriate size headers based on protocol.

Fixes: a583636a83ea ("inet: refactor inet[6]_lookup functions to take skb")
Signed-off-by: Tejaswi Tanikella <redacted>
Signed-off-by: Subash Abhinov Kasiviswanathan <redacted>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv4/netfilter/nf_socket_ipv4.c | 6 ++++--
 net/ipv6/netfilter/nf_socket_ipv6.c | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/netfilter/nf_socket_ipv4.c b/net/ipv4/netfilter/nf_socket_ipv4.c
index e9293bdebba0..4824b1e183a1 100644
--- a/net/ipv4/netfilter/nf_socket_ipv4.c
+++ b/net/ipv4/netfilter/nf_socket_ipv4.c
@@ -108,10 +108,12 @@ struct sock *nf_sk_lookup_slow_v4(struct net *net, const struct sk_buff *skb,
 	int doff = 0;
 
 	if (iph->protocol == IPPROTO_UDP || iph->protocol == IPPROTO_TCP) {
-		struct udphdr _hdr, *hp;
+		struct tcphdr _hdr;
+		struct udphdr *hp;
 
 		hp = skb_header_pointer(skb, ip_hdrlen(skb),
-					sizeof(_hdr), &_hdr);
+					iph->protocol == IPPROTO_UDP ?
+					sizeof(*hp) : sizeof(_hdr), &_hdr);
 		if (hp == NULL)
 			return NULL;
 
diff --git a/net/ipv6/netfilter/nf_socket_ipv6.c b/net/ipv6/netfilter/nf_socket_ipv6.c
index ebb2bf84232a..f14de4b6d639 100644
--- a/net/ipv6/netfilter/nf_socket_ipv6.c
+++ b/net/ipv6/netfilter/nf_socket_ipv6.c
@@ -116,9 +116,11 @@ struct sock *nf_sk_lookup_slow_v6(struct net *net, const struct sk_buff *skb,
 	}
 
 	if (tproto == IPPROTO_UDP || tproto == IPPROTO_TCP) {
-		struct udphdr _hdr, *hp;
+		struct tcphdr _hdr;
+		struct udphdr *hp;
 
-		hp = skb_header_pointer(skb, thoff, sizeof(_hdr), &_hdr);
+		hp = skb_header_pointer(skb, thoff, tproto == IPPROTO_UDP ?
+					sizeof(*hp) : sizeof(_hdr), &_hdr);
 		if (hp == NULL)
 			return NULL;
 
-- 
2.11.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help