* [PATCH net v3 0/6] amt: relay tunnel lifetime, validation and request-path hardening
@ 2026-10-12 12:59 Omar Ramadan
2026-10-12 12:59 ` [PATCH net v3 1/6] amt: fix use-after-free when a Membership Update re-arms a freed tunnel Omar Ramadan
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: Omar Ramadan @ 2026-10-12 12:59 UTC (permalink / raw)
To: Taehee Yoo, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Simon Horman
Six fixes to the AMT relay data and control paths, found while adding an
IPv6 outer transport (that feature follows for net-next, rebased on this).
All are independent of the IPv6 work and belong in net with stable in mind.
1/6 is the most serious: a Membership Update can re-arm the gc timer of a
tunnel amt_tunnel_expire() is already freeing, firing a delayed_work in
freed memory. 2/6 and 3/6 close two input-validation holes (a gateway
Query/Data from 0.0.0.0 before a relay is learned; a 16-byte IPv6 literal
accepted into an IPv4 netlink attribute). 4/6-6/6 harden the relay's
tunnel table: key it on the (address, port) endpoint RFC 7450 defines,
surface silently-dropped pre-Query reports, and stop allocating tunnel
state for unauthenticated (spoofable) Requests, which closes a table-
exhaustion / ICMP-reflector / session-desync primitive and moves the
max_tunnels bound onto verified gateways.
Each patch carries a Fixes: tag. Verified W=1 clean and with a
KASAN + lockdep + PROVE_RCU boot of the amt selftests, no splat, and
of amt.sh with max_tunnels 1, which keeps the tunnel table full while
the gateway keeps sending Requests.
v3:
- Add 1/6-3/6, found in review of v2 and of the net-next IPv6
series [1]: the gc_wq re-arm use-after-free, and the IPv4 halves of
two input-validation bugs. The IPv6 series will be rebased on this.
- 4/6: do not count the invalid-MAC drop in rx_dropped; amt_rcv()
already counts it.
- 5/6: commit message: the Query supplies the response MAC that the
Update echoes, not the nonce.
- 6/6: keep three secrets so a response MAC outlives the advertised
QQIC; cap the tunnels one source can hold in the locked re-check (a
fixed limit, no new UAPI); when the table is full, answer a gateway
that holds no tunnel with ICMP and no Query, so it rediscovers; seed
the secret before the socket is created; refresh last_seen rather
than re-arming gc_wq (see 1/6).
v2: https://lore.kernel.org/netdev/20261009201455.1904698-1-omar@blockcast.net/
v1: https://lore.kernel.org/netdev/20261008003606.3666617-1-omar@blockcast.net/
[1] https://lore.kernel.org/netdev/20261009122426.551178-1-omar@blockcast.net/
Omar Ramadan (6):
amt: fix use-after-free when a Membership Update re-arms a freed tunnel
amt: reject a gateway Query or Data from 0.0.0.0 before a relay is learned
amt: reject an IPv6 address passed in an IPv4 address attribute
amt: key relay tunnel state on the (address, port) endpoint, not the address
amt: make pre-query report drops visible
amt: do not create tunnel state for unauthenticated Requests
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net v3 1/6] amt: fix use-after-free when a Membership Update re-arms a freed tunnel
2026-10-12 12:59 [PATCH net v3 0/6] amt: relay tunnel lifetime, validation and request-path hardening Omar Ramadan
@ 2026-10-12 12:59 ` Omar Ramadan
2026-10-12 12:59 ` [PATCH net v3 2/6] amt: reject a gateway Query or Data from 0.0.0.0 before a relay is learned Omar Ramadan
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Omar Ramadan @ 2026-10-12 12:59 UTC (permalink / raw)
To: Taehee Yoo, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Simon Horman
amt_update_handler() refreshes a relay tunnel's garbage-collection timer
with mod_delayed_work(&tunnel->gc_wq, ...) on a tunnel found by a lockless
list_for_each_entry_rcu() walk, holding only rcu_read_lock(). Nothing
serialises this against amt_tunnel_expire(), which runs from that same
gc_wq: once expire has taken amt->lock, done list_del_rcu() and queued
kfree_rcu(), a concurrent Update that already cached the tunnel pointer
re-arms gc_wq on an object that is about to be freed. After the grace
period the delayed_work timer is left armed in freed memory, so it fires
into a use-after-free (and double-list_del_rcu/nr_tunnels underflow on the
way).
Make amt_tunnel_expire() the only context that frees a tunnel on the
timeout path and let it re-arm itself: the Update path only stamps
tunnel->last_seen, and expire re-reads last_seen under amt->lock to decide
whether to re-arm or free. The Update path therefore never touches gc_wq,
so it can no longer re-queue the work of a tunnel being torn down.
amt_dev_stop() is the other freer; mark each tunnel dead and splice the
list out under amt->lock before cancelling and freeing, so a gc_wq that is
already running sees ->dead and bails instead of freeing in parallel. The
tunnel is then freed exactly once.
Fixes: cbc21dc1cfe9 ("amt: add data plane of amt interface")
Assisted-by: LLM
Signed-off-by: Omar Ramadan <omar@blockcast.net>
---
drivers/net/amt.c | 48 +++++++++++++++++++++++++++++++++++++++++------
include/net/amt.h | 11 +++++++++++
2 files changed, 53 insertions(+), 6 deletions(-)
diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index b53f8ec..b284fa7 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -1328,8 +1328,29 @@ static void amt_tunnel_expire(struct work_struct *work)
struct amt_tunnel_list,
gc_wq);
struct amt_dev *amt = tunnel->amt;
+ unsigned long deadline;
spin_lock_bh(&amt->lock);
+ /* amt_dev_stop() has taken ownership of the teardown. */
+ if (tunnel->dead) {
+ spin_unlock_bh(&amt->lock);
+ return;
+ }
+ /* A Membership Update may have refreshed the tunnel after this work was
+ * queued. Re-arm from last_seen rather than letting amt_update_handler()
+ * re-arm gc_wq: this handler is then the only context that frees the
+ * tunnel (and the only one amt_dev_stop() has to cancel), which closes
+ * the use-after-free where a concurrent Update re-armed gc_wq on a tunnel
+ * this function had already list_del_rcu()'d and kfree_rcu()'d.
+ */
+ deadline = READ_ONCE(tunnel->last_seen) +
+ msecs_to_jiffies(amt_gmi(amt)) * 3;
+ if (time_before(jiffies, deadline)) {
+ mod_delayed_work(amt_wq, &tunnel->gc_wq, deadline - jiffies);
+ spin_unlock_bh(&amt->lock);
+ return;
+ }
+ tunnel->dead = true;
rcu_read_lock();
list_del_rcu(&tunnel->list);
amt->nr_tunnels--;
@@ -2494,9 +2515,12 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb)
if (tunnel->ip4 == saddr) {
if ((nonce == tunnel->nonce &&
response_mac == tunnel->mac)) {
- mod_delayed_work(amt_wq, &tunnel->gc_wq,
- msecs_to_jiffies(amt_gmi(amt))
- * 3);
+ /* Refresh liveness only. gc_wq is re-armed by
+ * amt_tunnel_expire() itself from last_seen, never
+ * from here, so this cannot re-queue the work of a
+ * tunnel that is being freed.
+ */
+ WRITE_ONCE(tunnel->last_seen, jiffies);
goto report;
} else {
netdev_dbg(amt->dev, "Invalid MAC\n");
@@ -2732,6 +2756,7 @@ static bool amt_request_handler(struct amt_dev *amt, struct sk_buff *skb)
for (i = 0; i < amt->hash_buckets; i++)
INIT_HLIST_HEAD(&tunnel->groups[i]);
+ tunnel->last_seen = jiffies;
INIT_DELAYED_WORK(&tunnel->gc_wq, amt_tunnel_expire);
list_add_tail_rcu(&tunnel->list, &amt->tunnel_list);
@@ -3053,6 +3078,7 @@ static int amt_dev_stop(struct net_device *dev)
struct amt_tunnel_list *tunnel, *tmp;
struct sk_buff *skb;
struct sock *sk;
+ LIST_HEAD(doomed);
int i;
disable_delayed_work_sync(&amt->req_wq);
@@ -3079,9 +3105,19 @@ static int amt_dev_stop(struct net_device *dev)
amt->req_cnt = 0;
WRITE_ONCE(amt->remote_ip, 0);
- list_for_each_entry_safe(tunnel, tmp, &amt->tunnel_list, list) {
- list_del_rcu(&tunnel->list);
- amt->nr_tunnels--;
+ /* Mark every tunnel dead and detach the list under the lock, so a
+ * concurrent amt_tunnel_expire() sees ->dead and bails instead of
+ * freeing a tunnel in parallel. The detached list is then private to
+ * this path, which is the sole freer.
+ */
+ spin_lock_bh(&amt->lock);
+ list_for_each_entry(tunnel, &amt->tunnel_list, list)
+ tunnel->dead = true;
+ list_splice_init(&amt->tunnel_list, &doomed);
+ amt->nr_tunnels = 0;
+ spin_unlock_bh(&amt->lock);
+
+ list_for_each_entry_safe(tunnel, tmp, &doomed, list) {
cancel_delayed_work_sync(&tunnel->gc_wq);
amt_clear_groups(tunnel);
kfree_rcu(tunnel, rcu);
diff --git a/include/net/amt.h b/include/net/amt.h
index 2846dde..0520121 100644
--- a/include/net/amt.h
+++ b/include/net/amt.h
@@ -240,6 +240,17 @@ struct amt_tunnel_list {
u32 nr_sources;
enum amt_status status;
struct delayed_work gc_wq;
+ /* Last time a valid Membership Update refreshed this tunnel. gc_wq
+ * re-arms itself from this rather than being re-armed by the Update
+ * path, so the Update path cannot re-queue the work of a tunnel that
+ * amt_tunnel_expire() is freeing.
+ */
+ unsigned long last_seen;
+ /* Set under amt->lock by whichever of amt_tunnel_expire() or
+ * amt_dev_stop() reaches the tunnel first; the other then leaves it
+ * alone, so the tunnel is freed exactly once.
+ */
+ bool dead;
__be16 source_port;
__be32 ip4;
__be32 nonce;
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net v3 2/6] amt: reject a gateway Query or Data from 0.0.0.0 before a relay is learned
2026-10-12 12:59 [PATCH net v3 0/6] amt: relay tunnel lifetime, validation and request-path hardening Omar Ramadan
2026-10-12 12:59 ` [PATCH net v3 1/6] amt: fix use-after-free when a Membership Update re-arms a freed tunnel Omar Ramadan
@ 2026-10-12 12:59 ` Omar Ramadan
2026-10-12 12:59 ` [PATCH net v3 3/6] amt: reject an IPv6 address passed in an IPv4 address attribute Omar Ramadan
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Omar Ramadan @ 2026-10-12 12:59 UTC (permalink / raw)
To: Taehee Yoo, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Simon Horman
A gateway's amt_rcv() admits a Membership Query or Multicast Data message
when "saddr != remote_ip" is false. remote_ip is 0 before discovery has
learned a relay (and is reset to 0 by amt_dev_open() and when discovery
gives up), so a packet whose source is 0.0.0.0 -- for example a
0.0.0.0 -> 255.255.255.255 broadcast delivered to the socket -- satisfies
0 != 0 and is accepted. With the nonce freshly reset to 0, a spoofed
Query is then taken as genuine: it sets ready4/qi and the response MAC and
suppresses rediscovery until the discovery timeout, and a spoofed Data
message is injected the same way.
Reject both when no relay is learned: a 0 remote_ip must match nothing, so
guard the check with !remote_ip. Discovery is unaffected -- Relay
Advertisements are validated against discovery_ip, not remote_ip.
Fixes: b9022b53adad ("amt: add control plane of amt interface")
Assisted-by: LLM
Signed-off-by: Omar Ramadan <omar@blockcast.net>
---
drivers/net/amt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index b284fa7..c3716ba 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -2858,7 +2858,7 @@ static int amt_rcv(struct sock *sk, struct sk_buff *skb)
}
goto out;
case AMT_MSG_MULTICAST_DATA:
- if (saddr != remote_ip) {
+ if (!remote_ip || saddr != remote_ip) {
netdev_dbg(amt->dev, "Invalid Relay IP\n");
err = true;
goto drop;
@@ -2869,7 +2869,7 @@ static int amt_rcv(struct sock *sk, struct sk_buff *skb)
else
goto out;
case AMT_MSG_MEMBERSHIP_QUERY:
- if (saddr != remote_ip) {
+ if (!remote_ip || saddr != remote_ip) {
netdev_dbg(amt->dev, "Invalid Relay IP\n");
err = true;
goto drop;
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net v3 3/6] amt: reject an IPv6 address passed in an IPv4 address attribute
2026-10-12 12:59 [PATCH net v3 0/6] amt: relay tunnel lifetime, validation and request-path hardening Omar Ramadan
2026-10-12 12:59 ` [PATCH net v3 1/6] amt: fix use-after-free when a Membership Update re-arms a freed tunnel Omar Ramadan
2026-10-12 12:59 ` [PATCH net v3 2/6] amt: reject a gateway Query or Data from 0.0.0.0 before a relay is learned Omar Ramadan
@ 2026-10-12 12:59 ` Omar Ramadan
2026-10-12 12:59 ` [PATCH net v3 4/6] amt: key relay tunnel state on the (address, port) endpoint, not the address Omar Ramadan
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Omar Ramadan @ 2026-10-12 12:59 UTC (permalink / raw)
To: Taehee Yoo, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Simon Horman
The netlink policy for IFLA_AMT_LOCAL_IP and IFLA_AMT_DISCOVERY_IP sets
only a minimum length (the size of an IPv4 address), so an iproute2 that
puts a 16-byte IPv6 literal in one of these IPv4 attributes passes
validation. nla_get_in_addr() then reads just the first four bytes, so
"2001:db8::" silently becomes 32.1.13.184 and the device is created with an
address the user never asked for.
Reject an attribute whose length is exactly an IPv6 address for the two
IPv4 attributes, so the mistake is refused with an extack instead of being
truncated.
Fixes: b9022b53adad ("amt: add control plane of amt interface")
Assisted-by: LLM
Signed-off-by: Omar Ramadan <omar@blockcast.net>
---
drivers/net/amt.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index c3716ba..550b0a5 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -3194,6 +3194,15 @@ static const struct nla_policy amt_policy[IFLA_AMT_MAX + 1] = {
[IFLA_AMT_MAX_TUNNELS] = { .type = NLA_U32 },
};
+static bool amt_ip6_in_ip4_attr(const struct nlattr *attr,
+ struct netlink_ext_ack *extack)
+{
+ if (!attr || nla_len(attr) != sizeof(struct in6_addr))
+ return false;
+ NL_SET_ERR_MSG_ATTR(extack, attr, "IPv6 address in an IPv4 attribute");
+ return true;
+}
+
static int amt_validate(struct nlattr *tb[], struct nlattr *data[],
struct netlink_ext_ack *extack)
{
@@ -3224,6 +3233,9 @@ static int amt_validate(struct nlattr *tb[], struct nlattr *data[],
return -EINVAL;
}
+ if (amt_ip6_in_ip4_attr(data[IFLA_AMT_LOCAL_IP], extack))
+ return -EINVAL;
+
if (!data[IFLA_AMT_DISCOVERY_IP] &&
nla_get_u32(data[IFLA_AMT_MODE]) == AMT_MODE_GATEWAY) {
NL_SET_ERR_MSG_ATTR(extack, data[IFLA_AMT_LOCAL_IP],
@@ -3231,6 +3243,9 @@ static int amt_validate(struct nlattr *tb[], struct nlattr *data[],
return -EINVAL;
}
+ if (amt_ip6_in_ip4_attr(data[IFLA_AMT_DISCOVERY_IP], extack))
+ return -EINVAL;
+
return 0;
}
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net v3 4/6] amt: key relay tunnel state on the (address, port) endpoint, not the address
2026-10-12 12:59 [PATCH net v3 0/6] amt: relay tunnel lifetime, validation and request-path hardening Omar Ramadan
` (2 preceding siblings ...)
2026-10-12 12:59 ` [PATCH net v3 3/6] amt: reject an IPv6 address passed in an IPv4 address attribute Omar Ramadan
@ 2026-10-12 12:59 ` Omar Ramadan
2026-10-12 12:59 ` [PATCH net v3 5/6] amt: make pre-query report drops visible Omar Ramadan
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Omar Ramadan @ 2026-10-12 12:59 UTC (permalink / raw)
To: Taehee Yoo, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Simon Horman
amt_request_handler and amt_update_handler both look a tunnel up by the
outer source address alone:
list_for_each_entry_rcu(tunnel, &amt->tunnel_list, list)
if (tunnel->ip4 == iph->saddr)
goto send;
RFC 7450 s4.2.2 defines the unit of relay tunnel state differently: "an
AMT 'tunnel' is identified by the IP address and UDP port pair used as
the destination address for sending encapsulated multicast IP datagrams
to a gateway", and "each unique combination represents a unique tunnel
endpoint".
Because the port term is missing, two distinct endpoints that share a
source address alias onto one tunnel. The second Request to arrive
reaches `send:`, overwrites tunnel->nonce and re-derives tunnel->mac, so
the first gateway's subsequent Membership Updates no longer match and
are dropped at the "Invalid MAC" arm. That arm returns rather than
continuing the walk, so there is no recovery path: the first gateway has
had its Request answered and its membership accepted, and simply never
receives data again. The failure is silent on both sides.
Two deployments reach this, and the same RFC section names both:
- NAT, which s4.2.2 calls out explicitly ("this address may differ from
that carried by the message when it exited the gateway as a result of
network address translation"). CGNAT, a single-WAN site with a
redundant gateway pair, or two subscriber devices behind one
residential NAT all present as one source address.
- A single gateway host, with no NAT anywhere, which s4.2.2 says "may
use separate ports for the IPv4/IGMP and IPv6/MLD protocols".
Add the port term to both lookups. amt_update_handler snapshots the
source port before iptunnel_pull_header() strips the encap, alongside
the existing pre-pull reads.
With the endpoint keyed correctly, a gateway that re-Requests from a new
ephemeral port no longer aliases onto its own previous tunnel: it gets a
new one addressed to the port it is listening on, and the old one ages
out on gc_wq. That is the same stale-Membership-Query symptom addressed
by refreshing tunnel->source_port at `send:`, fixed at the cause instead
-- so this change supersedes that approach rather than stacking on it.
Note this removes an accidental bound: while tunnels were keyed on the
address alone, one source address could never hold more than one tunnel,
whatever it did. RFC 7450 s5.3.3 asks for that bound explicitly, and it
is restored in a companion net-next patch ("amt: bound relay tunnels
admitted per source address") rather than here, since it adds UAPI and
this is a fix.
Fixes: cbc21dc1cfe9 ("amt: add data plane of amt interface")
Assisted-by: LLM
Signed-off-by: Omar Ramadan <omar@blockcast.net>
---
drivers/net/amt.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index 550b0a5..2feb418 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -2492,6 +2492,7 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb)
u64 response_mac;
__be32 saddr;
__be32 nonce;
+ __be16 sport;
saddr = ip_hdr(skb)->saddr;
@@ -2505,6 +2506,8 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb)
nonce = amtmu->nonce;
response_mac = amtmu->response_mac;
+ /* Snapshot the tunnel endpoint port before the encap is stripped. */
+ sport = udp_hdr(skb)->source;
if (iptunnel_pull_header(skb, hdr_size, skb->protocol, false))
return true;
@@ -2512,7 +2515,8 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb)
skb_reset_network_header(skb);
list_for_each_entry_rcu(tunnel, &amt->tunnel_list, list) {
- if (tunnel->ip4 == saddr) {
+ if (tunnel->ip4 == saddr &&
+ tunnel->source_port == sport) {
if ((nonce == tunnel->nonce &&
response_mac == tunnel->mac)) {
/* Refresh liveness only. gc_wq is re-armed by
@@ -2729,7 +2733,8 @@ static bool amt_request_handler(struct amt_dev *amt, struct sk_buff *skb)
return true;
list_for_each_entry_rcu(tunnel, &amt->tunnel_list, list)
- if (tunnel->ip4 == iph->saddr)
+ if (tunnel->ip4 == iph->saddr &&
+ tunnel->source_port == udph->source)
goto send;
spin_lock_bh(&amt->lock);
@@ -2768,6 +2773,12 @@ static bool amt_request_handler(struct amt_dev *amt, struct sk_buff *skb)
spin_unlock_bh(&amt->lock);
send:
+ /* source_port is part of the tunnel's identity and is set once, in
+ * the allocation path above; the lookup only reaches here on an exact
+ * (address, port) match, so it already equals udph->source. Do not
+ * "refresh" it here: doing so let a colliding Request from a new port
+ * steal an established tunnel outright.
+ */
tunnel->nonce = amtrh->nonce;
mac = siphash_3u32((__force u32)tunnel->ip4,
(__force u32)tunnel->source_port,
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net v3 5/6] amt: make pre-query report drops visible
2026-10-12 12:59 [PATCH net v3 0/6] amt: relay tunnel lifetime, validation and request-path hardening Omar Ramadan
` (3 preceding siblings ...)
2026-10-12 12:59 ` [PATCH net v3 4/6] amt: key relay tunnel state on the (address, port) endpoint, not the address Omar Ramadan
@ 2026-10-12 12:59 ` Omar Ramadan
2026-10-12 12:59 ` [PATCH net v3 6/6] amt: do not create tunnel state for unauthenticated Requests Omar Ramadan
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Omar Ramadan @ 2026-10-12 12:59 UTC (permalink / raw)
To: Taehee Yoo, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Simon Horman
A gateway cannot forward an IGMP or MLD report until it has received the
relay's Membership Query for that family. The Query supplies the
response MAC the Update echoes, so dropping an early report is required,
but doing so silently leaves operators with a dark multicast path and no
indication why the join never happened.
Emit a family-specific debug message before taking the existing drop
path. That path already increments tx_dropped, so each discarded report
is accounted exactly once.
Assisted-by: LLM
Signed-off-by: Omar Ramadan <omar@blockcast.net>
---
drivers/net/amt.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index 2feb418..fec3265 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -1249,9 +1249,17 @@ static netdev_tx_t amt_dev_xmit(struct sk_buff *skb, struct net_device *dev)
/* Gateway only passes IGMP/MLD packets */
if (!report)
goto free;
+ /* A validated report can only be forwarded after the relay's
+ * family-specific Membership Query supplies the state echoed
+ * by the Membership Update. Log this readiness failure before
+ * the shared drop path accounts it.
+ */
if ((!v6 && !READ_ONCE(amt->ready4)) ||
- (v6 && !READ_ONCE(amt->ready6)))
+ (v6 && !READ_ONCE(amt->ready6))) {
+ netdev_dbg(dev, "drop %s report: no Membership Query for this family yet\n",
+ v6 ? "MLD" : "IGMP");
goto free;
+ }
if (amt_send_membership_update(amt, skb, v6))
goto free;
goto unlock;
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net v3 6/6] amt: do not create tunnel state for unauthenticated Requests
2026-10-12 12:59 [PATCH net v3 0/6] amt: relay tunnel lifetime, validation and request-path hardening Omar Ramadan
` (4 preceding siblings ...)
2026-10-12 12:59 ` [PATCH net v3 5/6] amt: make pre-query report drops visible Omar Ramadan
@ 2026-10-12 12:59 ` Omar Ramadan
2026-10-12 13:04 ` [PATCH net v3 0/6] amt: relay tunnel lifetime, validation and request-path hardening netdev-bot+sinfo
2026-10-12 21:47 ` Omar Ramadan
7 siblings, 0 replies; 9+ messages in thread
From: Omar Ramadan @ 2026-10-12 12:59 UTC (permalink / raw)
To: Taehee Yoo, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel, Simon Horman
amt_request_handler() allocated a struct amt_tunnel_list for any incoming
Relay Membership Request, keyed on the claimed source endpoint, before
anything about that source had been verified. A Request is a bare UDP
datagram with a trivially spoofable source address, so this gave a remote
attacker two primitives:
- Resource exhaustion. Each entry is held for amt_gmi() (260s with the
default qrv=2/qi=125/qri=10) and the table is bounded by
amt->max_tunnels (default AMT_MAX_TUNNELS, 128). Requests from 128
spoofed addresses fill the table, and re-sending once per interval
keeps it full, so real gateways are refused. Once full, every further
Request also emitted an ICMP_DEST_UNREACH to the spoofed source,
turning the relay into an ICMP reflector.
- Session desynchronisation. The lookup hit at the top of the function
jumped to the send path, which took no lock and overwrote ->nonce and
->mac of an already-established tunnel. One spoofed packet carrying a
known gateway's source endpoint invalidates that gateway's outstanding
(nonce, response_mac), so its next Membership Update is dropped as
"Invalid MAC". This costs one packet, consumes no table slot, and is
therefore unaffected by max_tunnels tuning.
No state actually has to be created at Request time. Every input to the
keyed MAC is carried in the packet or is device state, so the MAC can be
generated on Request and recomputed on Update rather than stored:
- amt_request_handler() now allocates nothing. It computes the MAC and
replies, matching amt_send_advertisement(), which already answers
Discovery statelessly from the same context.
- amt_update_handler() recomputes the expected MAC from the packet's
source address, source port and nonce and drops the packet on
mismatch. Tunnel state is created only after that check passes, in
amt_tunnel_get_or_create(), so max_tunnels now bounds verified
gateways rather than unverified Requests.
- amt->key becomes a three-element array. amt_secret_work() rotates
every AMT_SECRET_TIMEOUT (60s) and an exchange may straddle a
rotation, so Update accepts the current or either of the two previous
secrets. Three generations keep a MAC valid for 120-180s, at least the
amt_gmi() the Query advertised; two generations (120s floor) could
expire a MAC before the interval it promised. A stateless recompute
cannot snapshot the key per tunnel the way the old code did.
Fold in four further issues on the same path:
- Per-source exhaustion. With the table now bounded by verified
gateways, one verified source could still take every slot. The locked
re-check in amt_tunnel_get_or_create() counts the tunnels a source
already holds and refuses past a fixed internal cap
(AMT_MAX_TUNNELS_PER_SRC). This needs no UAPI, so it is a backportable
fix; a configurable per-source bound is left to net-next.
- Table-full killed rediscovery. A Request that arrives when the table
is full, from a source that holds no tunnel, is now refused with ICMP
and no Query, rather than answered with a Query whose later Update
cannot be admitted. Withholding the Query lets the gateway's req_cnt
climb and drive rediscovery. The ICMP is rate-limited and allocates
nothing, unlike the old reflector. A source that holds a tunnel is
still answered, as before: its periodic Request renews the response
MAC its Updates echo.
- Zero-key window at open. amt_dev_open() seeded amt->key only after
amt_socket_create() had already enabled the encap receive path, so a
Request arriving in that window was answered with a MAC keyed on the
all-zero (predictable) secret. Seed the keys under amt->lock before
the socket is created.
- The entry was published by list_add_tail_rcu() before ->nonce and
->mac were assigned, leaving a window in which a concurrent Update
could match the zeroed fields of a kzalloc()'d entry; and the send
path read tunnel->key into an unused local before it was initialised.
Both go away now that nothing is published until it is fully built.
The General Query is passed its destination by value rather than a tunnel
pointer, because at Request time no tunnel exists to point at.
Fixes: cbc21dc1cfe9 ("amt: add data plane of amt interface")
Assisted-by: LLM
Signed-off-by: Omar Ramadan <omar@blockcast.net>
---
drivers/net/amt.c | 310 ++++++++++++++++++++++++++++++++--------------
include/net/amt.h | 17 ++-
2 files changed, 228 insertions(+), 99 deletions(-)
diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index fec3265..2d9f2d9 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -784,11 +784,16 @@ out:
static bool amt_send_membership_query(struct amt_dev *amt,
struct sk_buff *skb,
- struct amt_tunnel_list *tunnel,
+ __be32 daddr, __be16 dport,
+ __be32 nonce, u64 mac,
bool v6);
-static void amt_send_igmp_gq(struct amt_dev *amt,
- struct amt_tunnel_list *tunnel)
+/* The destination is passed by value rather than as a tunnel: the requesting
+ * source is not authenticated yet and no tunnel state exists for it - see
+ * amt_request_handler().
+ */
+static void amt_send_igmp_gq(struct amt_dev *amt, __be32 daddr, __be16 dport,
+ __be32 nonce, u64 mac)
{
struct sk_buff *skb;
@@ -797,7 +802,8 @@ static void amt_send_igmp_gq(struct amt_dev *amt,
return;
skb_pull(skb, sizeof(struct ethhdr));
- if (amt_send_membership_query(amt, skb, tunnel, false)) {
+ if (amt_send_membership_query(amt, skb, daddr, dport, nonce, mac,
+ false)) {
amt->dev->stats.tx_dropped++;
kfree_skb(skb);
}
@@ -876,7 +882,8 @@ static struct sk_buff *amt_build_mld_gq(struct amt_dev *amt)
return skb;
}
-static void amt_send_mld_gq(struct amt_dev *amt, struct amt_tunnel_list *tunnel)
+static void amt_send_mld_gq(struct amt_dev *amt, __be32 daddr, __be16 dport,
+ __be32 nonce, u64 mac)
{
struct sk_buff *skb;
@@ -885,13 +892,15 @@ static void amt_send_mld_gq(struct amt_dev *amt, struct amt_tunnel_list *tunnel)
return;
skb_pull(skb, sizeof(struct ethhdr));
- if (amt_send_membership_query(amt, skb, tunnel, true)) {
+ if (amt_send_membership_query(amt, skb, daddr, dport, nonce, mac,
+ true)) {
amt->dev->stats.tx_dropped++;
kfree_skb(skb);
}
}
#else
-static void amt_send_mld_gq(struct amt_dev *amt, struct amt_tunnel_list *tunnel)
+static void amt_send_mld_gq(struct amt_dev *amt, __be32 daddr, __be16 dport,
+ __be32 nonce, u64 mac)
{
}
#endif
@@ -925,7 +934,13 @@ static void amt_secret_work(struct work_struct *work)
secret_wq);
spin_lock_bh(&amt->lock);
- get_random_bytes(&amt->key, sizeof(siphash_key_t));
+ /* Rotate: keep the two previous generations so a MAC handed out just
+ * before a rotation still verifies for up to three AMT_SECRET_TIMEOUT
+ * windows (120-180s), covering the amt_gmi() the Query advertised.
+ */
+ amt->key[2] = amt->key[1];
+ amt->key[1] = amt->key[0];
+ get_random_bytes(&amt->key[0], sizeof(siphash_key_t));
spin_unlock_bh(&amt->lock);
mod_delayed_work(amt_wq, &amt->secret_wq,
msecs_to_jiffies(AMT_SECRET_TIMEOUT));
@@ -1115,9 +1130,16 @@ static void amt_send_multicast_data(struct amt_dev *amt,
0);
}
+/* Send a Membership Query to a source that has not been authenticated yet.
+ *
+ * No per-source state exists at this point and none is created: the nonce and
+ * the keyed MAC are carried in the Query and recomputed from the Membership
+ * Update when it comes back, so everything this needs is passed by value.
+ */
static bool amt_send_membership_query(struct amt_dev *amt,
struct sk_buff *skb,
- struct amt_tunnel_list *tunnel,
+ __be32 daddr, __be16 dport,
+ __be32 nonce, u64 mac,
bool v6)
{
struct amt_header_membership_query *amtmq;
@@ -1138,13 +1160,13 @@ static bool amt_send_membership_query(struct amt_dev *amt,
skb_reset_inner_headers(skb);
memset(&fl4, 0, sizeof(struct flowi4));
fl4.flowi4_oif = amt->stream_dev->ifindex;
- fl4.daddr = tunnel->ip4;
+ fl4.daddr = daddr;
fl4.saddr = amt->local_ip;
fl4.flowi4_dscp = inet_dsfield_to_dscp(AMT_TOS);
fl4.flowi4_proto = IPPROTO_UDP;
rt = ip_route_output_key(amt->net, &fl4);
if (IS_ERR(rt)) {
- netdev_dbg(amt->dev, "no route to %pI4\n", &tunnel->ip4);
+ netdev_dbg(amt->dev, "no route to %pI4\n", &daddr);
return true;
}
@@ -1154,8 +1176,8 @@ static bool amt_send_membership_query(struct amt_dev *amt,
amtmq->reserved = 0;
amtmq->l = 0;
amtmq->g = 0;
- amtmq->nonce = tunnel->nonce;
- amtmq->response_mac = tunnel->mac;
+ amtmq->nonce = nonce;
+ amtmq->response_mac = mac;
if (!v6)
skb_set_inner_protocol(skb, htons(ETH_P_IP));
@@ -1168,11 +1190,10 @@ static bool amt_send_membership_query(struct amt_dev *amt,
ip4_dst_hoplimit(&rt->dst),
0,
amt->relay_port,
- tunnel->source_port,
+ dport,
false,
false,
0);
- amt_update_relay_status(tunnel, AMT_STATUS_SENT_QUERY, true);
return false;
}
@@ -2490,14 +2511,99 @@ static bool amt_membership_query_handler(struct amt_dev *amt,
return false;
}
+/* The tunnel of the gateway endpoint @saddr:@sport, or NULL. */
+static struct amt_tunnel_list *amt_tunnel_lookup(struct amt_dev *amt,
+ __be32 saddr, __be16 sport)
+{
+ struct amt_tunnel_list *tunnel;
+
+ list_for_each_entry_rcu(tunnel, &amt->tunnel_list, list)
+ if (tunnel->ip4 == saddr && tunnel->source_port == sport)
+ return tunnel;
+ return NULL;
+}
+
+/* Look up the tunnel for a source whose Membership Update has already been
+ * authenticated, creating it on first contact.
+ *
+ * This is now the only place tunnel state is allocated, so amt->max_tunnels
+ * bounds the number of *verified* gateways rather than the number of
+ * unverified Requests anyone can send. A fixed per-source cap additionally
+ * stops one verified endpoint from monopolising the table; a single gateway
+ * needs at most one tunnel per family (RFC 7450 s4.2.2).
+ */
+static struct amt_tunnel_list *amt_tunnel_get_or_create(struct amt_dev *amt,
+ __be32 saddr,
+ __be16 sport)
+{
+ struct amt_tunnel_list *tunnel;
+ int per_src = 0;
+ int i;
+
+ tunnel = amt_tunnel_lookup(amt, saddr, sport);
+ if (tunnel)
+ return tunnel;
+
+ spin_lock_bh(&amt->lock);
+
+ /* Re-check under the lock in case a concurrent Update won the race, and
+ * count how many tunnels this source already holds on the same walk.
+ */
+ list_for_each_entry(tunnel, &amt->tunnel_list, list) {
+ if (tunnel->ip4 != saddr)
+ continue;
+ if (tunnel->source_port == sport) {
+ spin_unlock_bh(&amt->lock);
+ return tunnel;
+ }
+ per_src++;
+ }
+
+ if (amt->nr_tunnels >= amt->max_tunnels ||
+ per_src >= AMT_MAX_TUNNELS_PER_SRC) {
+ spin_unlock_bh(&amt->lock);
+ return NULL;
+ }
+
+ tunnel = kzalloc(sizeof(*tunnel) +
+ (sizeof(struct hlist_head) * amt->hash_buckets),
+ GFP_ATOMIC);
+ if (!tunnel) {
+ spin_unlock_bh(&amt->lock);
+ return NULL;
+ }
+
+ tunnel->source_port = sport;
+ tunnel->ip4 = saddr;
+ tunnel->amt = amt;
+ spin_lock_init(&tunnel->lock);
+ for (i = 0; i < amt->hash_buckets; i++)
+ INIT_HLIST_HEAD(&tunnel->groups[i]);
+
+ tunnel->last_seen = jiffies;
+ INIT_DELAYED_WORK(&tunnel->gc_wq, amt_tunnel_expire);
+ __amt_update_relay_status(tunnel, AMT_STATUS_RECEIVED_UPDATE, false);
+
+ /* Publish only once the entry is fully initialised. */
+ list_add_tail_rcu(&tunnel->list, &amt->tunnel_list);
+ amt->nr_tunnels++;
+ mod_delayed_work(amt_wq, &tunnel->gc_wq,
+ msecs_to_jiffies(amt_gmi(amt)) * 3);
+ spin_unlock_bh(&amt->lock);
+
+ return tunnel;
+}
+
static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb)
{
struct amt_header_membership_update *amtmu;
struct amt_tunnel_list *tunnel;
struct ethhdr *eth;
struct iphdr *iph;
- int len, hdr_size;
+ siphash_key_t key[3];
+ int len, hdr_size, i;
u64 response_mac;
+ bool verified = false;
__be32 saddr;
__be32 nonce;
__be16 sport;
@@ -2517,33 +2623,57 @@ static bool amt_update_handler(struct amt_dev *amt, struct sk_buff *skb)
/* Snapshot the tunnel endpoint port before the encap is stripped. */
sport = udp_hdr(skb)->source;
- if (iptunnel_pull_header(skb, hdr_size, skb->protocol, false))
- return true;
+ /* Recompute the MAC handed out in the Membership Query rather than
+ * comparing against a stored copy. All three live secrets are accepted
+ * so an exchange straddling one or two rotations by amt_secret_work() is
+ * not spuriously rejected (three generations cover 120-180s, at least
+ * the amt_gmi() the Query advertised). This runs before the packet is
+ * decapsulated and before any tunnel state is looked up or created, so
+ * an unverified source is rejected with no further work.
+ */
+ spin_lock_bh(&amt->lock);
+ for (i = 0; i < ARRAY_SIZE(key); i++)
+ key[i] = amt->key[i];
+ spin_unlock_bh(&amt->lock);
- skb_reset_network_header(skb);
+ for (i = 0; i < ARRAY_SIZE(key); i++) {
+ u64 mac = siphash_3u32((__force u32)saddr,
+ (__force u32)sport,
+ (__force u32)nonce,
+ &key[i]) >> 16;
- list_for_each_entry_rcu(tunnel, &amt->tunnel_list, list) {
- if (tunnel->ip4 == saddr &&
- tunnel->source_port == sport) {
- if ((nonce == tunnel->nonce &&
- response_mac == tunnel->mac)) {
- /* Refresh liveness only. gc_wq is re-armed by
- * amt_tunnel_expire() itself from last_seen, never
- * from here, so this cannot re-queue the work of a
- * tunnel that is being freed.
- */
- WRITE_ONCE(tunnel->last_seen, jiffies);
- goto report;
- } else {
- netdev_dbg(amt->dev, "Invalid MAC\n");
- return true;
- }
+ if (response_mac == mac) {
+ verified = true;
+ break;
}
}
- return true;
+ if (!verified) {
+ netdev_dbg(amt->dev, "Invalid MAC\n");
+ return true;
+ }
+
+ tunnel = amt_tunnel_get_or_create(amt, saddr, sport);
+ if (!tunnel) {
+ /* Out of tunnel slots. Unlike the Request path this reply only
+ * ever goes to a source that proved it received our Query, so it
+ * cannot be used to reflect at a spoofed third party.
+ */
+ icmp_ndo_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_UNREACH, 0);
+ return true;
+ }
+
+ /* Refresh liveness only. gc_wq is re-armed by amt_tunnel_expire()
+ * itself from last_seen, never from here, so this cannot re-queue the
+ * work of a tunnel that is being freed.
+ */
+ WRITE_ONCE(tunnel->last_seen, jiffies);
+
+ if (iptunnel_pull_header(skb, hdr_size, skb->protocol, false))
+ return true;
+
+ skb_reset_network_header(skb);
-report:
if (!pskb_may_pull(skb, sizeof(*iph)))
return true;
@@ -2720,15 +2850,27 @@ static bool amt_discovery_handler(struct amt_dev *amt, struct sk_buff *skb)
return false;
}
+/* Handle an AMT Relay Membership Request.
+ *
+ * The source of a Request is unauthenticated: it is a bare UDP datagram with a
+ * trivially spoofable source address. Allocating tunnel state here let an
+ * attacker fill the table from spoofed addresses, and let one spoofed packet
+ * overwrite the nonce/MAC of an established tunnel and cut that gateway off.
+ *
+ * Nothing needs to be remembered at this point. Every input to the keyed MAC
+ * is carried in the packet or is device state, so the MAC is generated here,
+ * echoed back by the gateway in its Membership Update, and recomputed and
+ * verified there - see amt_update_handler(). Tunnel state is created only once
+ * that verification succeeds.
+ */
static bool amt_request_handler(struct amt_dev *amt, struct sk_buff *skb)
{
struct amt_header_request *amtrh;
- struct amt_tunnel_list *tunnel;
- unsigned long long key;
struct udphdr *udph;
struct iphdr *iph;
+ siphash_key_t key;
+ bool full;
u64 mac;
- int i;
if (!pskb_may_pull(skb, sizeof(*udph) + sizeof(*amtrh)))
return true;
@@ -2740,67 +2882,42 @@ static bool amt_request_handler(struct amt_dev *amt, struct sk_buff *skb)
if (amtrh->reserved1 || amtrh->reserved2 || amtrh->version)
return true;
- list_for_each_entry_rcu(tunnel, &amt->tunnel_list, list)
- if (tunnel->ip4 == iph->saddr &&
- tunnel->source_port == udph->source)
- goto send;
-
- spin_lock_bh(&amt->lock);
- if (amt->nr_tunnels >= amt->max_tunnels) {
- spin_unlock_bh(&amt->lock);
- icmp_ndo_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_UNREACH, 0);
- return true;
- }
-
- tunnel = kzalloc(sizeof(*tunnel) +
- (sizeof(struct hlist_head) * amt->hash_buckets),
- GFP_ATOMIC);
- if (!tunnel) {
- spin_unlock_bh(&amt->lock);
+ if (!netif_running(amt->dev) || !netif_running(amt->stream_dev))
return true;
- }
- tunnel->source_port = udph->source;
- tunnel->ip4 = iph->saddr;
-
- memcpy(&key, &tunnel->key, sizeof(unsigned long long));
- tunnel->amt = amt;
- spin_lock_init(&tunnel->lock);
- for (i = 0; i < amt->hash_buckets; i++)
- INIT_HLIST_HEAD(&tunnel->groups[i]);
-
- tunnel->last_seen = jiffies;
- INIT_DELAYED_WORK(&tunnel->gc_wq, amt_tunnel_expire);
-
- list_add_tail_rcu(&tunnel->list, &amt->tunnel_list);
- tunnel->key = amt->key;
- __amt_update_relay_status(tunnel, AMT_STATUS_RECEIVED_REQUEST, true);
- amt->nr_tunnels++;
- mod_delayed_work(amt_wq, &tunnel->gc_wq,
- msecs_to_jiffies(amt_gmi(amt)));
+ /* No state is created for a Request (see amt_update_handler()); read the
+ * current secret and the table occupancy together under the lock.
+ */
+ spin_lock_bh(&amt->lock);
+ key = amt->key[0];
+ full = amt->nr_tunnels >= amt->max_tunnels;
spin_unlock_bh(&amt->lock);
-send:
- /* source_port is part of the tunnel's identity and is set once, in
- * the allocation path above; the lookup only reaches here on an exact
- * (address, port) match, so it already equals udph->source. Do not
- * "refresh" it here: doing so let a colliding Request from a new port
- * steal an established tunnel outright.
+ /* When the table is full, refuse a source that holds no tunnel without
+ * sending a Query. A silent Query would keep the gateway believing the
+ * exchange is progressing while its later, verified Update is refused;
+ * withholding it lets the gateway's req_cnt climb and drive rediscovery
+ * instead. The ICMP is rate-limited and no state is allocated, so this
+ * is not the reflector the pre-verification allocation path used to be.
+ * A source that holds a tunnel is answered: its periodic Request is how
+ * it renews the response MAC its Updates echo.
*/
- tunnel->nonce = amtrh->nonce;
- mac = siphash_3u32((__force u32)tunnel->ip4,
- (__force u32)tunnel->source_port,
- (__force u32)tunnel->nonce,
- &tunnel->key);
- tunnel->mac = mac >> 16;
-
- if (!netif_running(amt->dev) || !netif_running(amt->stream_dev))
+ if (full && !amt_tunnel_lookup(amt, iph->saddr, udph->source)) {
+ icmp_ndo_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_UNREACH, 0);
return true;
+ }
+
+ mac = siphash_3u32((__force u32)iph->saddr,
+ (__force u32)udph->source,
+ (__force u32)amtrh->nonce,
+ &key) >> 16;
if (!amtrh->p)
- amt_send_igmp_gq(amt, tunnel);
+ amt_send_igmp_gq(amt, iph->saddr, udph->source, amtrh->nonce,
+ mac);
else
- amt_send_mld_gq(amt, tunnel);
+ amt_send_mld_gq(amt, iph->saddr, udph->source, amtrh->nonce,
+ mac);
return false;
}
@@ -3068,6 +3185,14 @@ static int amt_dev_open(struct net_device *dev)
enable_delayed_work(&amt->discovery_wq);
enable_delayed_work(&amt->req_wq);
+ /* Seed the keys before amt_socket_create() enables the encap receive
+ * path, so a Request or Update that arrives immediately is never
+ * verified against an all-zero (predictable) secret.
+ */
+ spin_lock_bh(&amt->lock);
+ get_random_bytes(&amt->key, sizeof(amt->key));
+ spin_unlock_bh(&amt->lock);
+
err = amt_socket_create(amt);
if (err) {
disable_delayed_work(&amt->req_wq);
@@ -3078,7 +3203,6 @@ static int amt_dev_open(struct net_device *dev)
amt->req_cnt = 0;
WRITE_ONCE(amt->remote_ip, 0);
amt->nonce = 0;
- get_random_bytes(&amt->key, sizeof(siphash_key_t));
amt->status = AMT_STATUS_INIT;
if (amt->mode == AMT_MODE_GATEWAY) {
diff --git a/include/net/amt.h b/include/net/amt.h
index 0520121..1c7c144 100644
--- a/include/net/amt.h
+++ b/include/net/amt.h
@@ -253,10 +253,6 @@ struct amt_tunnel_list {
bool dead;
__be16 source_port;
__be32 ip4;
- __be32 nonce;
- siphash_key_t key;
- u64 mac:48,
- reserved:16;
struct rcu_head rcu;
struct hlist_head groups[];
};
@@ -336,8 +332,11 @@ struct amt_dev {
struct work_struct event_wq;
/* AMT status */
enum amt_status status;
- /* Generated key */
- siphash_key_t key;
+ /* Generated keys. key[0] is current; key[1] and key[2] are the two
+ * previous generations, kept so a Request/Update exchange straddling
+ * one or two secret rotations still verifies (see amt_secret_work()).
+ */
+ siphash_key_t key[3];
struct sock __rcu *sk;
u32 max_groups;
u32 max_sources;
@@ -394,6 +393,12 @@ struct amt_dev {
#define AMT_SECRET_TIMEOUT 60000
#define IANA_AMT_UDP_PORT 2268
#define AMT_MAX_TUNNELS 128
+/* Per-source tunnel cap. A single gateway needs at most one tunnel per
+ * family; this stops one verified source endpoint from monopolising the
+ * AMT_MAX_TUNNELS-slot table. Deployments that legitimately need more behind
+ * one address (e.g. CGNAT) use the configurable net-next per-source bound.
+ */
+#define AMT_MAX_TUNNELS_PER_SRC 16
#define AMT_MAX_REQS 128
#define AMT_GW_HLEN (sizeof(struct iphdr) + \
sizeof(struct udphdr) + \
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v3 0/6] amt: relay tunnel lifetime, validation and request-path hardening
2026-10-12 12:59 [PATCH net v3 0/6] amt: relay tunnel lifetime, validation and request-path hardening Omar Ramadan
` (5 preceding siblings ...)
2026-10-12 12:59 ` [PATCH net v3 6/6] amt: do not create tunnel state for unauthenticated Requests Omar Ramadan
@ 2026-10-12 13:04 ` netdev-bot+sinfo
2026-10-12 21:47 ` Omar Ramadan
7 siblings, 0 replies; 9+ messages in thread
From: netdev-bot+sinfo @ 2026-10-12 13:04 UTC (permalink / raw)
To: Omar Ramadan
Cc: Taehee Yoo, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, linux-kernel, Simon Horman
Hi!
This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:
- Whether the issue was actually triggered, or is only theoretical
(e.g. found by code inspection). If it was triggered please include
the symptoms, like the stack trace or error messages.
Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.
The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v3 0/6] amt: relay tunnel lifetime, validation and request-path hardening
2026-10-12 12:59 [PATCH net v3 0/6] amt: relay tunnel lifetime, validation and request-path hardening Omar Ramadan
` (6 preceding siblings ...)
2026-10-12 13:04 ` [PATCH net v3 0/6] amt: relay tunnel lifetime, validation and request-path hardening netdev-bot+sinfo
@ 2026-10-12 21:47 ` Omar Ramadan
7 siblings, 0 replies; 9+ messages in thread
From: Omar Ramadan @ 2026-10-12 21:47 UTC (permalink / raw)
To: netdev-bot+sinfo
Cc: Taehee Yoo, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, linux-kernel, Simon Horman
Thanks. 4/6-6/6 are carried over from v2, where the same question was
answered [1]. 1/6-3/6 are new in v3, so per patch:
1/6 (gc_wq re-arm use-after-free): theoretical. The Sashiko review of v2
pointed it out, and reading amt_update_handler() against
amt_tunnel_expire() and amt_dev_stop() confirms it. It needs a Membership
Update that has already found the tunnel under RCU to arrive while
amt_tunnel_expire() is freeing that tunnel. I have not triggered it, so
there is no trace to attach. After the patch the Update path no longer
touches gc_wq, so it cannot re-arm a tunnel that is being freed.
2/6 (Query or Data from 0.0.0.0): theoretical, found by inspection while
writing the IPv6 receive path in the net-next series [2]. Sashiko's review
of that series asked for the IPv4 part to be split out for net. While
remote_ip is still 0, "saddr != remote_ip" is false for a 0.0.0.0 source,
so a 0.0.0.0 -> 255.255.255.255 broadcast reaching the socket is accepted.
Not observed.
3/6 (IPv6 literal in an IPv4 attribute): triggered, while writing the IPv6
selftest. An iproute2 without the IPv6 attributes puts all 16 bytes of
"discovery 2001:db8:a::2" into IFLA_AMT_DISCOVERY_IP. On a stock kernel
the command exited 0 with no error or warning, and the device came up as
an IPv4 gateway with discovery address 32.1.13.184, the first four bytes.
There is no oops or trace: the symptom is the silently wrong address,
which the selftest's capability probe was fooled by and now checks for.
With the patch the attribute is refused with an extack.
Fix testing is in the cover letter: W=1 clean, a KASAN + lockdep +
PROVE_RCU boot running the amt selftests with no splat, and amt.sh with
max_tunnels 1.
[1] https://lore.kernel.org/netdev/20261009213749.2226283-1-omar@blockcast.net/
[2] https://lore.kernel.org/netdev/20261009122426.551178-1-omar@blockcast.net/
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-10-12 21:47 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-12 12:59 [PATCH net v3 0/6] amt: relay tunnel lifetime, validation and request-path hardening Omar Ramadan
2026-10-12 12:59 ` [PATCH net v3 1/6] amt: fix use-after-free when a Membership Update re-arms a freed tunnel Omar Ramadan
2026-10-12 12:59 ` [PATCH net v3 2/6] amt: reject a gateway Query or Data from 0.0.0.0 before a relay is learned Omar Ramadan
2026-10-12 12:59 ` [PATCH net v3 3/6] amt: reject an IPv6 address passed in an IPv4 address attribute Omar Ramadan
2026-10-12 12:59 ` [PATCH net v3 4/6] amt: key relay tunnel state on the (address, port) endpoint, not the address Omar Ramadan
2026-10-12 12:59 ` [PATCH net v3 5/6] amt: make pre-query report drops visible Omar Ramadan
2026-10-12 12:59 ` [PATCH net v3 6/6] amt: do not create tunnel state for unauthenticated Requests Omar Ramadan
2026-10-12 13:04 ` [PATCH net v3 0/6] amt: relay tunnel lifetime, validation and request-path hardening netdev-bot+sinfo
2026-10-12 21:47 ` Omar Ramadan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®