Hi,
On Thu, Mar 14, 2019 at 11:28:44AM +0800, xiao ruizhu wrote:
> diff --git a/net/netfilter/nf_conntrack_sip.c
> b/net/netfilter/nf_conntrack_sip.c
> index f067c6b..4398a53 100644
> --- a/net/netfilter/nf_conntrack_sip.c
> +++ b/net/netfilter/nf_conntrack_sip.c
> @@ -799,6 +799,23 @@ static int ct_sip_parse_sdp_addr(const struct nf_conn
> *ct, const char *dptr,
> return 1;
> }
>
> +/* Remove conflicting expect created by sip helper for another master
> + * conntrack, most likely related to this master.
Hm, this comment is confusing. Code here that checks that master is
not the same, however, this suggests this is likely the same master,
however, it is related to a different master?
More comments related to the code changes below.
> + */
> +static void release_conflicting_expect(const struct nf_conn *ct,
> + const struct nf_conntrack_expect *expect,
> + const enum sip_expectation_classes class)
> +{
> + struct nf_conntrack_expect *exp;
> + struct net *net = nf_ct_net(ct);
> +
We need to hold the nf_conntrack_expect_lock here, two CPUs may race
to destroy this expectation, unlikely but possible still.
> + exp = __nf_ct_expect_find(net, nf_ct_zone(ct), &expect->tuple);
> + if (exp && exp->master != ct &&
> + nfct_help(exp->master)->helper == nfct_help(ct)->helper &&
> + exp->class == class)
I'd suggest you place this into a funcion that we can reuse from
set_expected_rtp_rtcp(), eg.
static bool
nf_ct_sip_expect_exists(const struct nf_conntrack_expect *expect,
const struct nf_conn *ct)
{
return (exp && exp->master != ct &&
nfct_help(exp->master)->helper == nfct_help(ct)->helper &&
exp->class == class);
}
So we can reuse it from L927 in set_expected_rtp_rtcp()?
> + nf_ct_unexpect_related(exp);
Use nf_ct_remove_expect() instead of nf_ct_unexpect_related(), while
holding the spinlock.
And a more general question: Given the scenario you describe is quite
specific, is it not possible to handle this case from
process_sip_response() -> process_prack_response().
Thanks !
