[PATCH net] amt: pull the AMT header behind the transport header in amt_parse_type()

[PATCH net] amt: pull the AMT header behind the transport header in amt_parse_type()

[Omar Ramadan]

A gateway's encap socket passes ICMP errors to amt_err_lookup(), which
calls amt_parse_type() on the quoted datagram to see which AMT message
failed. On that path skb->data points at the quoted IP header and the
transport header at the quoted UDP header, and icmp_socket_deliver()
only guarantees the quoted IP header plus 8 bytes, that is, up to the
end of the UDP header.

amt_parse_type() pulls sizeof(struct udphdr) + sizeof(struct amt_header)
bytes from skb->data, which on this path stays inside the quoted IP
header, and then reads the AMT header behind udp_hdr(skb). An ICMP error
that quotes only the IP and UDP headers of a Request, the minimum RFC 792
asks for, therefore makes it read past the pulled data, and past the end
of the packet when nothing follows.

Pull up to the transport header plus the UDP and AMT headers, as
vxlan_err_lookup() does. amt_rcv() is called with the transport header
at skb->data, so the pull on the receive path does not change.

Fixes: cbc21dc1cfe9 ("amt: add data plane of amt interface")
Signed-off-by: Omar Ramadan <omar@blockcast.net>
---
 drivers/net/amt.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/amt.c b/drivers/net/amt.c
index bddc24e18..0277e4cac 100644
--- a/drivers/net/amt.c
+++ b/drivers/net/amt.c
@@ -1310,8 +1310,12 @@ static int amt_parse_type(struct sk_buff *skb)
 {
 	struct amt_header *amth;
 
-	if (!pskb_may_pull(skb, sizeof(struct udphdr) +
-			   sizeof(struct amt_header)))
+	/* skb->data is the UDP header on receive, but the quoted IP header
+	 * when amt_err_lookup() parses an ICMP error, so pull up to the
+	 * transport header rather than from skb->data.
+	 */
+	if (!pskb_may_pull(skb, skb_transport_offset(skb) +
+			   sizeof(struct udphdr) + sizeof(struct amt_header)))
 		return -1;
 
 	amth = (struct amt_header *)(udp_hdr(skb) + 1);

base-commit: a7bfaba4823e3c165bb2004c74eff7c096672bc7
-- 
2.47.3

Re: [PATCH net] amt: pull the AMT header behind the transport header in amt_parse_type()

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - 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.

Re: [PATCH net] amt: pull the AMT header behind the transport header in amt_parse_type()

[Omar Ramadan]

Found by code inspection: an LLM-assisted review of drivers/net/amt.c
while I was working on an IPv6 outer transport for amt, whose ICMPv6
errors would go through amt_err_lookup() the same way.

It has not been triggered. I have no reproducer; it comes from reading
the code. It needs an ICMP error reaching a gateway's encapsulation
socket that quotes only the IP and UDP headers of a Request, so that
amt_parse_type() reads the AMT header past the data it pulled.

Testing: W=1 build with CONFIG_IPV6=y and =n, and
tools/testing/selftests/net/amt.sh passes with the patch applied, but
no test injects such an ICMP error, so nothing exercises the changed
path.

Re: [PATCH net] amt: pull the AMT header behind the transport header in amt_parse_type()

[Eric Dumazet]

On Mon, Sep 28, 2026 at 8:16 PM Omar Ramadan <omar@blockcast.net> wrote:
>
> A gateway's encap socket passes ICMP errors to amt_err_lookup(), which
> calls amt_parse_type() on the quoted datagram to see which AMT message
> failed. On that path skb->data points at the quoted IP header and the
> transport header at the quoted UDP header, and icmp_socket_deliver()
> only guarantees the quoted IP header plus 8 bytes, that is, up to the
> end of the UDP header.
>
> amt_parse_type() pulls sizeof(struct udphdr) + sizeof(struct amt_header)
> bytes from skb->data, which on this path stays inside the quoted IP
> header, and then reads the AMT header behind udp_hdr(skb). An ICMP error
> that quotes only the IP and UDP headers of a Request, the minimum RFC 792
> asks for, therefore makes it read past the pulled data, and past the end
> of the packet when nothing follows.
>
> Pull up to the transport header plus the UDP and AMT headers, as
> vxlan_err_lookup() does. amt_rcv() is called with the transport header
> at skb->data, so the pull on the receive path does not change.
>
> Fixes: cbc21dc1cfe9 ("amt: add data plane of amt interface")
> Signed-off-by: Omar Ramadan <omar@blockcast.net>
> ---

Reviewed-by: Eric Dumazet <edumazet@@google.com>

Please read https://lore.kernel.org/netdev/20260928181557.85796-1-omar@blockcast.net/

Re: [PATCH net] amt: pull the AMT header behind the transport header in amt_parse_type()

[Eric Dumazet]

On Thu, Oct 1, 2026 at 9:15 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> On Mon, Sep 28, 2026 at 8:16 PM Omar Ramadan <omar@blockcast.net> wrote:
> >
> > A gateway's encap socket passes ICMP errors to amt_err_lookup(), which
> > calls amt_parse_type() on the quoted datagram to see which AMT message
> > failed. On that path skb->data points at the quoted IP header and the
> > transport header at the quoted UDP header, and icmp_socket_deliver()
> > only guarantees the quoted IP header plus 8 bytes, that is, up to the
> > end of the UDP header.
> >
> > amt_parse_type() pulls sizeof(struct udphdr) + sizeof(struct amt_header)
> > bytes from skb->data, which on this path stays inside the quoted IP
> > header, and then reads the AMT header behind udp_hdr(skb). An ICMP error
> > that quotes only the IP and UDP headers of a Request, the minimum RFC 792
> > asks for, therefore makes it read past the pulled data, and past the end
> > of the packet when nothing follows.
> >
> > Pull up to the transport header plus the UDP and AMT headers, as
> > vxlan_err_lookup() does. amt_rcv() is called with the transport header
> > at skb->data, so the pull on the receive path does not change.
> >
> > Fixes: cbc21dc1cfe9 ("amt: add data plane of amt interface")
> > Signed-off-by: Omar Ramadan <omar@blockcast.net>
> > ---
>
> Reviewed-by: Eric Dumazet <edumazet@@google.com>

This was meant to be:

Reviewed-by: Eric Dumazet <edumazet@kernel.org>

>
> Please read https://lore.kernel.org/netdev/20260928181557.85796-1-omar@blockcast.net/

Re: [PATCH net] amt: pull the AMT header behind the transport header in amt_parse_type()

[Paolo Abeni]

On 10/1/26 00:27, Omar Ramadan wrote:
> Found by code inspection: an LLM-assisted review of drivers/net/amt.c
> while I was working on an IPv6 outer transport for amt, whose ICMPv6
> errors would go through amt_err_lookup() the same way.
> 
> It has not been triggered. I have no reproducer; it comes from reading
> the code. It needs an ICMP error reaching a gateway's encapsulation
> socket that quotes only the IP and UDP headers of a Request, so that
> amt_parse_type() reads the AMT header past the data it pulled.
> 
> Testing: W=1 build with CONFIG_IPV6=y and =n, and
> tools/testing/selftests/net/amt.sh passes with the patch applied, but
> no test injects such an ICMP error, so nothing exercises the changed
> path.

Due to all the above, plus 'broken since the beginning' and Linus
asking only for critical fixes at this point of the cycle, I'm diverting
this one to net-next.

/P

Re: [PATCH net] amt: pull the AMT header behind the transport header in amt_parse_type()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon, 28 Sep 2026 21:15:57 +0300 you wrote:
> A gateway's encap socket passes ICMP errors to amt_err_lookup(), which
> calls amt_parse_type() on the quoted datagram to see which AMT message
> failed. On that path skb->data points at the quoted IP header and the
> transport header at the quoted UDP header, and icmp_socket_deliver()
> only guarantees the quoted IP header plus 8 bytes, that is, up to the
> end of the UDP header.
> 
> [...]

Here is the summary with links:
  - [net] amt: pull the AMT header behind the transport header in amt_parse_type()
    https://git.kernel.org/netdev/net-next/c/eb0c18404c89

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net] amt: pull the AMT header behind the transport header in amt_parse_type()

[Omar Ramadan]

On Thu, Oct 1, 2026, Paolo Abeni wrote:
> Due to all the above, plus 'broken since the beginning' and Linus
> asking only for critical fixes at this point of the cycle, I'm diverting
> this one to net-next.

Thanks, Paolo, net-next is fine.

For the record, since the commit message overstates it: from reading
the code again (not reproduced), the byte read behind the quoted UDP
header stays inside the skb's head allocation, so KASAN would not
report it. The effect is a misparsed type: amt_err_lookup() counts the
error in rx_dropped instead of resending its Request at once, or
resends one it did not need to.

My reply to Eric promising a v2 with an Assisted-by: tag went to him
only, by mistake; with the patch applied there will be no v2.