Re: [patch net-next v4 05/13] net: devlink: track netdev with devlink_port assigned

[Ido Schimmel <idosch@idosch.org>]

On Wed, Nov 02, 2022 at 05:02:03PM +0100, Jiri Pirko wrote:
> @@ -9645,10 +9649,13 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
>  
>  	ret = xa_alloc_cyclic(&devlinks, &devlink->index, devlink, xa_limit_31b,
>  			      &last_id, GFP_KERNEL);
> -	if (ret < 0) {
> -		kfree(devlink);
> -		return NULL;
> -	}
> +	if (ret < 0)
> +		goto err_xa_alloc;
> +
> +	devlink->netdevice_nb.notifier_call = devlink_netdevice_event;
> +	ret = register_netdevice_notifier_net(net, &devlink->netdevice_nb);
> +	if (ret)
> +		goto err_register_netdevice_notifier;
>  
>  	devlink->dev = dev;
>  	devlink->ops = ops;
> @@ -9675,6 +9682,12 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
>  	init_completion(&devlink->comp);
>  
>  	return devlink;
> +
> +err_register_netdevice_notifier:
> +	xa_erase(&devlinks, devlink->index);
> +err_xa_alloc:
> +	kfree(devlink);
> +	return NULL;
>  }
>  EXPORT_SYMBOL_GPL(devlink_alloc_ns);
>  
> @@ -9828,6 +9841,10 @@ void devlink_free(struct devlink *devlink)
>  	WARN_ON(!list_empty(&devlink->port_list));
>  
>  	xa_destroy(&devlink->snapshot_ids);
> +
> +	unregister_netdevice_notifier_net(devlink_net(devlink),
> +					  &devlink->netdevice_nb);
> +
>  	xa_erase(&devlinks, devlink->index);
>  
>  	kfree(devlink);

The network namespace of the devlink instance can change throughout the
lifetime of the devlink instance, but the notifier block is always
registered in the initial namespace. This leads to
unregister_netdevice_notifier_net() failing to unregister the notifier
block, which leads to use-after-free. Reproduce (with KASAN enabled):

# echo "10 0" > /sys/bus/netdevsim/new_device
# ip netns add bla
# devlink dev reload netdevsim/netdevsim10 netns bla
# echo 10 > /sys/bus/netdevsim/del_device
# ip link add dummy10 up type dummy

I see two possible solutions:

1. Use register_netdevice_notifier() instead of
register_netdevice_notifier_net().

2. Move the notifier block to the correct namespace in devlink_reload().