| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240403132456.41709-1-aleksander.lobakin@intel.com>
Date: Wed, 3 Apr 2024 15:24:56 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>
Cc: Alexander Lobakin <aleksander.lobakin@...el.com>,
Dmitry Safonov <dima@...sta.com>,
Francesco Ruggeri <fruggeri@...sta.com>,
Salam Noureddine <noureddine@...sta.com>,
David Ahern <dsahern@...nel.org>,
nex.sw.ncis.osdt.itp.upstreaming@...el.com,
netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: [PATCH net-next] net/tcp: move TCP hash fail messages out of line
tcp_hash_fail() is used multiple times on hotpath, including static
inlines. It contains a couple branches and a lot of code, which all
gets inlined into the call sites. For example, one call emits two
calls to net_ratelimit() etc.
Move as much as we can out of line to a new global function. Use enum
to determine the type of failure. Check for net_ratelimit() only once,
format common fields only once as well to pass only unique strings to
pr_info().
The result for vmlinux with Clang 19:
add/remove: 2/0 grow/shrink: 0/4 up/down: 773/-4908 (-4135)
Function old new delta
__tcp_hash_fail - 757 +757
__pfx___tcp_hash_fail - 16 +16
tcp_inbound_ao_hash 1819 1062 -757
tcp_ao_verify_hash 1217 451 -766
tcp_inbound_md5_hash 1591 374 -1217
tcp_inbound_hash 3566 1398 -2168
Signed-off-by: Alexander Lobakin <aleksander.lobakin@...el.com>
---
include/net/tcp.h | 15 ++---
include/net/tcp_ao.h | 58 ++++++++-----------
net/ipv4/tcp.c | 129 +++++++++++++++++++++++++++++++++++++------
net/ipv4/tcp_ao.c | 13 ++---
4 files changed, 148 insertions(+), 67 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 9ab5b37e9d53..fa2303c788cf 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2809,17 +2809,14 @@ tcp_inbound_hash(struct sock *sk, const struct request_sock *req,
/* Invalid option or two times meet any of auth options */
if (tcp_parse_auth_options(th, &md5_location, &aoh)) {
- tcp_hash_fail("TCP segment has incorrect auth options set",
- family, skb, "");
+ tcp_hash_fail(TCP_HASH_FAIL_OPTS, family, skb);
return SKB_DROP_REASON_TCP_AUTH_HDR;
}
if (req) {
if (tcp_rsk_used_ao(req) != !!aoh) {
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPAOBAD);
- tcp_hash_fail("TCP connection can't start/end using TCP-AO",
- family, skb, "%s",
- !aoh ? "missing AO" : "AO signed");
+ tcp_hash_fail(TCP_HASH_FAIL_CONN, family, skb, !aoh);
return SKB_DROP_REASON_TCP_AOFAILURE;
}
}
@@ -2837,14 +2834,14 @@ tcp_inbound_hash(struct sock *sk, const struct request_sock *req,
* always at least one current_key.
*/
if (tcp_ao_required(sk, saddr, family, l3index, true)) {
- tcp_hash_fail("AO hash is required, but not found",
- family, skb, "L3 index %d", l3index);
+ tcp_hash_fail(TCP_HASH_FAIL_NOAO, family, skb,
+ l3index);
return SKB_DROP_REASON_TCP_AONOTFOUND;
}
if (unlikely(tcp_md5_do_lookup(sk, l3index, saddr, family))) {
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5NOTFOUND);
- tcp_hash_fail("MD5 Hash not found",
- family, skb, "L3 index %d", l3index);
+ tcp_hash_fail(TCP_HASH_FAIL_NOMD5, family, skb,
+ l3index);
return SKB_DROP_REASON_TCP_MD5NOTFOUND;
}
return SKB_NOT_DROPPED_YET;
diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h
index 471e177362b4..a00d765be30b 100644
--- a/include/net/tcp_ao.h
+++ b/include/net/tcp_ao.h
@@ -143,42 +143,32 @@ extern struct static_key_false_deferred tcp_ao_needed;
#define static_branch_tcp_ao() false
#endif
-static inline bool tcp_hash_should_produce_warnings(void)
-{
- return static_branch_tcp_md5() || static_branch_tcp_ao();
-}
+enum {
+ TCP_HASH_FAIL_OPTS,
+ TCP_HASH_FAIL_CONN,
+ TCP_HASH_FAIL_NOAO,
+ TCP_HASH_FAIL_NOMD5,
+ TCP_HASH_FAIL_UNEXP,
+ TCP_HASH_FAIL_INET,
+ TCP_HASH_FAIL_LEN,
+ TCP_HASH_FAIL_MIS,
+ TCP_HASH_FAIL_NOKEY,
+ TCP_HASH_FAIL_NOID,
+};
+
+void __tcp_hash_fail(u8 reason, u8 family, const struct sk_buff *skb,
+ int arg0, int arg1, int arg2);
-#define tcp_hash_fail(msg, family, skb, fmt, ...) \
-do { \
- const struct tcphdr *th = tcp_hdr(skb); \
- char hdr_flags[6]; \
- char *f = hdr_flags; \
- \
- if (!tcp_hash_should_produce_warnings()) \
- break; \
- if (th->fin) \
- *f++ = 'F'; \
- if (th->syn) \
- *f++ = 'S'; \
- if (th->rst) \
- *f++ = 'R'; \
- if (th->psh) \
- *f++ = 'P'; \
- if (th->ack) \
- *f++ = '.'; \
- *f = 0; \
- if ((family) == AF_INET) { \
- net_info_ratelimited("%s for %pI4.%d->%pI4.%d [%s] " fmt "\n", \
- msg, &ip_hdr(skb)->saddr, ntohs(th->source), \
- &ip_hdr(skb)->daddr, ntohs(th->dest), \
- hdr_flags, ##__VA_ARGS__); \
- } else { \
- net_info_ratelimited("%s for [%pI6c].%d->[%pI6c].%d [%s]" fmt "\n", \
- msg, &ipv6_hdr(skb)->saddr, ntohs(th->source), \
- &ipv6_hdr(skb)->daddr, ntohs(th->dest), \
- hdr_flags, ##__VA_ARGS__); \
- } \
+#define _tcp_hash_fail(reason, family, skb, ua, ...) do { \
+ if (static_branch_tcp_md5() || static_branch_tcp_ao()) { \
+ int ua[] = { __VA_ARGS__ + 0, 0, 0, 0, }; \
+ \
+ __tcp_hash_fail(reason, family, skb, ua[0], ua[1], ua[2]); \
+ } \
} while (0)
+#define tcp_hash_fail(reason, family, skb, ...) \
+ _tcp_hash_fail(reason, family, skb, __UNIQUE_ID(args_), \
+ ##__VA_ARGS__)
#ifdef CONFIG_TCP_AO
/* TCP-AO structures and functions */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index e767721b3a58..a8a9e0f7f768 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -4379,6 +4379,116 @@ int tcp_getsockopt(struct sock *sk, int level, int optname, char __user *optval,
}
EXPORT_SYMBOL(tcp_getsockopt);
+#if defined(CONFIG_TCP_MD5SIG) || defined(CONFIG_TCP_AO)
+static const char *tcp_hash_fail_common(u8 family, const struct sk_buff *skb)
+{
+ const struct tcphdr *th = tcp_hdr(skb);
+ char hdr_flags[6];
+ char *f = hdr_flags;
+
+ if (th->fin)
+ *f++ = 'F';
+ if (th->syn)
+ *f++ = 'S';
+ if (th->rst)
+ *f++ = 'R';
+ if (th->psh)
+ *f++ = 'P';
+ if (th->ack)
+ *f++ = '.';
+ *f = '\0';
+
+ if (family == AF_INET)
+ return kasprintf(GFP_ATOMIC,
+ " for %pI4.%d->%pI4.%d [%s] ",
+ &ip_hdr(skb)->saddr, ntohs(th->source),
+ &ip_hdr(skb)->daddr, ntohs(th->dest),
+ hdr_flags);
+ else
+ return kasprintf(GFP_ATOMIC,
+ " for [%pI6c].%d->[%pI6c].%d [%s] ",
+ &ipv6_hdr(skb)->saddr, ntohs(th->source),
+ &ipv6_hdr(skb)->daddr, ntohs(th->dest),
+ hdr_flags);
+}
+
+void __tcp_hash_fail(u8 reason, u8 family, const struct sk_buff *skb,
+ int arg0, int arg1, int arg2)
+{
+ const char *common __free(kfree) = NULL;
+
+ if (!net_ratelimit())
+ return;
+
+ common = tcp_hash_fail_common(family, skb);
+ if (!common)
+ return;
+
+#define tcp_hash_fail_msg(msg, common, fmt, ...) \
+ pr_info(msg "%s" fmt "\n", common, ##__VA_ARGS__)
+ switch (reason) {
+ case TCP_HASH_FAIL_OPTS:
+ tcp_hash_fail_msg("TCP segment has incorrect auth options set",
+ common, "");
+ break;
+ case TCP_HASH_FAIL_CONN:
+ tcp_hash_fail_msg("TCP connection can't start/end using TCP-AO",
+ common, "%s",
+ arg0 ? "missing AO" : "AO signed");
+ break;
+ case TCP_HASH_FAIL_NOAO:
+ tcp_hash_fail_msg("AO hash is required, but not found",
+ common, "L3 index %d", arg0);
+ break;
+ case TCP_HASH_FAIL_NOMD5:
+ tcp_hash_fail_msg("MD5 Hash not found", common, "L3 index %d",
+ arg0);
+ break;
+ case TCP_HASH_FAIL_UNEXP:
+ tcp_hash_fail_msg("Unexpected MD5 Hash found", common, "");
+ break;
+ case TCP_HASH_FAIL_INET:
+ if (family == AF_INET) {
+ tcp_hash_fail_msg("MD5 Hash failed", common,
+ "%s L3 index %d",
+ arg0 ?
+ "tcp_v4_calc_md5_hash failed" : "",
+ arg1);
+ } else {
+ if (arg0)
+ tcp_hash_fail_msg("MD5 Hash failed",
+ common, "L3 index %d",
+ arg1);
+ else
+ tcp_hash_fail_msg("MD5 Hash mismatch",
+ common, "L3 index %d",
+ arg1);
+ }
+ break;
+ case TCP_HASH_FAIL_LEN:
+ tcp_hash_fail_msg("AO hash wrong length", common,
+ "%u != %d L3 index: %d", arg0, arg1, arg2);
+ break;
+ case TCP_HASH_FAIL_MIS:
+ tcp_hash_fail_msg("AO hash mismatch", common, "L3 index: %d",
+ arg0);
+ break;
+ case TCP_HASH_FAIL_NOKEY:
+ tcp_hash_fail_msg("AO key not found", common,
+ "keyid: %u L3 index: %d", arg0, arg1);
+ break;
+ case TCP_HASH_FAIL_NOID:
+ tcp_hash_fail_msg("Requested by the peer AO key id not found",
+ common, "L3 index: %d", arg0);
+ break;
+ default:
+ break;
+ }
+#undef tcp_hash_fail_msg
+}
+EXPORT_SYMBOL(__tcp_hash_fail);
+#endif /* CONFIG_TCP_MD5SIG || CONFIG_TCP_AO */
+
#ifdef CONFIG_TCP_MD5SIG
int tcp_md5_sigpool_id = -1;
EXPORT_SYMBOL_GPL(tcp_md5_sigpool_id);
@@ -4450,7 +4560,7 @@ tcp_inbound_md5_hash(const struct sock *sk, const struct sk_buff *skb,
if (!key && hash_location) {
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5UNEXPECTED);
- tcp_hash_fail("Unexpected MD5 Hash found", family, skb, "");
+ tcp_hash_fail(TCP_HASH_FAIL_UNEXP, family, skb);
return SKB_DROP_REASON_TCP_MD5UNEXPECTED;
}
@@ -4465,21 +4575,8 @@ tcp_inbound_md5_hash(const struct sock *sk, const struct sk_buff *skb,
NULL, skb);
if (genhash || memcmp(hash_location, newhash, 16) != 0) {
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPMD5FAILURE);
- if (family == AF_INET) {
- tcp_hash_fail("MD5 Hash failed", AF_INET, skb, "%s L3 index %d",
- genhash ? "tcp_v4_calc_md5_hash failed"
- : "", l3index);
- } else {
- if (genhash) {
- tcp_hash_fail("MD5 Hash failed",
- AF_INET6, skb, "L3 index %d",
- l3index);
- } else {
- tcp_hash_fail("MD5 Hash mismatch",
- AF_INET6, skb, "L3 index %d",
- l3index);
- }
- }
+ tcp_hash_fail(TCP_HASH_FAIL_INET, family, skb, genhash,
+ l3index);
return SKB_DROP_REASON_TCP_MD5FAILURE;
}
return SKB_NOT_DROPPED_YET;
diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
index 3afeeb68e8a7..f4dbb23e549b 100644
--- a/net/ipv4/tcp_ao.c
+++ b/net/ipv4/tcp_ao.c
@@ -892,8 +892,7 @@ tcp_ao_verify_hash(const struct sock *sk, const struct sk_buff *skb,
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPAOBAD);
atomic64_inc(&info->counters.pkt_bad);
atomic64_inc(&key->pkt_bad);
- tcp_hash_fail("AO hash wrong length", family, skb,
- "%u != %d L3index: %d", maclen,
+ tcp_hash_fail(TCP_HASH_FAIL_LEN, family, skb, maclen,
tcp_ao_maclen(key), l3index);
return SKB_DROP_REASON_TCP_AOFAILURE;
}
@@ -909,8 +908,7 @@ tcp_ao_verify_hash(const struct sock *sk, const struct sk_buff *skb,
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPAOBAD);
atomic64_inc(&info->counters.pkt_bad);
atomic64_inc(&key->pkt_bad);
- tcp_hash_fail("AO hash mismatch", family, skb,
- "L3index: %d", l3index);
+ tcp_hash_fail(TCP_HASH_FAIL_MIS, family, skb, l3index);
kfree(hash_buf);
return SKB_DROP_REASON_TCP_AOFAILURE;
}
@@ -938,8 +936,8 @@ tcp_inbound_ao_hash(struct sock *sk, const struct sk_buff *skb,
info = rcu_dereference(tcp_sk(sk)->ao_info);
if (!info) {
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPAOKEYNOTFOUND);
- tcp_hash_fail("AO key not found", family, skb,
- "keyid: %u L3index: %d", aoh->keyid, l3index);
+ tcp_hash_fail(TCP_HASH_FAIL_NOKEY, family, skb, aoh->keyid,
+ l3index);
return SKB_DROP_REASON_TCP_AOUNEXPECTED;
}
@@ -1041,8 +1039,7 @@ tcp_inbound_ao_hash(struct sock *sk, const struct sk_buff *skb,
key_not_found:
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPAOKEYNOTFOUND);
atomic64_inc(&info->counters.key_not_found);
- tcp_hash_fail("Requested by the peer AO key id not found",
- family, skb, "L3index: %d", l3index);
+ tcp_hash_fail(TCP_HASH_FAIL_NOID, family, skb, l3index);
return SKB_DROP_REASON_TCP_AOKEYNOTFOUND;
}
--
2.44.0
Powered by blists - more mailing lists