[PATCH net-next 0/2] netconsole: add locking to configfs _show callbacks

[PATCH net-next 0/2] netconsole: add locking to configfs _show callbacks

[Gustavo Luiz Duarte]

Patch 1 fixes a couple of reports from sashiko by adding proper locking
to configs read callbacks to avoid printing partially updated data.
Patch 2 is mostly cosmetic but worth fixing.

Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
Gustavo Luiz Duarte (2):
      netconsole: avoid printing partially updated target attributes
      netconsole: remove unnecessary target refcounting from the netdev notifier

 drivers/net/netconsole.c | 93 ++++++++++++++++++++++++++----------------------
 1 file changed, 51 insertions(+), 42 deletions(-)
---
base-commit: 8b4e7209c842d8cb9516f1f5ef0a88aa2d8831a6
change-id: 20261005-netcons-fixes-next-af0db028f570

Best regards,
--  
Gustavo Luiz Duarte <gustavold@gmail.com>

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

[Gustavo Luiz Duarte]

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;
 }
 
 static ssize_t local_port_show(struct config_item *item, char *buf)
@@ -875,25 +887,39 @@ static ssize_t remote_port_show(struct config_item *item, char *buf)
 static ssize_t local_ip_show(struct config_item *item, char *buf)
 {
 	struct netconsole_target *nt = to_target(item);
+	int ret;
+
+	dynamic_netconsole_mutex_lock();
 
 	if (nt->local_ip.family == AF_UNSPEC)
-		return sysfs_emit(buf, "\n");
-	if (nt->local_ip.family == AF_INET6)
-		return sysfs_emit(buf, "%pI6c\n", &nt->local_ip.in6);
+		ret = sysfs_emit(buf, "\n");
+	else if (nt->local_ip.family == AF_INET6)
+		ret = sysfs_emit(buf, "%pI6c\n", &nt->local_ip.in6);
 	else
-		return sysfs_emit(buf, "%pI4\n", &nt->local_ip.ip);
+		ret = sysfs_emit(buf, "%pI4\n", &nt->local_ip.ip);
+
+	dynamic_netconsole_mutex_unlock();
+
+	return ret;
 }
 
 static ssize_t remote_ip_show(struct config_item *item, char *buf)
 {
 	struct netconsole_target *nt = to_target(item);
+	int ret;
+
+	dynamic_netconsole_mutex_lock();
 
 	if (nt->remote_ip.family == AF_UNSPEC)
-		return sysfs_emit(buf, "\n");
-	if (nt->remote_ip.family == AF_INET6)
-		return sysfs_emit(buf, "%pI6c\n", &nt->remote_ip.in6);
+		ret = sysfs_emit(buf, "\n");
+	else if (nt->remote_ip.family == AF_INET6)
+		ret = sysfs_emit(buf, "%pI6c\n", &nt->remote_ip.in6);
 	else
-		return sysfs_emit(buf, "%pI4\n", &nt->remote_ip.ip);
+		ret = sysfs_emit(buf, "%pI4\n", &nt->remote_ip.ip);
+
+	dynamic_netconsole_mutex_unlock();
+
+	return ret;
 }
 
 static ssize_t local_mac_show(struct config_item *item, char *buf)
@@ -906,7 +932,14 @@ static ssize_t local_mac_show(struct config_item *item, char *buf)
 
 static ssize_t remote_mac_show(struct config_item *item, char *buf)
 {
-	return sysfs_emit(buf, "%pM\n", to_target(item)->remote_mac);
+	struct netconsole_target *nt = to_target(item);
+	int ret;
+
+	dynamic_netconsole_mutex_lock();
+	ret = sysfs_emit(buf, "%pM\n", nt->remote_mac);
+	dynamic_netconsole_mutex_unlock();
+
+	return ret;
 }
 
 static ssize_t transmit_errors_show(struct config_item *item, char *buf)
@@ -1342,7 +1375,14 @@ static struct netconsole_target *userdata_to_target(struct userdata *ud)
 
 static ssize_t userdatum_value_show(struct config_item *item, char *buf)
 {
-	return sysfs_emit(buf, "%s\n", &(to_userdatum(item)->value[0]));
+	struct userdatum *udm = to_userdatum(item);
+	int ret;
+
+	dynamic_netconsole_mutex_lock();
+	ret = sysfs_emit(buf, "%s\n", udm->value);
+	dynamic_netconsole_mutex_unlock();
+
+	return ret;
 }
 
 /* Navigate configfs and calculate the lentgh of the formatted string

-- 
2.55.0

[PATCH net-next 2/2] netconsole: remove unnecessary target refcounting from the netdev notifier

[Gustavo Luiz Duarte]

Since netconsole_netdev_event() holds target_list_lock, there is no need
to also protect each target with netconsole_target_get/put()
refcounting. There is no way a target reachable from the target_list
would go away while we hold target_list_lock.

This is similar to commit a6d403ac9689 ("netconsole: remove unnecessary
netconsole_target_get/out() from write_msg()").

Since this is the only remaining user of netconsole_target_get/put(),
remove those helpers as well.

Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
 drivers/net/netconsole.c | 31 -------------------------------
 1 file changed, 31 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 188beacb308d..ddc5fcf19fe6 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -262,23 +262,6 @@ static void __exit dynamic_netconsole_exit(void)
 	configfs_unregister_subsystem(&netconsole_subsys);
 }
 
-/*
- * Targets that were created by parsing the boot/module option string
- * do not exist in the configfs hierarchy (and have NULL names) and will
- * never go away, so make these a no-op for them.
- */
-static void netconsole_target_get(struct netconsole_target *nt)
-{
-	if (config_item_name(&nt->group.cg_item))
-		config_group_get(&nt->group);
-}
-
-static void netconsole_target_put(struct netconsole_target *nt)
-{
-	if (config_item_name(&nt->group.cg_item))
-		config_group_put(&nt->group);
-}
-
 static void dynamic_netconsole_mutex_lock(void)
 {
 	mutex_lock(&dynamic_netconsole_mutex);
@@ -300,18 +283,6 @@ static void __exit dynamic_netconsole_exit(void)
 {
 }
 
-/*
- * No danger of targets going away from under us when dynamic
- * reconfigurability is off.
- */
-static void netconsole_target_get(struct netconsole_target *nt)
-{
-}
-
-static void netconsole_target_put(struct netconsole_target *nt)
-{
-}
-
 static void populate_configfs_item(struct netconsole_target *nt,
 				   int cmdline_count)
 {
@@ -1979,7 +1950,6 @@ static int netconsole_netdev_event(struct notifier_block *this,
 	mutex_lock(&target_cleanup_list_lock);
 	spin_lock_irqsave(&target_list_lock, flags);
 	list_for_each_entry_safe(nt, tmp, &target_list, list) {
-		netconsole_target_get(nt);
 		if (nt->np.dev == dev) {
 			switch (event) {
 			case NETDEV_CHANGENAME:
@@ -2009,7 +1979,6 @@ static int netconsole_netdev_event(struct notifier_block *this,
 			 * notifier.
 			 */
 			queue_work(netconsole_wq, &nt->resume_wq);
-		netconsole_target_put(nt);
 	}
 	spin_unlock_irqrestore(&target_list_lock, flags);
 	mutex_unlock(&target_cleanup_list_lock);

-- 
2.55.0

Re: [PATCH net-next 0/2] netconsole: add locking to configfs _show callbacks

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

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

[Eric Dumazet]

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();

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

[Gustavo Luiz Duarte]

Hi Eric, thanks for the review!

On Tue, Oct 6, 2026 at 9:35 PM Eric Dumazet <edumazet@kernel.org> wrote:
>
>
>
> 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);

This could lead to a use-after-free if we race with NETDEV_UNREGISTER
and 'dev' gets freed. It is the same issue I'm trying to fix in [1].
Claude suggested adding an rcu_read_lock(). Since
unregister_netdevice_many_notify() calls synchronize_rcu(), it is
guaranteed that the net_device won't be freed until the grace period
finishes.
Something like:

  dynamic_netconsole_mutex_lock();
  strscpy(name, nt->np.dev_name, sizeof(name));
  rcu_read_lock();
  dev = READ_ONCE(nt->np.dev);
  if (nt->state == STATE_ENABLED && dev)
        netdev_copy_name(dev, name);
  rcu_read_unlock();
  dynamic_netconsole_mutex_unlock();
  return sysfs_emit(buf, "%s\n", name);

Also, netdev_copy_name is not an exported symbol, I would have to
EXPORT_SYMBOL(netdev_copy_name).

Would something like this be acceptable?

I can try and apply the same RCU approach to [1]. We don't have a
similar seqlock for dev_addr though, does it make sense to create one?

[1] https://lore.kernel.org/all/20261006-netcons-fixes-v2-2-ca652d55fd4a@gmail.com

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The patch is meant to stop *_show() callbacks from racing with
  writers, but local_mac_show() in drivers/net/netconsole.c is still…
- [Low] The commit fixes a user-visible race: reading local_ip,
  remote_ip, dev_name, remote_mac or userdatum value while it is being…

--

netconsole: avoid printing partially updated target attributes

This commit takes dynamic_netconsole_mutex in several configfs *_show()
callbacks so that reads are serialized against the store callbacks. It
also takes RTNL in dev_name_show() so the read cannot race with
netconsole_netdev_event() updating np.dev_name on NETDEV_CHANGENAME.

> 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>

[Severity: Low]
This fixes a race that users can see, but the trailers have no Fixes: tag.
Should one be added so stable and backport tooling can tell which trees
need the fix?

The family-dependent formatting in local_ip_show() and remote_ip_show()
(the AF_UNSPEC and AF_INET6 branches) came from these commits:

46b3f9e7e761 ("netconsole: use the address family instead of the ipv6 flag")
56ddc8d48e75 ("netconsole: show empty string for an unset IP address")

The first Closes: link points at that netcons_ipv6 series.

The lockless reads in dev_name_show() and remote_mac_show() go back to:

0bcc1816188e ("[NET] netconsole: Support dynamic reconfiguration using configfs")

> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 267254f046de9..188beacb308d7 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;
>  }

[ ... ]

> @@ -906,7 +932,14 @@ static ssize_t local_mac_show(struct config_item *item, char *buf)
>  
>  static ssize_t remote_mac_show(struct config_item *item, char *buf)
>  {
> -	return sysfs_emit(buf, "%pM\n", to_target(item)->remote_mac);
> +	struct netconsole_target *nt = to_target(item);
> +	int ret;
> +
> +	dynamic_netconsole_mutex_lock();
> +	ret = sysfs_emit(buf, "%pM\n", nt->remote_mac);
> +	dynamic_netconsole_mutex_unlock();
> +
> +	return ret;
>  }

[Severity: High]
local_mac_show(), just above this hunk, still has no locking. Is it still
open to the same race the commit message describes?

drivers/net/netconsole.c:local_mac_show() {
	struct net_device *dev = to_target(item)->np.dev;
	static const u8 bcast[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };

	return sysfs_emit(buf, "%pM\n", dev ? dev->dev_addr : bcast);
}

It loads np.dev and dereferences dev->dev_addr without holding
dynamic_netconsole_mutex, without RTNL, and without a netdev reference.

The netdev notifier can clear np.dev and drop its reference while holding
only RTNL:

netconsole_netdev_event(NETDEV_UNREGISTER)
  netconsole_process_cleanups_core()   /* ASSERT_RTNL() */
    netcons_release_dev()
      do_netpoll_cleanup()
        netdev_put(np->dev, &np->dev_tracker);
        np->dev = NULL;

enabled_store() also reaches this cleanup, under the mutex.

Suppose a reader of local_mac (world-readable through CONFIGFS_ATTR_RO)
loads a non-NULL dev and is then preempted while the device is
unregistered and freed. Could sysfs_emit() then read dev->dev_addr from a
freed net_device?

Even without unregistration, dev->dev_addr is rewritten under RTNL when
the MAC address changes. A lockless read could then print a mix of old
and new bytes.

dev_name_show() already takes RTNL because of the notifier-side writer.
Would taking dynamic_netconsole_mutex and then rtnl_lock() around both the
np.dev load and the sysfs_emit() call in local_mac_show() cover both
cases?

The later commit in this series, "netconsole: remove unnecessary target
refcounting from the netdev notifier", does not touch local_mac_show(). So
the problem still seems to be there at the end of the series.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006-netcons-fixes-next-v1-0-231cd26f8c51%40gmail.com

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

[Breno Leitao]

On Thu, Oct 08, 2026 at 08:55:40PM +0100, Gustavo Luiz Duarte wrote:
> Hi Eric, thanks for the review!
> 
> On Tue, Oct 6, 2026 at 9:35 PM Eric Dumazet <edumazet@kernel.org> wrote:
> >
> >
> >
> > 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);
> 
> This could lead to a use-after-free if we race with NETDEV_UNREGISTER
> and 'dev' gets freed.

Any chance you can get the lock (either dynamic_netconsole_mutex or
target_list_lock) in netdev notifiers, so, it doens't conflict with this
one?

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

[Gustavo Luiz Duarte]

On Fri, Oct 9, 2026 at 1:35 PM Breno Leitao <leitao@debian.org> wrote:
>
> On Thu, Oct 08, 2026 at 08:55:40PM +0100, Gustavo Luiz Duarte wrote:
> > Hi Eric, thanks for the review!
> >
> > On Tue, Oct 6, 2026 at 9:35 PM Eric Dumazet <edumazet@kernel.org> wrote:
> > >
> > >
> > >
> > > 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);
> >
> > This could lead to a use-after-free if we race with NETDEV_UNREGISTER
> > and 'dev' gets freed.
>
> Any chance you can get the lock (either dynamic_netconsole_mutex or
> target_list_lock) in netdev notifiers, so, it doens't conflict with this
> one?

We can sidestep the device teardown entirely if we just stick to
np->dev_name and protect it with target_list_lock which is held in
netconsole_netdev_event() while handling NETDEV_CHANGENAME:

  dynamic_netconsole_mutex_lock();
  spin_lock_irqsave(&target_list_lock, flags);
  ret = sysfs_emit(buf, "%s\n", nt->np.dev_name);
  spin_unlock_irqrestore(&target_list_lock, flags);
  dynamic_netconsole_mutex_unlock();

The local_mac_show() case is not as simple because we don't keep a
copy of dev_addr, but we can couple target_list_lock with nt->state ==
STATE_ENABLED, which ensures the target is not queued for cleanup so
the device won't be freed from under us.

I will send a new revision for this and for local_mac_show() using
target_list_lock.