[PATCH net-next] icmp: pass RFC 8335 reply messages to ping_rcv

[PATCH net-next] icmp: pass RFC 8335 reply messages to ping_rcv

[Andreas Roeseler]

The current icmp_rcv function drops all unknown ICMP types, including
ICMP_EXT_ECHOREPLY (type 43). In order to parse Extended Echo Reply messages, we have
to pass these packets to the ping_rcv function, which does not do any
other filtering and passes the packet to the designated socket.

Pass incoming RFC 8335 ICMP Extended Echo Reply packets to the ping_rcv
handler instead of discarding the packet.

Signed-off-by: Andreas Roeseler <andreas.a.roeseler@gmail.com>
---
 net/ipv4/icmp.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 76990e13a2f9..8bd988fbcb31 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -1196,6 +1196,11 @@ int icmp_rcv(struct sk_buff *skb)
 		goto success_check;
 	}
 
+	if (icmph->type == ICMP_EXT_ECHOREPLY) {
+		success = ping_rcv(skb);
+		goto success_check;
+	}
+
 	/*
 	 *	18 is the highest 'known' ICMP type. Anything else is a mystery
 	 *
-- 
2.30.0

Re: [PATCH net-next] icmp: pass RFC 8335 reply messages to ping_rcv

[Willem de Bruijn]

On Mon, Apr 12, 2021 at 3:09 PM Andreas Roeseler
<andreas.a.roeseler@gmail.com> wrote:
>
> The current icmp_rcv function drops all unknown ICMP types, including
> ICMP_EXT_ECHOREPLY (type 43). In order to parse Extended Echo Reply messages, we have
> to pass these packets to the ping_rcv function, which does not do any
> other filtering and passes the packet to the designated socket.
>
> Pass incoming RFC 8335 ICMP Extended Echo Reply packets to the ping_rcv
> handler instead of discarding the packet.
>
> Signed-off-by: Andreas Roeseler <andreas.a.roeseler@gmail.com>
> ---
>  net/ipv4/icmp.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index 76990e13a2f9..8bd988fbcb31 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -1196,6 +1196,11 @@ int icmp_rcv(struct sk_buff *skb)
>                 goto success_check;
>         }
>
> +       if (icmph->type == ICMP_EXT_ECHOREPLY) {
> +               success = ping_rcv(skb);
> +               goto success_check;
> +       }
> +

Do you need the same for ICMPV6_EXT_ECHO_REPLY ?

Re: [PATCH net-next] icmp: pass RFC 8335 reply messages to ping_rcv

[Andreas Roeseler]

On Mon, 2021-04-12 at 15:28 -0400, Willem de Bruijn wrote:
> On Mon, Apr 12, 2021 at 3:09 PM Andreas Roeseler
> <andreas.a.roeseler@gmail.com> wrote:
> > 
> > The current icmp_rcv function drops all unknown ICMP types,
> > including
> > ICMP_EXT_ECHOREPLY (type 43). In order to parse Extended Echo Reply
> > messages, we have
> > to pass these packets to the ping_rcv function, which does not do
> > any
> > other filtering and passes the packet to the designated socket.
> > 
> > Pass incoming RFC 8335 ICMP Extended Echo Reply packets to the
> > ping_rcv
> > handler instead of discarding the packet.
> > 
> > Signed-off-by: Andreas Roeseler <andreas.a.roeseler@gmail.com>
> > ---
> >  net/ipv4/icmp.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> > index 76990e13a2f9..8bd988fbcb31 100644
> > --- a/net/ipv4/icmp.c
> > +++ b/net/ipv4/icmp.c
> > @@ -1196,6 +1196,11 @@ int icmp_rcv(struct sk_buff *skb)
> >                 goto success_check;
> >         }
> > 
> > +       if (icmph->type == ICMP_EXT_ECHOREPLY) {
> > +               success = ping_rcv(skb);
> > +               goto success_check;
> > +       }
> > +
> 
> Do you need the same for ICMPV6_EXT_ECHO_REPLY ?

Yes, but this should be handled in icmpv6_rcv in net/ipv6/icmp.c and
we're thinking of including all icmpv6 support for RFC 8335 (replying
and parsing replies) in a separate patch.

Re: [PATCH net-next] icmp: pass RFC 8335 reply messages to ping_rcv

[Willem de Bruijn]

On Mon, Apr 12, 2021 at 3:41 PM Andreas Roeseler
<andreas.a.roeseler@gmail.com> wrote:
>
> On Mon, 2021-04-12 at 15:28 -0400, Willem de Bruijn wrote:
> > On Mon, Apr 12, 2021 at 3:09 PM Andreas Roeseler
> > <andreas.a.roeseler@gmail.com> wrote:
> > >
> > > The current icmp_rcv function drops all unknown ICMP types,
> > > including
> > > ICMP_EXT_ECHOREPLY (type 43). In order to parse Extended Echo Reply
> > > messages, we have
> > > to pass these packets to the ping_rcv function, which does not do
> > > any
> > > other filtering and passes the packet to the designated socket.
> > >
> > > Pass incoming RFC 8335 ICMP Extended Echo Reply packets to the
> > > ping_rcv
> > > handler instead of discarding the packet.
> > >
> > > Signed-off-by: Andreas Roeseler <andreas.a.roeseler@gmail.com>
> > > ---
> > >  net/ipv4/icmp.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> > > index 76990e13a2f9..8bd988fbcb31 100644
> > > --- a/net/ipv4/icmp.c
> > > +++ b/net/ipv4/icmp.c
> > > @@ -1196,6 +1196,11 @@ int icmp_rcv(struct sk_buff *skb)
> > >                 goto success_check;
> > >         }
> > >
> > > +       if (icmph->type == ICMP_EXT_ECHOREPLY) {
> > > +               success = ping_rcv(skb);
> > > +               goto success_check;
> > > +       }
> > > +
> >
> > Do you need the same for ICMPV6_EXT_ECHO_REPLY ?
>
> Yes, but this should be handled in icmpv6_rcv in net/ipv6/icmp.c and
> we're thinking of including all icmpv6 support for RFC 8335 (replying
> and parsing replies) in a separate patch.

Please send them together in the same patchset.

Sending ipv4 and ipv6 separately can lead to missing or subtly
differently implemented features. It's preferable to be able to review
both at the same time.