netdev - Re: [PATCH wpan-next 4/5] ieee802154: Avoid confusing changes after associating

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 for Android: free password hash cracker in your pocket

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

Date: Thu, 7 Dec 2023 21:47:05 +0100
From: Stefan Schmidt <stefan@...enfreihafen.org>
To: Miquel Raynal <miquel.raynal@...tlin.com>,
 Alexander Aring <alex.aring@...il.com>, linux-wpan@...r.kernel.org
Cc: David Girault <david.girault@...vo.com>,
 Romuald Despres <romuald.despres@...vo.com>,
 Frederic Blain <frederic.blain@...vo.com>,
 Nicolas Schodet <nico@...fr.eu.org>,
 Guilhem Imberton <guilhem.imberton@...vo.com>,
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
 "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
 netdev@...r.kernel.org
Subject: Re: [PATCH wpan-next 4/5] ieee802154: Avoid confusing changes after
 associating

Hello.

On 28.11.23 12:16, Miquel Raynal wrote:
> Once associated with any device, we are part of a PAN (with a specific
> PAN ID), and we are expected to be present on a particular
> channel. Let's avoid confusing other devices by preventing any PAN
> ID/channel change once associated.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@...tlin.com>
> ---
>   net/ieee802154/nl802154.c | 30 ++++++++++++++++++------------
>   1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
> index e4d290d0e0a0..5c73b5fcadc0 100644
> --- a/net/ieee802154/nl802154.c
> +++ b/net/ieee802154/nl802154.c
> @@ -1087,6 +1087,15 @@ static int nl802154_set_pan_id(struct sk_buff *skb, struct genl_info *info)
>   
>   	pan_id = nla_get_le16(info->attrs[NL802154_ATTR_PAN_ID]);
>   
> +	/* Only allow changing the PAN ID when the device has no more
> +	 * associations ongoing to avoid confusing peers.
> +	 */
> +	if (cfg802154_device_is_associated(wpan_dev)) {
> +		NL_SET_ERR_MSG(info->extack,
> +			       "Existing associations, changing PAN ID forbidden");
> +		return -EINVAL;
> +	}
> +
>   	return rdev_set_pan_id(rdev, wpan_dev, pan_id);
>   }
>   
> @@ -1113,20 +1122,17 @@ static int nl802154_set_short_addr(struct sk_buff *skb, struct genl_info *info)
>   
>   	short_addr = nla_get_le16(info->attrs[NL802154_ATTR_SHORT_ADDR]);
>   
> -	/* TODO
> -	 * I am not sure about to check here on broadcast short_addr.
> -	 * Broadcast is a valid setting, comment from 802.15.4:
> -	 * A value of 0xfffe indicates that the device has
> -	 * associated but has not been allocated an address. A
> -	 * value of 0xffff indicates that the device does not
> -	 * have a short address.
> -	 *
> -	 * I think we should allow to set these settings but
> -	 * don't allow to allow socket communication with it.
> +	/* The short address only has a meaning when part of a PAN, after a
> +	 * proper association procedure. However, we want to still offer the
> +	 * possibility to create static networks so changing the short address
> +	 * is only allowed when not already associated to other devices with
> +	 * the official handshake.
>   	 */
> -	if (short_addr == cpu_to_le16(IEEE802154_ADDR_SHORT_UNSPEC) ||
> -	    short_addr == cpu_to_le16(IEEE802154_ADDR_SHORT_BROADCAST))
> +	if (cfg802154_device_is_associated(wpan_dev)) {
> +		NL_SET_ERR_MSG(info->extack,
> +			       "Existing associations, changing short address forbidden");
>   		return -EINVAL;
> +	}
>   
>   	return rdev_set_short_addr(rdev, wpan_dev, short_addr);
>   }

Acked-by: Stefan Schmidt <stefan@...enfreihafen.org>

regards
Stefan Schmidt