Skip to content

Commit 30d628e

Browse files
Tung Nguyengregkh
authored andcommitted
tipc: fix memory leak caused by tipc_buf_append()
[ Upstream commit ceb1eb2 ] Commit ed42989 ("tipc: 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, the pointer to the fragment is set to NULL before calling skb_unshare() to make sure that the original skb is not freed after freeing the fragment 2 times in case skb_unshare() returns NULL. Fixes: ed42989 ("tipc: fix the skb_unshare() in tipc_buf_append()") Acked-by: Jon Maloy <[email protected]> Reported-by: Thang Hoang Ngo <[email protected]> Signed-off-by: Tung Nguyen <[email protected]> Reviewed-by: Xin Long <[email protected]> Acked-by: Cong Wang <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 8cc351a commit 30d628e

File tree

1 file changed

+2
-3
lines changed

1 file changed

+2
-3
lines changed

net/tipc/msg.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,12 +140,11 @@ int tipc_buf_append(struct sk_buff **headbuf, struct sk_buff **buf)
140140
if (fragid == FIRST_FRAGMENT) {
141141
if (unlikely(head))
142142
goto err;
143-
if (skb_cloned(frag))
144-
frag = skb_copy(frag, GFP_ATOMIC);
143+
*buf = NULL;
144+
frag = skb_unshare(frag, GFP_ATOMIC);
145145
if (unlikely(!frag))
146146
goto err;
147147
head = *headbuf = frag;
148-
*buf = NULL;
149148
TIPC_SKB_CB(head)->tail = NULL;
150149
if (skb_is_nonlinear(head)) {
151150
skb_walk_frags(head, tail) {

0 commit comments

Comments
 (0)