Commit ed42989eab57 ("fix the skb_unshare() in tipc_buf_append()")
replaced skb_unshare() with skb_copy() to not reduce the data reference
counter of the original skb intentionally. This is not the correct
way to handle the cloned skb because it causes memory leak in 2
following cases:
1/ Sending multicast messages via broadcast link
The original skb list is cloned to the local skb list for local
destination. After that, the data reference counter of each skb
in the original list has the value of 2. This causes each skb not
to be freed after receiving ACK:
tipc_link_advance_transmq()
{
...
/* release skb */
__skb_unlink(skb, &l->transmq);
kfree_skb(skb); <-- memory exists after being freed
}
2/ Sending multicast messages via replicast link
Similar to the above case, each skb cannot be freed after purging
the skb list:
tipc_mcast_xmit()
{
...
__skb_queue_purge(pkts); <-- memory exists after being freed
}
This commit fixes this issue by using skb_unshare() instead. Besides,
to avoid use-after-free error reported by KASAN, kfree_skb(head) in
tipc_buf_reassemble() is called only if the pointer to the appending skb
is not NULL.
Fixes: ed42989eab57 ("fix the skb_unshare() in tipc_buf_append()")
Acked-by: Jon Maloy <jma...@redhat.com>
Reported-by: Thang Hoang Ngo <thang.h....@dektech.com.au>
Signed-off-by: Tung Nguyen <tung.q.ngu...@dektech.com.au>
---
net/tipc/msg.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index 2a78aa701572..46c36c5093de 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -150,8 +150,7 @@ int tipc_buf_append(struct sk_buff **headbuf, struct
sk_buff **buf)
if (fragid == FIRST_FRAGMENT) {
if (unlikely(head))
goto err;
- if (skb_cloned(frag))
- frag = skb_copy(frag, GFP_ATOMIC);
+ frag = skb_unshare(frag, GFP_ATOMIC);
if (unlikely(!frag))
goto err;
head = *headbuf = frag;
@@ -797,7 +796,8 @@ bool tipc_msg_reassemble(struct sk_buff_head *list, struct
sk_buff_head *rcvq)
return true;
error:
pr_warn("Failed do clone local mcast rcv buffer\n");
- kfree_skb(head);
+ if (head)
+ kfree_skb(head);
return false;
}
--
2.17.1
