Re: [PATCH net] net/sched: act_ct: fix skb leak and crash on ooo frags

On Wed, Jan 3, 2024 at 8:49 PM Jakub Kicinski [off-list ref] wrote:

On Thu, 28 Dec 2023 16:14:57 +0800 Tao Liu wrote:

quoted

act_ct adds skb->users before defragmentation. If frags arrive in order,
the last frag's reference is reset in:

  inet_frag_reasm_prepare
    skb_morph

which is not straightforward.

However when frags arrive out of order, nobody unref the last frag, and
all frags are leaked. The situation is even worse, as initiating packet
capture can lead to a crash[0] when skb has been cloned and shared at the
same time.

Fix the issue by removing skb_get() before defragmentation. act_ct
returns TC_ACT_CONSUMED when defrag failed or in progress.

Vlad, Xin Long, does this look good to you?

Looks good to me.

It seems that skb_get() must be avoided to use before ip defrag,
and also I see no reason to keep the skb if defrag fails in tcf_ct_act().

nf_ct_handle_fragments() is also called in ovs_ct_handle_fragments()
where it doesn't hold the skb.