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: stable@vger.kernel.org Reported-by: Vega Assisted-by: Codex:gpt-5.4 Signed-off-by: Zihan Xi --- 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