[PATCH nf] netfilter: nf_nat: do not reuse an unexpected expectation on RTCP clash

Since commit b8b09dc2bf35 ("netfilter: nf_conntrack_expect: use conntrack
GC to reap expectations") nf_ct_unexpect_related() no longer unlinks the
expectation from the global hash and from the per-master list. It only
marks it with NF_CT_EXPECT_DEAD and defers the unlink to the conntrack GC
worker and to the expectation list walkers; the reference taken at insert
time is kept until a walker calls nf_ct_unlink_expect().

The NAT SIP and H.323 helpers still assume the old, synchronous contract.
Their RTP/RTCP port-search loop unexpects rtp_exp on an RTCP -EBUSY clash
and then continues the loop, reusing the very same rtp_exp object:

	ret = nf_ct_expect_related(rtcp_exp, ...);
	...
	else if (ret == -EBUSY) {
		nf_ct_unexpect_related(rtp_exp);
		continue;			/* reuse rtp_exp next port */
	}

rtp_exp is still linked in nf_ct_expect_hash[] and in
master_help->expectations when the loop mutates its tuple and calls
nf_ct_expect_related() again. nf_ct_expect_insert() then runs
hlist_add_head_rcu() on the already-linked lnode -- same master list, so
it becomes self-referential -- and on hnode, and takes an extra,
unbalanced reference. The per-master expectation list is corrupted and
the conntrack GC worker trips over it:

  Oops: general protection fault, probably for non-canonical address
        0xdead000000000122
  Workqueue: events_power_efficient gc_worker
  RIP: 0010:nf_ct_unlink_expect_report+0x3e/0xd0
  Call Trace:
   nf_ct_expectation_gc+0x59/0x70
   gc_worker+0x344/0x620
   process_one_work+0x19f/0x3d0

nf_ct_expectation_gc() walks the cyclic master_help->expectations,
unlinks the entry, then visits the same now-poisoned node again and
unlinks it a second time. Under KASAN this is reported as a
use-after-free of the nf_conntrack_expect slab object. It is reachable by
an unprivileged user in a user+net namespace that assigns the "sip" or
H.323 conntrack helper with NAT and drives the RTP-succeeds /
RTCP-clashes case, and is thus a local privilege escalation primitive.

A DEAD expectation must be treated as a terminal object: never mutate its
tuple and never re-insert it. Stop reusing rtp_exp after unexpecting it in
both helpers and fail the media setup instead of retrying the next port
pair. rtp_exp is left DEAD and linked and is reaped by the GC / master
teardown as intended, and the callers drop the allocation references
unconditionally, so nothing is leaked.

This gives up the "try the next port pair" retry when only the RTCP port
of a pair is busy; that best-effort search is not a correctness
requirement, the packet is dropped and the endpoint renegotiates.

Fixes: b8b09dc2bf35 ("netfilter: nf_conntrack_expect: use conntrack GC to reap expectations")
Signed-off-by: Jaeyeong Lee <[email protected]>
---
 net/ipv4/netfilter/nf_nat_h323.c | 3 ++-
 net/netfilter/nf_nat_sip.c       | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/netfilter/nf_nat_h323.c b/net/ipv4/netfilter/nf_nat_h323.c
index 183e8a3ff2ba..78cdc107a0dc 100644
--- a/net/ipv4/netfilter/nf_nat_h323.c
+++ b/net/ipv4/netfilter/nf_nat_h323.c
@@ -236,7 +236,8 @@ static int nat_rtp_rtcp(struct sk_buff *skb, struct nf_conn *ct,
 				break;
 			else if (ret == -EBUSY) {
 				nf_ct_unexpect_related(rtp_exp);
-				continue;
+				nated_port = 0;
+				break;
 			} else if (ret < 0) {
 				nf_ct_unexpect_related(rtp_exp);
 				nated_port = 0;
diff --git a/net/netfilter/nf_nat_sip.c b/net/netfilter/nf_nat_sip.c
index aea02f6aff09..aa34b7eb96f2 100644
--- a/net/netfilter/nf_nat_sip.c
+++ b/net/netfilter/nf_nat_sip.c
@@ -637,7 +637,8 @@ static unsigned int nf_nat_sdp_media(struct sk_buff *skb, unsigned int protoff,
 			break;
 		else if (ret == -EBUSY) {
 			nf_ct_unexpect_related(rtp_exp);
-			continue;
+			port = 0;
+			break;
 		} else if (ret < 0) {
 			nf_ct_unexpect_related(rtp_exp);
 			port = 0;
-- 
2.43.0