netdev - Re: [PATCH net-next] improve handling of ICMP_EXT

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220714215132.058a1d4e@kernel.org>
Date:   Thu, 14 Jul 2022 21:51:32 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Mathias Lark <mathiaslark@...il.com>
Cc:     davem@...emloft.net, yoshfuji@...ux-ipv6.org, dsahern@...nel.org,
        pabeni@...hat.com, pablo@...filter.org, kadlec@...filter.org,
        fw@...len.de, netdev@...r.kernel.org,
        netfilter-devel@...r.kernel.org, coreteam@...filter.org
Subject: Re: [PATCH net-next] improve handling of ICMP_EXT_ECHO icmp type

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.