tcp: highest_sack fix
syzbot easily found a regression added in our latest patches [1]
No longer set tp->highest_sack to the head of the send queue since
this is not logical and error prone.
Only sack processing should maintain the pointer to an skb from rtx queue.
We might in the future only remember the sequence instead of a pointer to skb,
since rb-tree should allow a fast lookup.
[1]
BUG: KASAN: use-after-free in tcp_highest_sack_seq include/net/tcp.h:1706 [inline]
BUG: KASAN: use-after-free in tcp_ack+0x42bb/0x4fd0 net/ipv4/tcp_input.c:3537
Read of size 4 at addr ffff8801c154faa8 by task syz-executor4/12860
CPU: 0 PID: 12860 Comm: syz-executor4 Not tainted 4.14.0-next-20171113+ #41
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:17 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:53
print_address_description+0x73/0x250 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351 [inline]
kasan_report+0x25b/0x340 mm/kasan/report.c:409
__asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:429
tcp_highest_sack_seq include/net/tcp.h:1706 [inline]
tcp_ack+0x42bb/0x4fd0 net/ipv4/tcp_input.c:3537
tcp_rcv_established+0x672/0x18a0 net/ipv4/tcp_input.c:5439
tcp_v4_do_rcv+0x2ab/0x7d0 net/ipv4/tcp_ipv4.c:1468
sk_backlog_rcv include/net/sock.h:909 [inline]
__release_sock+0x124/0x360 net/core/sock.c:2264
release_sock+0xa4/0x2a0 net/core/sock.c:2778
tcp_sendmsg+0x3a/0x50 net/ipv4/tcp.c:1462
inet_sendmsg+0x11f/0x5e0 net/ipv4/af_inet.c:763
sock_sendmsg_nosec net/socket.c:632 [inline]
sock_sendmsg+0xca/0x110 net/socket.c:642
___sys_sendmsg+0x75b/0x8a0 net/socket.c:2048
__sys_sendmsg+0xe5/0x210 net/socket.c:2082
SYSC_sendmsg net/socket.c:2093 [inline]
SyS_sendmsg+0x2d/0x50 net/socket.c:2089
entry_SYSCALL_64_fastpath+0x1f/0x96
RIP: 0033:0x452879
RSP: 002b:00007fc9761bfbe8 EFLAGS: 00000212 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000758020 RCX: 0000000000452879
RDX: 0000000000000000 RSI: 0000000020917fc8 RDI: 0000000000000015
RBP: 0000000000000086 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000212 R12: 00000000006ee3a0
R13: 00000000ffffffff R14: 00007fc9761c06d4 R15: 0000000000000000
Allocated by task 12860:
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:489
kmem_cache_alloc_node+0x144/0x760 mm/slab.c:3638
__alloc_skb+0xf1/0x780 net/core/skbuff.c:193
alloc_skb_fclone include/linux/skbuff.h:1023 [inline]
sk_stream_alloc_skb+0x11d/0x900 net/ipv4/tcp.c:870
tcp_sendmsg_locked+0x1341/0x3b80 net/ipv4/tcp.c:1299
tcp_sendmsg+0x2f/0x50 net/ipv4/tcp.c:1461
inet_sendmsg+0x11f/0x5e0 net/ipv4/af_inet.c:763
sock_sendmsg_nosec net/socket.c:632 [inline]
sock_sendmsg+0xca/0x110 net/socket.c:642
SYSC_sendto+0x358/0x5a0 net/socket.c:1749
SyS_sendto+0x40/0x50 net/socket.c:1717
entry_SYSCALL_64_fastpath+0x1f/0x96
Freed by task 12860:
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_slab_free+0x71/0xc0 mm/kasan/kasan.c:524
__cache_free mm/slab.c:3492 [inline]
kmem_cache_free+0x77/0x280 mm/slab.c:3750
kfree_skbmem+0xdd/0x1d0 net/core/skbuff.c:603
__kfree_skb+0x1d/0x20 net/core/skbuff.c:642
sk_wmem_free_skb include/net/sock.h:1419 [inline]
tcp_rtx_queue_unlink_and_free include/net/tcp.h:1682 [inline]
tcp_clean_rtx_queue net/ipv4/tcp_input.c:3111 [inline]
tcp_ack+0x1b17/0x4fd0 net/ipv4/tcp_input.c:3593
tcp_rcv_established+0x672/0x18a0 net/ipv4/tcp_input.c:5439
tcp_v4_do_rcv+0x2ab/0x7d0 net/ipv4/tcp_ipv4.c:1468
sk_backlog_rcv include/net/sock.h:909 [inline]
__release_sock+0x124/0x360 net/core/sock.c:2264
release_sock+0xa4/0x2a0 net/core/sock.c:2778
tcp_sendmsg+0x3a/0x50 net/ipv4/tcp.c:1462
inet_sendmsg+0x11f/0x5e0 net/ipv4/af_inet.c:763
sock_sendmsg_nosec net/socket.c:632 [inline]
sock_sendmsg+0xca/0x110 net/socket.c:642
___sys_sendmsg+0x75b/0x8a0 net/socket.c:2048
__sys_sendmsg+0xe5/0x210 net/socket.c:2082
SYSC_sendmsg net/socket.c:2093 [inline]
SyS_sendmsg+0x2d/0x50 net/socket.c:2089
entry_SYSCALL_64_fastpath+0x1f/0x96
The buggy address belongs to the object at ffff8801c154fa80
which belongs to the cache skbuff_fclone_cache of size 456
The buggy address is located 40 bytes inside of
456-byte region [ffff8801c154fa80, ffff8801c154fc48)
The buggy address belongs to the page:
page:ffffea00070553c0 count:1 mapcount:0 mapping:ffff8801c154f080 index:0x0
flags: 0x2fffc0000000100(slab)
raw: 02fffc0000000100 ffff8801c154f080 0000000000000000 0000000100000006
raw: ffffea00070a5a20 ffffea0006a18360 ffff8801d9ca0500 0000000000000000
page dumped because: kasan: bad access detected
Fixes: 737ff31456
("tcp: use sequence distance to detect reordering")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
parent
fd7eafd021
commit
50895b9de1
|
@ -1630,9 +1630,6 @@ static inline void tcp_check_send_head(struct sock *sk, struct sk_buff *skb_unli
|
|||
{
|
||||
if (tcp_write_queue_empty(sk))
|
||||
tcp_chrono_stop(sk, TCP_CHRONO_BUSY);
|
||||
|
||||
if (tcp_sk(sk)->highest_sack == skb_unlinked)
|
||||
tcp_sk(sk)->highest_sack = NULL;
|
||||
}
|
||||
|
||||
static inline void __tcp_add_write_queue_tail(struct sock *sk, struct sk_buff *skb)
|
||||
|
@ -1645,12 +1642,8 @@ static inline void tcp_add_write_queue_tail(struct sock *sk, struct sk_buff *skb
|
|||
__tcp_add_write_queue_tail(sk, skb);
|
||||
|
||||
/* Queue it, remembering where we must start sending. */
|
||||
if (sk->sk_write_queue.next == skb) {
|
||||
if (sk->sk_write_queue.next == skb)
|
||||
tcp_chrono_start(sk, TCP_CHRONO_BUSY);
|
||||
|
||||
if (tcp_sk(sk)->highest_sack == NULL)
|
||||
tcp_sk(sk)->highest_sack = skb;
|
||||
}
|
||||
}
|
||||
|
||||
/* Insert new before skb on the write queue of sk. */
|
||||
|
@ -1708,9 +1701,7 @@ static inline u32 tcp_highest_sack_seq(struct tcp_sock *tp)
|
|||
|
||||
static inline void tcp_advance_highest_sack(struct sock *sk, struct sk_buff *skb)
|
||||
{
|
||||
struct sk_buff *next = skb_rb_next(skb);
|
||||
|
||||
tcp_sk(sk)->highest_sack = next ?: tcp_send_head(sk);
|
||||
tcp_sk(sk)->highest_sack = skb_rb_next(skb);
|
||||
}
|
||||
|
||||
static inline struct sk_buff *tcp_highest_sack(struct sock *sk)
|
||||
|
@ -1720,9 +1711,7 @@ static inline struct sk_buff *tcp_highest_sack(struct sock *sk)
|
|||
|
||||
static inline void tcp_highest_sack_reset(struct sock *sk)
|
||||
{
|
||||
struct sk_buff *skb = tcp_rtx_queue_head(sk);
|
||||
|
||||
tcp_sk(sk)->highest_sack = skb ?: tcp_send_head(sk);
|
||||
tcp_sk(sk)->highest_sack = tcp_rtx_queue_head(sk);
|
||||
}
|
||||
|
||||
/* Called when old skb is about to be deleted and replaced by new skb */
|
||||
|
|
|
@ -3534,7 +3534,7 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
|
|||
icsk->icsk_retransmits = 0;
|
||||
}
|
||||
|
||||
prior_fack = tcp_highest_sack_seq(tp);
|
||||
prior_fack = tcp_is_sack(tp) ? tcp_highest_sack_seq(tp) : tp->snd_una;
|
||||
rs.prior_in_flight = tcp_packets_in_flight(tp);
|
||||
|
||||
/* ts_recent update must be made after we are sure that the packet
|
||||
|
|
Loading…
Reference in New Issue
Block a user