Skip to contentSkip to conversation

net: deal wrong skb and failure ret from __tcp_retransmit_skb

PATCH In review Liu, Changcheng 2 participants 3 messages Apr 19, 2018

Subsystems (3)
Networking Network drivers TCP
lore
Share & tools
lore

Liu, Changcheng

author
#0 lore raw

PATCH 1/1 net: deal wrong skb and failure ret from __tcp_retransmit_skb

Hit below panic due to skb is NULL, WARN wrong skb first. if __tcp_retransmit_skb return failure e.g. -EAGAIN, it needn't do further action in tcp_retransmit_skb.

gdb vmlinux
Reading symbols from vmlinux...done.
(gdb) p &((struct tcp_skb_cb *) \
&(((struct sk_buff *)0)->cb[0]))->tcp_gso_segs
$1 = (u16 *) 0x30 <irq_stack_union+48>

[ 9040.917533] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030
[ 9040.926279] IP: tcp_retransmit_skb+0x5c/0xc0
[ 9040.931043] PGD 0 P4D 0
[ 9040.933865] Oops: 0000 [#1] PREEMPT SMP PTI
[ 9040.972151] RIP: 0010:tcp_retransmit_skb+0x5c/0xc0
[ 9040.977496] RSP: 0018:ffff8802bec83e40 EFLAGS: 00010202
[ 9041.062527] Call Trace:
[ 9041.065250]  <IRQ>
[ 9041.067489]  tcp_retransmit_timer+0x481/0x820
[ 9041.077697]  tcp_write_timer_handler+0xe9/0x230
[ 9041.082751]  tcp_write_timer+0x75/0x80
[ 9041.086932]  call_timer_fn+0x29/0x150
[ 9041.091018]  run_timer_softirq+0x411/0x460
[ 9041.105017]  __do_softirq+0x115/0x311
[ 9041.109103]  irq_exit+0xb0/0xc0
[ 9041.112605]  smp_apic_timer_interrupt+0x67/0x140
Signed-off-by Liu Changcheng <[email protected]>
net/ipv4/tcp_output.c +6 -1
index 383cac0..545b9b3 100644
@@ -2920,7 +2920,10 @@ int __tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)
 int tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
-	int err = __tcp_retransmit_skb(sk, skb, segs);
+	int err = 0;
+
+	WARN_ONCE(!skb, "sk_buff is NULL\n");
+	err = __tcp_retransmit_skb(sk, skb, segs);
 
 	if (err == 0) {
 #if FASTRETRANS_DEBUG > 0

@@ -2935,6 +2938,8 @@ int tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)
 		if (!tp->retrans_stamp)
 			tp->retrans_stamp = tcp_skb_timestamp(skb);
 
+	} else {
+		return err;
 	}
 
 	if (tp->undo_retrans < 0)
Signature · 1 line
2.7.4
#1 lore raw
Quoted 27 lines · Changcheng

Hit below panic due to skb is NULL, WARN wrong skb first. if __tcp_retransmit_skb return failure e.g. -EAGAIN, it needn't do further action in tcp_retransmit_skb.

gdb vmlinux
Reading symbols from vmlinux...done.
(gdb) p &((struct tcp_skb_cb *) \
&(((struct sk_buff *)0)->cb[0]))->tcp_gso_segs
$1 = (u16 *) 0x30 <irq_stack_union+48>

[ 9040.917533] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030
[ 9040.926279] IP: tcp_retransmit_skb+0x5c/0xc0
[ 9040.931043] PGD 0 P4D 0
[ 9040.933865] Oops: 0000 [#1] PREEMPT SMP PTI
[ 9040.972151] RIP: 0010:tcp_retransmit_skb+0x5c/0xc0
[ 9040.977496] RSP: 0018:ffff8802bec83e40 EFLAGS: 00010202
[ 9041.062527] Call Trace:
[ 9041.065250]  <IRQ>
[ 9041.067489]  tcp_retransmit_timer+0x481/0x820
[ 9041.077697]  tcp_write_timer_handler+0xe9/0x230
[ 9041.082751]  tcp_write_timer+0x75/0x80
[ 9041.086932]  call_timer_fn+0x29/0x150
[ 9041.091018]  run_timer_softirq+0x411/0x460
[ 9041.105017]  __do_softirq+0x115/0x311
[ 9041.109103]  irq_exit+0xb0/0xc0
[ 9041.112605]  smp_apic_timer_interrupt+0x67/0x140
Quoted 2 lines
Signed-off-by Liu Changcheng <[email protected]>

On which kernel have you seen this problem ?

In networking, we want patches with a 'Fixes:' tag to clearly identify which patch added a regression.

Anyway your patch comes too late I guess

https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/?id=bffd168c3fc5cc7d2bad4c668fa90e7a9010db4b

author
#2 lore raw
Quoted 30 lines · Eric Dumazet
Quoted 27 lines · Changcheng

Hit below panic due to skb is NULL, WARN wrong skb first. if __tcp_retransmit_skb return failure e.g. -EAGAIN, it needn't do further action in tcp_retransmit_skb.

gdb vmlinux
Reading symbols from vmlinux...done.
(gdb) p &((struct tcp_skb_cb *) \
&(((struct sk_buff *)0)->cb[0]))->tcp_gso_segs
$1 = (u16 *) 0x30 <irq_stack_union+48>

[ 9040.917533] BUG: unable to handle kernel NULL pointer dereference at 0000000000000030
[ 9040.926279] IP: tcp_retransmit_skb+0x5c/0xc0
[ 9040.931043] PGD 0 P4D 0
[ 9040.933865] Oops: 0000 [#1] PREEMPT SMP PTI
[ 9040.972151] RIP: 0010:tcp_retransmit_skb+0x5c/0xc0
[ 9040.977496] RSP: 0018:ffff8802bec83e40 EFLAGS: 00010202
[ 9041.062527] Call Trace:
[ 9041.065250]  <IRQ>
[ 9041.067489]  tcp_retransmit_timer+0x481/0x820
[ 9041.077697]  tcp_write_timer_handler+0xe9/0x230
[ 9041.082751]  tcp_write_timer+0x75/0x80
[ 9041.086932]  call_timer_fn+0x29/0x150
[ 9041.091018]  run_timer_softirq+0x411/0x460
[ 9041.105017]  __do_softirq+0x115/0x311
[ 9041.109103]  irq_exit+0xb0/0xc0
[ 9041.112605]  smp_apic_timer_interrupt+0x67/0x140
Quoted 4 lines
Quoted 2 lines
Signed-off-by Liu Changcheng <[email protected]>

On which kernel have you seen this problem ?

[Changcheng]: My kernel is 4.16.0 Fearless Coyote

Quoted 8 lines

In networking, we want patches with a 'Fixes:' tag to clearly identify which patch added a regression.

Anyway your patch comes too late I guess

https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/?id=bffd168c3fc5cc7d2bad4c668fa90e7a9010db4b

[Changcheng]: Check upstream master kernel(head: 87ef12027b9b1), patch isn't merged yet. I'll check it in my side.

Quoted 2 lines