Re: [PATCH net-next 1/2] netconsole: avoid printing partially updated target attributes

[Eric Dumazet <edumazet@kernel.org>]

On 10/6/26 20:58, Gustavo Luiz Duarte wrote:
> The configfs store callbacks all serialize on dynamic_netconsole_mutex
> but not on the read side, so reading an attribute while it is being
> written returns a partially updated value.
> 
> Hold dynamic_netconsole_mutex on *_show() callbacks to avoid racing with
> writers.
> 
> The dev_name_show() callback can also race with
> netconsole_netdev_event() writing to np.dev_name due to
> NETDEV_CHANGENAME. So it needs to hold RTNL in addition to
> dynamic_netconsole_mutex.
> 
> Reported-by: Sashiko <netdev-bot+sashiko@kernel.org>
> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260814-netcons_ipv6-v3-7-bc0915e8c75f@gmail.com
> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928-netcons-fixes-v1-0-bb5ffe5e698a%40gmail.com
> Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
> ---
>   drivers/net/netconsole.c | 62 +++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 51 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 267254f046de..188beacb308d 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -859,7 +859,19 @@ static ssize_t release_show(struct config_item *item, char *buf)
>   
>   static ssize_t dev_name_show(struct config_item *item, char *buf)
>   {
> -	return sysfs_emit(buf, "%s\n", to_target(item)->np.dev_name);
> +	struct netconsole_target *nt = to_target(item);
> +	int ret;
> +
> +	dynamic_netconsole_mutex_lock();
> +	/* Hold RTNL to prevent racing against netconsole_netdev_event()
> +	 * changing np.dev_name.
> +	 */
> +	rtnl_lock();
> +	ret = sysfs_emit(buf, "%s\n", nt->np.dev_name);
> +	rtnl_unlock();
> +	dynamic_netconsole_mutex_unlock();
> +
> +	return ret;
>   }

Please do not add rtnl_lock() in a _show() sysfs handler unless there is 
no other way?

Something like:

     dynamic_netconsole_mutex_lock();
     strscpy(name, nt->np.dev_name, sizeof(name));
     if (nt->state == STATE_ENABLED) {
         struct net_device *dev = nt->np.dev;

         if (dev)
             netdev_copy_name(dev, name);
     }
     dynamic_netconsole_mutex_unlock();