[PATCH nf 1/1] netfilter: nf_conntrack_tcp: defer invalid logging until after unlock

nf_conntrack_tcp_packet() can log invalid packets while ct->lock is still
held. This happens both through tcp_in_window() -> nf_tcp_log_invalid()
and through the NFCT_TCP_INVALID path -> nf_tcp_handle_invalid().

If IPv4 invalid logging is bound to nfnetlink_log and the NFLLOG instance
has NFULNL_CFG_F_CONNTRACK enabled, the logging path re-enters conntrack
netlink glue and reaches tcp_to_nlattr(), which tries to take ct->lock
again. This results in recursive lock acquisition and can deadlock the
sender in queued_spin_lock_slowpath(), followed by stalls or panic.

Fix this by recording the minimal invalid-log context while ct->lock is
held and emitting the actual log records only after dropping the lock.
This keeps the existing invalid-log messages and still covers the timeout
lowering log from nf_tcp_handle_invalid(), but removes the lock recursion
from the logging path.

Fixes: a29a9a585b28 ("netfilter: nfnetlink_log: allow to attach conntrack")
Cc: [email protected]
Reported-by: Vega <[email protected]>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zihan Xi <[email protected]>
---
 net/netfilter/nf_conntrack_proto_tcp.c | 167 +++++++++++++++++++------
 1 file changed, 128 insertions(+), 39 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c
index ceeed3d7fe52..00f017d9997f 100644
--- a/net/netfilter/nf_conntrack_proto_tcp.c
+++ b/net/netfilter/nf_conntrack_proto_tcp.c
@@ -480,37 +480,125 @@ static void tcp_init_sender(struct ip_ct_tcp_state *sender,
 	}
 }
 
-__printf(6, 7)
-static enum nf_ct_tcp_action nf_tcp_log_invalid(const struct sk_buff *skb,
-						const struct nf_conn *ct,
-						const struct nf_hook_state *state,
-						const struct ip_ct_tcp_state *sender,
-						enum nf_ct_tcp_action ret,
-						const char *fmt, ...)
+enum nf_tcp_invalid_log_type {
+	NF_TCP_LOG_NONE,
+	NF_TCP_LOG_OVERSHOT,
+	NF_TCP_LOG_SEQ_OVER,
+	NF_TCP_LOG_ACK_OVER,
+	NF_TCP_LOG_SEQ_UNDER,
+	NF_TCP_LOG_ACK_UNDER,
+	NF_TCP_LOG_LOWER_TIMEOUT,
+};
+
+struct nf_tcp_invalid_log {
+	enum nf_tcp_invalid_log_type type;
+	u32 value;
+	u8 index;
+	u8 dir;
+	u8 last_index;
+};
+
+struct nf_tcp_invalid_logs {
+	/* At most one tcp_in_window() log plus one lower-timeout log. */
+	struct nf_tcp_invalid_log entries[2];
+	u8 num;
+};
+
+static void nf_tcp_queue_invalid_log(struct nf_tcp_invalid_logs *logs,
+				     enum nf_tcp_invalid_log_type type,
+				     u32 value, u8 index, u8 dir,
+				     u8 last_index)
+{
+	struct nf_tcp_invalid_log *log;
+
+	if (logs->num >= ARRAY_SIZE(logs->entries))
+		return;
+
+	log = &logs->entries[logs->num++];
+	log->type = type;
+	log->value = value;
+	log->index = index;
+	log->dir = dir;
+	log->last_index = last_index;
+}
+
+static enum nf_ct_tcp_action
+nf_tcp_log_invalid(const struct nf_conn *ct,
+		   const struct ip_ct_tcp_state *sender,
+		   struct nf_tcp_invalid_logs *logs,
+		   enum nf_ct_tcp_action ret,
+		   enum nf_tcp_invalid_log_type type,
+		   u32 value)
 {
 	const struct nf_tcp_net *tn = nf_tcp_pernet(nf_ct_net(ct));
-	struct va_format vaf;
-	va_list args;
 	bool be_liberal;
 
 	be_liberal = sender->flags & IP_CT_TCP_FLAG_BE_LIBERAL || tn->tcp_be_liberal;
 	if (be_liberal)
 		return NFCT_TCP_ACCEPT;
 
-	va_start(args, fmt);
-	vaf.fmt = fmt;
-	vaf.va = &args;
-	nf_ct_l4proto_log_invalid(skb, ct, state, "%pV", &vaf);
-	va_end(args);
-
+	nf_tcp_queue_invalid_log(logs, type, value, 0, 0, 0);
 	return ret;
 }
 
+static void nf_tcp_emit_invalid_log(const struct sk_buff *skb,
+				    const struct nf_conn *ct,
+				    const struct nf_hook_state *state,
+				    const struct nf_tcp_invalid_log *log)
+{
+	switch (log->type) {
+	case NF_TCP_LOG_OVERSHOT:
+		nf_ct_l4proto_log_invalid(skb, ct, state,
+					  "%u bytes more than expected",
+					  log->value);
+		break;
+	case NF_TCP_LOG_SEQ_OVER:
+		nf_ct_l4proto_log_invalid(skb, ct, state,
+					  "SEQ is over upper bound %u (over the window of the receiver)",
+					  log->value);
+		break;
+	case NF_TCP_LOG_ACK_OVER:
+		nf_ct_l4proto_log_invalid(skb, ct, state,
+					  "ACK is over upper bound %u (ACKed data not seen yet)",
+					  log->value);
+		break;
+	case NF_TCP_LOG_SEQ_UNDER:
+		nf_ct_l4proto_log_invalid(skb, ct, state,
+					  "SEQ is under lower bound %u (already ACKed data retransmitted)",
+					  log->value);
+		break;
+	case NF_TCP_LOG_ACK_UNDER:
+		nf_ct_l4proto_log_invalid(skb, ct, state,
+					  "ignored ACK under lower bound %u (possible overly delayed)",
+					  log->value);
+		break;
+	case NF_TCP_LOG_LOWER_TIMEOUT:
+		nf_ct_l4proto_log_invalid(skb, ct, state,
+					  "packet (index %d, dir %d) response for index %d lower timeout to %u",
+					  log->index, log->dir,
+					  log->last_index, log->value);
+		break;
+	case NF_TCP_LOG_NONE:
+		break;
+	}
+}
+
+static void nf_tcp_emit_invalid_logs(const struct sk_buff *skb,
+				     const struct nf_conn *ct,
+				     const struct nf_hook_state *state,
+				     const struct nf_tcp_invalid_logs *logs)
+{
+	u8 i;
+
+	for (i = 0; i < logs->num; i++)
+		nf_tcp_emit_invalid_log(skb, ct, state, &logs->entries[i]);
+}
+
 static enum nf_ct_tcp_action
 tcp_in_window(struct nf_conn *ct, enum ip_conntrack_dir dir,
 	      unsigned int index, const struct sk_buff *skb,
 	      unsigned int dataoff, const struct tcphdr *tcph,
-	      const struct nf_hook_state *hook_state)
+	      struct nf_tcp_invalid_logs *logs)
 {
 	struct ip_ct_tcp *state = &ct->proto.tcp;
 	struct ip_ct_tcp_state *sender = &state->seen[dir];
@@ -640,31 +728,29 @@ tcp_in_window(struct nf_conn *ct, enum ip_conntrack_dir dir,
 			sender->td_end = end;
 			sender->flags |= IP_CT_TCP_FLAG_DATA_UNACKNOWLEDGED;
 
-			return nf_tcp_log_invalid(skb, ct, hook_state, sender, NFCT_TCP_IGNORE,
-						  "%u bytes more than expected", overshot);
+			return nf_tcp_log_invalid(ct, sender, logs, NFCT_TCP_IGNORE,
+					  NF_TCP_LOG_OVERSHOT, overshot);
 		}
 
-		return nf_tcp_log_invalid(skb, ct, hook_state, sender, NFCT_TCP_INVALID,
-					  "SEQ is over upper bound %u (over the window of the receiver)",
-					  sender->td_maxend + 1);
+		return nf_tcp_log_invalid(ct, sender, logs, NFCT_TCP_INVALID,
+				  NF_TCP_LOG_SEQ_OVER, sender->td_maxend + 1);
 	}
 
 	if (!before(sack, receiver->td_end + 1))
-		return nf_tcp_log_invalid(skb, ct, hook_state, sender, NFCT_TCP_INVALID,
-					  "ACK is over upper bound %u (ACKed data not seen yet)",
-					  receiver->td_end + 1);
+		return nf_tcp_log_invalid(ct, sender, logs, NFCT_TCP_INVALID,
+				  NF_TCP_LOG_ACK_OVER, receiver->td_end + 1);
 
 	/* Is the ending sequence in the receive window (if available)? */
 	in_recv_win = !receiver->td_maxwin ||
 		      after(end, sender->td_end - receiver->td_maxwin - 1);
 	if (!in_recv_win)
-		return nf_tcp_log_invalid(skb, ct, hook_state, sender, NFCT_TCP_IGNORE,
-					  "SEQ is under lower bound %u (already ACKed data retransmitted)",
-					  sender->td_end - receiver->td_maxwin - 1);
+		return nf_tcp_log_invalid(ct, sender, logs, NFCT_TCP_IGNORE,
+				  NF_TCP_LOG_SEQ_UNDER,
+				  sender->td_end - receiver->td_maxwin - 1);
 	if (!after(sack, receiver->td_end - MAXACKWINDOW(sender) - 1))
-		return nf_tcp_log_invalid(skb, ct, hook_state, sender, NFCT_TCP_IGNORE,
-					  "ignored ACK under lower bound %u (possible overly delayed)",
-					  receiver->td_end - MAXACKWINDOW(sender) - 1);
+		return nf_tcp_log_invalid(ct, sender, logs, NFCT_TCP_IGNORE,
+				  NF_TCP_LOG_ACK_UNDER,
+				  receiver->td_end - MAXACKWINDOW(sender) - 1);
 
 	/* Take into account window scaling (RFC 1323). */
 	if (!tcph->syn)
@@ -720,10 +806,9 @@ tcp_in_window(struct nf_conn *ct, enum ip_conntrack_dir dir,
 }
 
 static void __cold nf_tcp_handle_invalid(struct nf_conn *ct,
-					 enum ip_conntrack_dir dir,
-					 int index,
-					 const struct sk_buff *skb,
-					 const struct nf_hook_state *hook_state)
+				 enum ip_conntrack_dir dir,
+				 int index,
+				 struct nf_tcp_invalid_logs *logs)
 {
 	const unsigned int *timeouts;
 	const struct nf_tcp_net *tn;
@@ -764,9 +849,10 @@ static void __cold nf_tcp_handle_invalid(struct nf_conn *ct,
 
 		timeout = READ_ONCE(timeouts[TCP_CONNTRACK_UNACK]);
 		if (expires > timeout) {
-			nf_ct_l4proto_log_invalid(skb, ct, hook_state,
-					  "packet (index %d, dir %d) response for index %d lower timeout to %u",
-					  index, dir, ct->proto.tcp.last_index, timeout);
+			nf_tcp_queue_invalid_log(logs,
+						 NF_TCP_LOG_LOWER_TIMEOUT,
+						 timeout, index, dir,
+						 ct->proto.tcp.last_index);
 
 			WRITE_ONCE(ct->timeout, timeout + nfct_time_stamp);
 		}
@@ -971,6 +1057,7 @@ int nf_conntrack_tcp_packet(struct nf_conn *ct,
 	enum tcp_conntrack new_state, old_state;
 	unsigned int index, *timeouts;
 	enum nf_ct_tcp_action res;
+	struct nf_tcp_invalid_logs logs = {};
 	enum ip_conntrack_dir dir;
 	const struct tcphdr *th;
 	struct tcphdr _tcph;
@@ -1252,14 +1339,16 @@ int nf_conntrack_tcp_packet(struct nf_conn *ct,
 	}
 
 	res = tcp_in_window(ct, dir, index,
-			    skb, dataoff, th, state);
+			    skb, dataoff, th, &logs);
 	switch (res) {
 	case NFCT_TCP_IGNORE:
 		spin_unlock_bh(&ct->lock);
+		nf_tcp_emit_invalid_logs(skb, ct, state, &logs);
 		return NF_ACCEPT;
 	case NFCT_TCP_INVALID:
-		nf_tcp_handle_invalid(ct, dir, index, skb, state);
+		nf_tcp_handle_invalid(ct, dir, index, &logs);
 		spin_unlock_bh(&ct->lock);
+		nf_tcp_emit_invalid_logs(skb, ct, state, &logs);
 		return -NF_ACCEPT;
 	case NFCT_TCP_ACCEPT:
 		break;
-- 
2.43.0