| Message ID | 1420738047-16466-1-git-send-email-jon.maloy@ericsson.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Jon Maloy <jon.maloy@ericsson.com> Date: Thu, 8 Jan 2015 12:27:27 -0500 > In commit 58dc55f25631178ee74cd27185956a8f7dcb3e32 ("tipc: use generic > SKB list APIs to manage link transmission queue") we replace all list > traversal loops with the macros skb_queue_walk() or > skb_queue_walk_safe(). While the previous loops were based on the > assumption that the list was NULL-terminated, the standard macros > stop when the iterator reaches the list head, which is non-NULL. > > In the function bclink_retransmit_pkt() this macro replacement has > lead to a bug. When we receive a BCAST STATE_MSG we unconditionally > call the function bclink_retransmit_pkt(), whether there really is > anything to retransmit or not, assuming that the sequence number > comparisons will lead to the correct behavior. However, if the > transmission queue is empty, or if there are no eligible buffers in > the transmission queue, we will by mistake pass the list head pointer > to the function tipc_link_retransmit(). Since the list head is not a > valid sk_buff, this leads to a crash. > > In this commit we fix this by only calling tipc_link_retransmit() > if we actually found eligible buffers in the transmission queue. > > Reviewed-by: Ying Xue <ying.xue@windriver.com> > Signed-off-by: Jon Maloy <jon.maloy@ericsson.com> Applied, thanks Jon. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c index 96ceefe..a9e174f 100644 --- a/net/tipc/bcast.c +++ b/net/tipc/bcast.c @@ -220,10 +220,11 @@ static void bclink_retransmit_pkt(u32 after, u32 to) struct sk_buff *skb; skb_queue_walk(&bcl->outqueue, skb) { - if (more(buf_seqno(skb), after)) + if (more(buf_seqno(skb), after)) { + tipc_link_retransmit(bcl, skb, mod(to - after)); break; + } } - tipc_link_retransmit(bcl, skb, mod(to - after)); } /**
In commit 58dc55f25631178ee74cd27185956a8f7dcb3e32 ("tipc: use generic SKB list APIs to manage link transmission queue") we replace all list traversal loops with the macros skb_queue_walk() or skb_queue_walk_safe(). While the previous loops were based on the assumption that the list was NULL-terminated, the standard macros stop when the iterator reaches the list head, which is non-NULL. In the function bclink_retransmit_pkt() this macro replacement has lead to a bug. When we receive a BCAST STATE_MSG we unconditionally call the function bclink_retransmit_pkt(), whether there really is anything to retransmit or not, assuming that the sequence number comparisons will lead to the correct behavior. However, if the transmission queue is empty, or if there are no eligible buffers in the transmission queue, we will by mistake pass the list head pointer to the function tipc_link_retransmit(). Since the list head is not a valid sk_buff, this leads to a crash. In this commit we fix this by only calling tipc_link_retransmit() if we actually found eligible buffers in the transmission queue. Reviewed-by: Ying Xue <ying.xue@windriver.com> Signed-off-by: Jon Maloy <jon.maloy@ericsson.com> --- net/tipc/bcast.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)