Re: [PATCH net] net: mctp: free reassembly head skb when key_add fails on SOM

Hi Jeremy,

[merging two threads here; Guikin reported the same issue, around 12
hours earlier]

Thanks, I hadn't seen Guikun's report. If a v2 is wanted I'll add:

Reported-by: Guikun Yang <redacted>
Closes: https://lore.kernel.org/all/tencent_43A65D51143E18518EA968DF9323694EFD08@qq.com/ (local)

We could just defer setting skb to NULL until the mctp_key_add() has
succeeded; there's already the common error path that cleans up any
remaining skb.

I think that one needs care. For the first fragment, mctp_frag_queue()
stores skb_unclone_full(skb) in key->reasm_head, and if skb is a clone,
skb_unclone_full() frees it and returns a copy. The kfree_skb(skb) at
out: would then free the original a second time, and the copy in
reasm_head would still leak. Taking skb back from key->reasm_head on
failure would work, but the explicit free looked simpler to me.

Alternatively, we could change the semantics mctp_key_unref() to also
free any pending reassembly, but I'm okay with the current approach of
being explicit about the state handling.

Then I'll keep it as is. Let me know if you want a v2 with the tags
above.

Thanks,
Joas