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

[Joas Antonio dos Santos <joasantonio108@gmail.com>]

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 <yangguikun19@qq.com>
Closes: https://lore.kernel.org/all/tencent_43A65D51143E18518EA968DF9323694EFD08@qq.com/

> 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