netdev - Re: [PATCH net-next 1/3] ipv4: icmp: Add RFC 5837 support

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: <aPno7Zt2Fu1DapvM@shredder>
Date: Thu, 23 Oct 2025 11:35:57 +0300
From: Ido Schimmel <idosch@...dia.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>, rbonica@...iper.net
Cc: netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
	pabeni@...hat.com, edumazet@...gle.com, horms@...nel.org,
	dsahern@...nel.org, petrm@...dia.com, willemb@...gle.com,
	daniel@...earbox.net, fw@...len.de, ishaangandhi@...il.com,
	tom@...bertland.com
Subject: Re: [PATCH net-next 1/3] ipv4: icmp: Add RFC 5837 support

On Wed, Oct 22, 2025 at 06:00:28PM -0400, Willem de Bruijn wrote:
> Ido Schimmel wrote:
> > +static __be32 icmp_ext_iio_addr4_find(const struct net_device *dev)
> > +{
> > +	struct in_device *in_dev;
> > +	struct in_ifaddr *ifa;
> > +
> > +	in_dev = __in_dev_get_rcu(dev);
> > +	if (!in_dev)
> > +		return 0;
> > +
> > +	/* It is unclear from RFC 5837 which IP address should be chosen, but
> > +	 * it makes sense to choose a global unicast address.
> 
> Is it possible for no such address to exist, and in that case should
> one of the backup options be considered?

It is possible for no such address to exist, but I believe this
extension is mainly going to be used in unnumbered networks where router
interfaces do not have IPv4 addresses assigned to them. At least that is
the use case I'm interested in supporting. So, while we can try to fall
back to backup options, I think that in the common case they are not
going to exist.

Ron, do you have any opinion on this? I read RFC 5837 again and could
not find any hints about which IP address to choose. I suspect because
the IP address is the least interesting sub-object.

> 
> > +	 */
> > +	in_dev_for_each_ifa_rcu(ifa, in_dev) {
> > +		if (READ_ONCE(ifa->ifa_flags) & IFA_F_SECONDARY)
> > +			continue;
> > +		if (ifa->ifa_scope != RT_SCOPE_UNIVERSE ||
> > +		    ipv4_is_multicast(ifa->ifa_address))
> > +			continue;
> > +		return ifa->ifa_address;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void icmp_ext_iio_iif_append(struct net *net, struct sk_buff *skb,
> > +				    int iif)
> > +{
> > +	struct icmp_ext_iio_name_subobj *name_subobj;
> > +	struct icmp_extobj_hdr *objh;
> > +	struct net_device *dev;
> > +	__be32 data;
> > +
> > +	if (!iif)
> > +		return;
> > +
> 
> Might be good to add a comment that field order is prescribed by the RFC.

Good idea. Will add. Looking at the traceroute code, I expect the
selftest to fail if someone changes this order, but it's good to be
explicit about it.