(struct nf_conn)->timeout is an interval before the conntrack confirmed. After confirmed, it becomes a timestamp[1]. It is observed that timeout of an unconfirmed conntrack have been altered by calling ctnetlink_change_timeout(). As a result, `nfct_time_stamp` was wrongly added to `ct->timeout` twice[2]. Differentiate the 2 cases in all `ct->timeout` accesses. [1]: https://elixir.bootlin.com/linux/v6.3-rc5/source/net/netfilter/nf_conntrack_core.c#L1257 [2]: https://elixir.bootlin.com/linux/v6.3-rc5/source/include/net/netfilter/nf_conntrack_core.h#L92 Signed-off-by: Tzung-Bi Shih <tzungbi@xxxxxxxxxx> --- include/net/netfilter/nf_conntrack.h | 27 +++++++++++++++++++---- include/net/netfilter/nf_conntrack_core.h | 2 +- net/netfilter/nf_conntrack_core.c | 11 ++++----- net/netfilter/nf_conntrack_proto_tcp.c | 4 ++-- net/netfilter/nf_flow_table_core.c | 4 ++-- 5 files changed, 32 insertions(+), 16 deletions(-) diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h index a72028dbef0c..48e020db3fb3 100644 --- a/include/net/netfilter/nf_conntrack.h +++ b/include/net/netfilter/nf_conntrack.h @@ -290,17 +290,36 @@ static inline bool nf_is_loopback_packet(const struct sk_buff *skb) #define nfct_time_stamp ((u32)(jiffies)) +static inline s32 nf_ct_read_timeout(const struct nf_conn *ct) +{ + s32 timeout; + + if (nf_ct_is_confirmed(ct)) + timeout = READ_ONCE(ct->timeout) - nfct_time_stamp; + else + timeout = READ_ONCE(ct->timeout); + + return timeout; +} + +static inline void nf_ct_write_timeout(struct nf_conn *ct, s32 timeout) +{ + if (nf_ct_is_confirmed(ct)) + WRITE_ONCE(ct->timeout, timeout + nfct_time_stamp); + else + WRITE_ONCE(ct->timeout, timeout); +} + /* jiffies until ct expires, 0 if already expired */ static inline unsigned long nf_ct_expires(const struct nf_conn *ct) { - s32 timeout = READ_ONCE(ct->timeout) - nfct_time_stamp; - + s32 timeout = nf_ct_read_timeout(ct); return max(timeout, 0); } static inline bool nf_ct_is_expired(const struct nf_conn *ct) { - return (__s32)(READ_ONCE(ct->timeout) - nfct_time_stamp) <= 0; + return nf_ct_expires(ct) == 0; } /* use after obtaining a reference count */ @@ -319,7 +338,7 @@ static inline bool nf_ct_should_gc(const struct nf_conn *ct) static inline void nf_ct_offload_timeout(struct nf_conn *ct) { if (nf_ct_expires(ct) < NF_CT_DAY / 2) - WRITE_ONCE(ct->timeout, nfct_time_stamp + NF_CT_DAY); + nf_ct_write_timeout(ct, NF_CT_DAY); } struct kernel_param; diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h index 71d1269fe4d4..7a6e49a66d20 100644 --- a/include/net/netfilter/nf_conntrack_core.h +++ b/include/net/netfilter/nf_conntrack_core.h @@ -89,7 +89,7 @@ static inline void __nf_ct_set_timeout(struct nf_conn *ct, u64 timeout) { if (timeout > INT_MAX) timeout = INT_MAX; - WRITE_ONCE(ct->timeout, nfct_time_stamp + (u32)timeout); + nf_ct_write_timeout(ct, (u32)timeout); } int __nf_ct_change_timeout(struct nf_conn *ct, u64 cta_timeout); diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c index db1ea361f2da..47166576c195 100644 --- a/net/netfilter/nf_conntrack_core.c +++ b/net/netfilter/nf_conntrack_core.c @@ -657,7 +657,7 @@ bool nf_ct_delete(struct nf_conn *ct, u32 portid, int report) tstamp = nf_conn_tstamp_find(ct); if (tstamp) { - s32 timeout = READ_ONCE(ct->timeout) - nfct_time_stamp; + s32 timeout = nf_ct_read_timeout(ct); tstamp->stop = ktime_get_real_ns(); if (timeout < 0) @@ -1063,7 +1063,7 @@ static int nf_ct_resolve_clash_harder(struct sk_buff *skb, u32 repl_idx) } /* We want the clashing entry to go away real soon: 1 second timeout. */ - WRITE_ONCE(loser_ct->timeout, nfct_time_stamp + HZ); + nf_ct_write_timeout(loser_ct, HZ); /* IPS_NAT_CLASH removes the entry automatically on the first * reply. Also prevents UDP tracker from moving the entry to @@ -2079,11 +2079,8 @@ void __nf_ct_refresh_acct(struct nf_conn *ct, goto acct; /* If not in hash table, timer will not be active yet */ - if (nf_ct_is_confirmed(ct)) - extra_jiffies += nfct_time_stamp; - - if (READ_ONCE(ct->timeout) != extra_jiffies) - WRITE_ONCE(ct->timeout, extra_jiffies); + if (nf_ct_read_timeout(ct) != extra_jiffies) + nf_ct_write_timeout(ct, extra_jiffies); acct: if (do_acct) nf_ct_acct_update(ct, CTINFO2DIR(ctinfo), skb->len); diff --git a/net/netfilter/nf_conntrack_proto_tcp.c b/net/netfilter/nf_conntrack_proto_tcp.c index 4018acb1d674..a2797d026943 100644 --- a/net/netfilter/nf_conntrack_proto_tcp.c +++ b/net/netfilter/nf_conntrack_proto_tcp.c @@ -766,7 +766,7 @@ static void __cold nf_tcp_handle_invalid(struct nf_conn *ct, "packet (index %d, dir %d) response for index %d lower timeout to %u", index, dir, ct->proto.tcp.last_index, timeout); - WRITE_ONCE(ct->timeout, timeout + nfct_time_stamp); + nf_ct_write_timeout(ct, timeout); } } else { ct->proto.tcp.last_index = index; @@ -939,7 +939,7 @@ void nf_conntrack_tcp_set_closing(struct nf_conn *ct) } timeout = timeouts[TCP_CONNTRACK_CLOSE]; - WRITE_ONCE(ct->timeout, timeout + nfct_time_stamp); + nf_ct_write_timeout(ct, timeout); spin_unlock_bh(&ct->lock); diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c index 04bd0ed4d2ae..113bd361e537 100644 --- a/net/netfilter/nf_flow_table_core.c +++ b/net/netfilter/nf_flow_table_core.c @@ -206,8 +206,8 @@ static void flow_offload_fixup_ct(struct nf_conn *ct) if (timeout < 0) timeout = 0; - if (nf_flow_timeout_delta(READ_ONCE(ct->timeout)) > (__s32)timeout) - WRITE_ONCE(ct->timeout, nfct_time_stamp + timeout); + if (nf_flow_timeout_delta(nf_ct_read_timeout(ct)) > (__s32)timeout) + nf_ct_write_timeout(ct, timeout); } static void flow_offload_route_release(struct flow_offload *flow) -- 2.40.0.577.gac1e443424-goog
