Subject: Re: [PATCH net-next] improve handling of ICMP_EXT_ECHO icmp type

From: Jakub Kicinski <[email-masked]>
Date: Thu, 14 Jul 2022 21:51:32 -0700

On Thu, 14 Jul 2022 17:13:58 +0200 Mathias Lark wrote:
> Introduce a helper for icmp type checking - icmp_is_valid_type.
> 
> There is a number of code paths handling ICMP packets. To check
> icmp type validity, some of those code paths perform the check
> `type <= NR_ICMP_TYPES`. Since the introduction of ICMP_EXT_ECHO
> and ICMP_EXT_ECHOREPLY (RFC 8335), this check is no longer correct.
> 
> To fix this inconsistency and avoid further problems with future
> ICMP types, the patch inserts the icmp_is_valid_type helper
> wherever it is required.

Would be good to note in the commit message why we can't bump
NR_ICMP_TYPES to include all the types. What are the types between 
18 and 42?

> diff --git a/include/uapi/linux/icmp.h b/include/uapi/linux/icmp.h
> index 163c0998aec9..ad736a24f0c8 100644
> --- a/include/uapi/linux/icmp.h
> +++ b/include/uapi/linux/icmp.h
> @@ -159,4 +159,9 @@ struct icmp_ext_echo_iio {
>  		} addr;
>  	} ident;
>  };
> +
> +static inline bool icmp_is_valid_type(__u8 type)
> +{
> +	return type <= NR_ICMP_TYPES || type == ICMP_EXT_ECHO || type == ICMP_EXT_ECHOREPLY;
> +}

This doesn't look related to uAPI, include/linux/icmp.h or
include/net/icmp.h seems like a better place for it.