netdev - Re: [PATCH net] net/sched: act_ct: Fix ct template allocation for zone 0

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: <YK/LcLVMhFDe/7KF@horizon.localdomain>
Date:   Thu, 27 May 2021 13:40:16 -0300
From:   Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
To:     Ariel Levkovich <lariel@...dia.com>
Cc:     netdev@...r.kernel.org, paulb@...dia.com, jiri@...nulli.us
Subject: Re: [PATCH net] net/sched: act_ct: Fix ct template allocation for
 zone 0

On Thu, May 27, 2021 at 12:14:21PM -0400, Ariel Levkovich wrote:
...
> I meant if there's no ct_clear action.
> 
> Assume we already when through zone X in some previous action.
> 
> In such case skb->_nfct has that zone's id.
> 
> Now, if we go to zone=0, we skip this entirely, since p->tmpl is NULL :
> 
> /* Associate skb with specified zone. */
>                 if (tmpl) {
>                         nf_conntrack_put(skb_nfct(skb));
> nf_conntrack_get(&tmpl->ct_general);
>                         nf_ct_set(skb, tmpl, IP_CT_NEW);
>                 }
> 
> 
> And then in nf_conntrack_in it continues with the previous zone:
> 
> err = nf_conntrack_in(skb, &state)
> 
>    calling ->   ret = resolve_normal_ct(tmpl, skb, dataoff,
>                                   protonum, state);
> 
>            calling -> zone = nf_ct_zone_tmpl(tmpl, skb, &tmp);
> 
> 
> I encountered it by accident while running one of our test which was buggy
> 
> but due to the zone=0 bug the bug in the test was hidden and test passed.
> 
> Once I enabled my fix to alloc templated for zone=0 it was exposed.
> 
> The test doesn't use ct_clear between zones.

I see now, thanks. When you had said without ct_clear, I thought only
about the first run through ct. Didn't think there would be this
implicit ct_clear expectation. It could also erase _nfct if zone==0,
but agree, too much especial handling for zone 0.

Seems it needs both fixes then, while this patch is good as is.

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>