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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADvbK_euHyYmBSUGUCBsV13b8EU8HLe=Z0jZq7nysUP1qQwzRQ@mail.gmail.com>
Date: Thu, 4 Jan 2024 11:48:19 -0500
From: Xin Long <lucien.xin@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: vladbu@...dia.com, Tao Liu <taoliu828@....com>, davem@...emloft.net, 
	edumazet@...gle.com, pabeni@...hat.com, paulb@...dia.com, 
	netdev@...r.kernel.org, simon.horman@...igine.com, xiyou.wangcong@...il.com, 
	pablo@...filter.org
Subject: 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 <kuba@...nel.org> wrote:
>
> On Thu, 28 Dec 2023 16:14:57 +0800 Tao Liu wrote:
> > 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.