Re: [PATCH v4] net: hsr: Provide RedBox support (HSR-SAN)

[Paolo Abeni]

On Tue, 2024-04-02 at 10:58 +0200, Lukasz Majewski wrote:
> Introduce RedBox support (HSR-SAN to be more precise) for HSR networks.
> Following traffic reduction optimizations have been implemented:
> - Do not send HSR supervisory frames to Port C (interlink)
> - Do not forward to HSR ring frames addressed to Port C
> - Do not forward to Port C frames from HSR ring
> - Do not send duplicate HSR frame to HSR ring when destination is Port C
> 
> The corresponding patch to modify iptable2 sources has already been sent:
> https://lore.kernel.org/netdev/20240308145729.490863-1-lukma@xxxxxxx/T/
> 
> Testing procedure:
> ------------------
> The EVB-KSZ9477 has been used for testing on net-next branch
> (SHA1: 5fc68320c1fb3c7d456ddcae0b4757326a043e6f).
> 
> Ports 4/5 were used for SW managed HSR (hsr1) as first hsr0 for ports 1/2
> (with HW offloading for ksz9477) was created. Port 3 has been used as
> interlink port (single USB-ETH dongle).
> 
> Configuration - RedBox (EVB-KSZ9477):
> if link set lan1 down;ip link set lan2 down
> ip link add name hsr0 type hsr slave1 lan1 slave2 lan2 supervision 45 version 1
> ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 interlink lan3 supervision 45 version 1
> ip link set lan4 up;ip link set lan5 up
> ip link set lan3 up
> ip addr add 192.168.0.11/24 dev hsr1
> ip link set hsr1 up
> 
> Configuration - DAN-H (EVB-KSZ9477):
> 
> ip link set lan1 down;ip link set lan2 down
> ip link add name hsr0 type hsr slave1 lan1 slave2 lan2 supervision 45 version 1
> ip link add name hsr1 type hsr slave1 lan4 slave2 lan5 supervision 45 version 1
> ip link set lan4 up;ip link set lan5 up
> ip addr add 192.168.0.12/24 dev hsr1
> ip link set hsr1 up
> 
> This approach uses only SW based HSR devices (hsr1).
> 
> --------------          -----------------       ------------
> DAN-H  Port5 | <------> | Port5         |       |
>        Port4 | <------> | Port4   Port3 | <---> | PC
>              |          | (RedBox)      |       | (USB-ETH)
> EVB-KSZ9477  |          | EVB-KSZ9477   |       |
> --------------          -----------------       ------------
> 
> Signed-off-by: Lukasz Majewski <lukma@xxxxxxx>

This is 'net-next' patch, you must insert the target tree in the subj
prefix.

Does not apply cleanly to 'net-next', please rebase.

Introducing a new functionality, this deserve some paired self-tests. 
Does this have specific H/W requirement or can it run e.g. on top of
veths? If the latter applies, please bundle some basic test with the
next revision (separate patch, same series).

> @@ -561,6 +582,37 @@ void hsr_prune_nodes(struct timer_list *t)
>  		  jiffies + msecs_to_jiffies(PRUNE_PERIOD));
>  }
>  
> +void hsr_prune_proxy_nodes(struct timer_list *t)
> +{
> +	struct hsr_priv *hsr = from_timer(hsr, t, prune_proxy_timer);
> +	unsigned long timestamp;
> +	struct hsr_node *node;
> +	struct hsr_node *tmp;
> +
> +	spin_lock_bh(&hsr->list_lock);
> +	list_for_each_entry_safe(node, tmp, &hsr->proxy_node_db, mac_list) {
> +		timestamp = node->time_in[HSR_PT_INTERLINK];
> +
> +		/* Prune old entries */
> +		if (time_is_before_jiffies(timestamp +
> +				msecs_to_jiffies(HSR_PROXY_NODE_FORGET_TIME))) {
> +			hsr_nl_nodedown(hsr, node->macaddress_A);
> +			if (!node->removed) {
> +				list_del_rcu(&node->mac_list);
> +				node->removed = true;
> +				/* Note that we need to free this entry later: */
> +				kfree_rcu(node, rcu_head);
> +			}
> +		}
> +	}
> +
> +	spin_unlock_bh(&hsr->list_lock);
> +
> +	/* Restart timer */
> +	mod_timer(&hsr->prune_proxy_timer,
> +		  jiffies + msecs_to_jiffies(PRUNE_PROXY_PERIOD));

AFAICS this timer not explicitly cancelled at hsr port tear-down time.

What prevent it from expiring after a port has been deleted and causing
UaF?

Cheers,

Paolo