Pablo, I'm looking at a regression introduced by this patch
and I'm not sure about the intentions:
+int nf_ct_expect_related(struct nf_conntrack_expect *expect)
+{
+ int ret;
+
+ spin_lock_bh(&nf_conntrack_lock);
+ ret = __nf_ct_expect_check(expect);
+ if (ret < 0)
+ goto out;
This is unfortunately broken since we return 0 when refreshing an
existing expectation. This will create an identical expectation
for each refresh.
nf_ct_expect_insert(expect);
+ atomic_inc(&expect->use);
This I don't understand - the caller is holding a reference, why
do we need another one?
+ spin_unlock_bh(&nf_conntrack_lock);
nf_ct_expect_event(IPEXP_NEW, expect);
- ret = 0;
+ nf_ct_expect_put(expect);
+ return ret;
out:
spin_unlock_bh(&nf_conntrack_lock);
return ret;
}
EXPORT_SYMBOL_GPL(nf_ct_expect_related);
+int nf_ct_expect_related_report(struct nf_conntrack_expect *expect,
+ u32 pid, int report)
+{
+ int ret;
+
+ spin_lock_bh(&nf_conntrack_lock);
+ ret = __nf_ct_expect_check(expect);
+ if (ret < 0)
+ goto out;
Same problem
+ nf_ct_expect_insert(expect);
+out:
+ spin_unlock_bh(&nf_conntrack_lock);
+ if (ret == 0)
+ nf_ct_expect_event_report(IPEXP_NEW, expect, pid, report);
But here we don't take the reference, despite having the exact
same situation.
The next question would be - why do we need those two functions at
all? Aside from the apparently unnecessary reference counting, the
only difference is reporting, and that actually uses the exact
same code path.
+ return ret;
+}
+EXPORT_SYMBOL_GPL(nf_ct_expect_related_report);
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
