Skip to content
  • Tung Nguyen's avatar
    ceb1eb2f
    tipc: fix memory leak caused by tipc_buf_append() · ceb1eb2f
    Tung Nguyen authored
    
    
    Commit ed42989e ("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: ed42989e ("tipc: fix the skb_unshare() in tipc_buf_append()")
    Acked-by: default avatarJon Maloy <jmaloy@redhat.com>
    Reported-by: default avatarThang Hoang Ngo <thang.h.ngo@dektech.com.au>
    Signed-off-by: default avatarTung Nguyen <tung.q.nguyen@dektech.com.au>
    Reviewed-by: default avatarXin Long <lucien.xin@gmail.com>
    Acked-by: default avatarCong Wang <xiyou.wangcong@gmail.com>
    Link: https://lore.kernel.org/r/20201027032403.1823-1-tung.q.nguyen@dektech.com.au
    
    
    Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
    ceb1eb2f
    tipc: fix memory leak caused by tipc_buf_append()
    Tung Nguyen authored
    
    
    Commit ed42989e ("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: ed42989e ("tipc: fix the skb_unshare() in tipc_buf_append()")
    Acked-by: default avatarJon Maloy <jmaloy@redhat.com>
    Reported-by: default avatarThang Hoang Ngo <thang.h.ngo@dektech.com.au>
    Signed-off-by: default avatarTung Nguyen <tung.q.nguyen@dektech.com.au>
    Reviewed-by: default avatarXin Long <lucien.xin@gmail.com>
    Acked-by: default avatarCong Wang <xiyou.wangcong@gmail.com>
    Link: https://lore.kernel.org/r/20201027032403.1823-1-tung.q.nguyen@dektech.com.au
    
    
    Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
Loading