* [PATCH v2 net-next] tipc: correctly unlink packets from deferred queue
@ 2013-12-17 12:08 erik.hugne
2013-12-17 17:27 ` Paul Gortmaker
0 siblings, 1 reply; 2+ messages in thread
From: erik.hugne @ 2013-12-17 12:08 UTC (permalink / raw)
To: netdev, jon.maloy; +Cc: ying.xue, paul.gortmaker, tipc-discussion, Erik Hugne
From: Erik Hugne <erik.hugne@ericsson.com>
When we pull a packet from the deferred queue, the next
pointer for the current packet being processed might still
refer to deferred packets. This is incorrect, and will
lead to an oops if the last fragment have once been put on
the deferred queue, and at least one packet have been
deferred after this fragment. What we have seen as a result
of this is that after successful delivery of a fragmented
message, the last packet in the fragment chain will point
into the deferred queue. When we later free the chain,
kfree_skb_list will also free packets from the defer-queue.
It is our understanding that this problem has always existed,
however, with the recent change of commit 40ba3cdf5
("tipc: message reassembly using fragment chain"), the window
for it possibly happening has increased.
We fix this by clearing the next pointer for the current
packet being processed.
general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC
Modules linked in: tipc
CPU: 4 PID: 0 Comm: swapper/4 Tainted: G W
3.13.0-rc2+ #6
Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
task: ffff880017af4880 ti: ffff880017aee000 task.ti:
ffff880017aee000
RIP: 0010:[<ffffffff81710694>] [<ffffffff81710694>]
skb_try_coalesce+0x44/0x3d0
RSP: 0018:ffff880016603a78 EFLAGS: 00010212
RAX: 6b6b6b6bd6d6d6d6 RBX: ffff880013106ac0 RCX:
ffff880016603ad0
RDX: ffff880016603ad7 RSI: ffff88001223ed00 RDI:
ffff880013106ac0
RBP: ffff880016603ab8 R08: 0000000000000000 R09:
0000000000000000
R10: 0000000000000001 R11: 0000000000000000 R12:
ffff88001223ed00
R13: ffff880016603ad0 R14: 000000000000058c R15:
ffff880012297650
FS: 0000000000000000(0000) GS:ffff880016600000(0000)
knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 000000000805b000 CR3: 0000000011f5d000 CR4:
00000000000006e0
Stack:
ffff880016603a88 ffffffff810a38ed ffff880016603aa8
ffff88001223ed00
0000000000000001 ffff880012297648 ffff880016603b68
ffff880012297650
ffff880016603b08 ffffffffa0006c51 ffff880016603b08
00ffffffa00005fc
Call Trace:
<IRQ>
[<ffffffff810a38ed>] ? trace_hardirqs_on+0xd/0x10
[<ffffffffa0006c51>] tipc_link_recv_fragment+0xd1/0x1b0 [tipc]
[<ffffffffa0007214>] tipc_recv_msg+0x4e4/0x920 [tipc]
[<ffffffffa00016f0>] ? tipc_l2_rcv_msg+0x40/0x250 [tipc]
[<ffffffffa000177c>] tipc_l2_rcv_msg+0xcc/0x250 [tipc]
[<ffffffffa00016f0>] ? tipc_l2_rcv_msg+0x40/0x250 [tipc]
[<ffffffff8171e65b>] __netif_receive_skb_core+0x80b/0xd00
[<ffffffff8171df94>] ? __netif_receive_skb_core+0x144/0xd00
[<ffffffff8171eb76>] __netif_receive_skb+0x26/0x70
[<ffffffff8171ed6d>] netif_receive_skb+0x2d/0x200
[<ffffffff8171fe70>] napi_gro_receive+0xb0/0x130
[<ffffffff815647c2>] e1000_clean_rx_irq+0x2c2/0x530
[<ffffffff81565986>] e1000_clean+0x266/0x9c0
[<ffffffff81985f7b>] ? notifier_call_chain+0x2b/0x160
[<ffffffff8171f971>] net_rx_action+0x141/0x310
[<ffffffff81051c1b>] __do_softirq+0xeb/0x480
[<ffffffff819817bb>] ? _raw_spin_unlock+0x2b/0x40
[<ffffffff810b8c42>] ? handle_fasteoi_irq+0x72/0x100
[<ffffffff81052346>] irq_exit+0x96/0xc0
[<ffffffff8198cbc3>] do_IRQ+0x63/0xe0
[<ffffffff81981def>] common_interrupt+0x6f/0x6f
<EOI>
Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
Reported-by: Ying Xue <ying.xue@windriver.com>
---
v2: revised commit message based on comments from Paul G.
net/tipc/link.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 3d73144..447e2c4 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1444,6 +1444,7 @@ void tipc_recv_msg(struct sk_buff *head, struct tipc_bearer *b_ptr)
int type;
head = head->next;
+ buf->next = NULL;
/* Ensure bearer is still enabled */
if (unlikely(!b_ptr->active))
--
1.7.9.5
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2 net-next] tipc: correctly unlink packets from deferred queue
2013-12-17 12:08 [PATCH v2 net-next] tipc: correctly unlink packets from deferred queue erik.hugne
@ 2013-12-17 17:27 ` Paul Gortmaker
0 siblings, 0 replies; 2+ messages in thread
From: Paul Gortmaker @ 2013-12-17 17:27 UTC (permalink / raw)
To: erik.hugne, netdev, jon.maloy; +Cc: ying.xue, tipc-discussion
On 13-12-17 07:08 AM, erik.hugne@ericsson.com wrote:
> From: Erik Hugne <erik.hugne@ericsson.com>
>
> When we pull a packet from the deferred queue, the next
> pointer for the current packet being processed might still
> refer to deferred packets. This is incorrect, and will
> lead to an oops if the last fragment have once been put on
> the deferred queue, and at least one packet have been
> deferred after this fragment. What we have seen as a result
> of this is that after successful delivery of a fragmented
> message, the last packet in the fragment chain will point
> into the deferred queue. When we later free the chain,
> kfree_skb_list will also free packets from the defer-queue.
> It is our understanding that this problem has always existed,
> however, with the recent change of commit 40ba3cdf5
> ("tipc: message reassembly using fragment chain"), the window
> for it possibly happening has increased.
For the above:
se tw=68
gqap
In addition to that, adding a paragraph break between the problem
description and the symptom description, and the impacted version
description would improve readability nicely (and then gqap each
paragraph, of course.)
>
> We fix this by clearing the next pointer for the current
> packet being processed.
The below oops appears to be mailer mangled, with extra line
wrapping introduced where there should be none. Please
fix and resend. Ideally use git send-email vs. composing
in a GUI will avoid such things in the future.
Thanks,
Paul.
--
>
> general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC
> Modules linked in: tipc
> CPU: 4 PID: 0 Comm: swapper/4 Tainted: G W
> 3.13.0-rc2+ #6
> Hardware name: Bochs Bochs, BIOS Bochs 01/01/2007
> task: ffff880017af4880 ti: ffff880017aee000 task.ti:
> ffff880017aee000
> RIP: 0010:[<ffffffff81710694>] [<ffffffff81710694>]
> skb_try_coalesce+0x44/0x3d0
> RSP: 0018:ffff880016603a78 EFLAGS: 00010212
> RAX: 6b6b6b6bd6d6d6d6 RBX: ffff880013106ac0 RCX:
> ffff880016603ad0
> RDX: ffff880016603ad7 RSI: ffff88001223ed00 RDI:
> ffff880013106ac0
> RBP: ffff880016603ab8 R08: 0000000000000000 R09:
> 0000000000000000
> R10: 0000000000000001 R11: 0000000000000000 R12:
> ffff88001223ed00
> R13: ffff880016603ad0 R14: 000000000000058c R15:
> ffff880012297650
> FS: 0000000000000000(0000) GS:ffff880016600000(0000)
> knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 000000000805b000 CR3: 0000000011f5d000 CR4:
> 00000000000006e0
> Stack:
> ffff880016603a88 ffffffff810a38ed ffff880016603aa8
> ffff88001223ed00
> 0000000000000001 ffff880012297648 ffff880016603b68
> ffff880012297650
> ffff880016603b08 ffffffffa0006c51 ffff880016603b08
> 00ffffffa00005fc
> Call Trace:
> <IRQ>
> [<ffffffff810a38ed>] ? trace_hardirqs_on+0xd/0x10
> [<ffffffffa0006c51>] tipc_link_recv_fragment+0xd1/0x1b0 [tipc]
> [<ffffffffa0007214>] tipc_recv_msg+0x4e4/0x920 [tipc]
> [<ffffffffa00016f0>] ? tipc_l2_rcv_msg+0x40/0x250 [tipc]
> [<ffffffffa000177c>] tipc_l2_rcv_msg+0xcc/0x250 [tipc]
> [<ffffffffa00016f0>] ? tipc_l2_rcv_msg+0x40/0x250 [tipc]
> [<ffffffff8171e65b>] __netif_receive_skb_core+0x80b/0xd00
> [<ffffffff8171df94>] ? __netif_receive_skb_core+0x144/0xd00
> [<ffffffff8171eb76>] __netif_receive_skb+0x26/0x70
> [<ffffffff8171ed6d>] netif_receive_skb+0x2d/0x200
> [<ffffffff8171fe70>] napi_gro_receive+0xb0/0x130
> [<ffffffff815647c2>] e1000_clean_rx_irq+0x2c2/0x530
> [<ffffffff81565986>] e1000_clean+0x266/0x9c0
> [<ffffffff81985f7b>] ? notifier_call_chain+0x2b/0x160
> [<ffffffff8171f971>] net_rx_action+0x141/0x310
> [<ffffffff81051c1b>] __do_softirq+0xeb/0x480
> [<ffffffff819817bb>] ? _raw_spin_unlock+0x2b/0x40
> [<ffffffff810b8c42>] ? handle_fasteoi_irq+0x72/0x100
> [<ffffffff81052346>] irq_exit+0x96/0xc0
> [<ffffffff8198cbc3>] do_IRQ+0x63/0xe0
> [<ffffffff81981def>] common_interrupt+0x6f/0x6f
> <EOI>
>
> Signed-off-by: Erik Hugne <erik.hugne@ericsson.com>
> Reported-by: Ying Xue <ying.xue@windriver.com>
> ---
> v2: revised commit message based on comments from Paul G.
>
> net/tipc/link.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/tipc/link.c b/net/tipc/link.c
> index 3d73144..447e2c4 100644
> --- a/net/tipc/link.c
> +++ b/net/tipc/link.c
> @@ -1444,6 +1444,7 @@ void tipc_recv_msg(struct sk_buff *head, struct tipc_bearer *b_ptr)
> int type;
>
> head = head->next;
> + buf->next = NULL;
>
> /* Ensure bearer is still enabled */
> if (unlikely(!b_ptr->active))
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-12-17 17:26 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-17 12:08 [PATCH v2 net-next] tipc: correctly unlink packets from deferred queue erik.hugne
2013-12-17 17:27 ` Paul Gortmaker
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).