unregister_netdevice: waiting for batadv_slave_0 to become free. Usage count = 2

unregister_netdevice: waiting for batadv_slave_0 to become free. Usage count = 2

[Tetsuo Handa]

Hello.

I made a minimized C reproducer (shown bottom) for

  unregister_netdevice: waiting for batadv_slave_0 to become free. Usage count = 2
  ref_tracker: netdev@ffff88807bb4c620 has 1/1 users at
       __netdev_tracker_alloc include/linux/netdevice.h:4390 [inline]
       netdev_hold include/linux/netdevice.h:4419 [inline]
       batadv_hardif_add_interface net/batman-adv/hard-interface.c:878 [inline]
       batadv_hard_if_event+0xbd1/0x1280 net/batman-adv/hard-interface.c:958
       notifier_call_chain+0x1b6/0x3e0 kernel/notifier.c:85
       call_netdevice_notifiers_extack net/core/dev.c:2267 [inline]
       call_netdevice_notifiers net/core/dev.c:2281 [inline]
       register_netdevice+0x1608/0x1ae0 net/core/dev.c:11325
       veth_newlink+0x5cc/0xa50 drivers/net/veth.c:1884
       rtnl_newlink_create+0x310/0xb00 net/core/rtnetlink.c:3825
       __rtnl_newlink net/core/rtnetlink.c:3942 [inline]
       rtnl_newlink+0x16d6/0x1c70 net/core/rtnetlink.c:4057
       rtnetlink_rcv_msg+0x7cf/0xb70 net/core/rtnetlink.c:6946
       netlink_rcv_skb+0x208/0x470 net/netlink/af_netlink.c:2552
       netlink_unicast_kernel net/netlink/af_netlink.c:1320 [inline]
       netlink_unicast+0x82f/0x9e0 net/netlink/af_netlink.c:1346
       netlink_sendmsg+0x805/0xb30 net/netlink/af_netlink.c:1896
       sock_sendmsg_nosec net/socket.c:714 [inline]
       __sock_sendmsg+0x21c/0x270 net/socket.c:729
       __sys_sendto+0x3bd/0x520 net/socket.c:2231
       __do_sys_sendto net/socket.c:2238 [inline]
       __se_sys_sendto net/socket.c:2234 [inline]
       __x64_sys_sendto+0xde/0x100 net/socket.c:2234
       do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
       do_syscall_64+0xfa/0xfa0 arch/x86/entry/syscall_64.c:94
       entry_SYSCALL_64_after_hwframe+0x77/0x7f

problem at https://syzkaller.appspot.com/bug?extid=881d65229ca4f9ae8c84 .

When this problem happens, the

  batman_adv: batadv0: Adding interface: batadv_slave_0
  batman_adv: batadv0: The MTU of interface batadv_slave_0 is too small (1500) to handle the transport of batman-adv packets. Packets going over this interface will be fragmented on layer2 which could impact the performance. Setting the MTU to 1560 would solve the problem.
  batman_adv: batadv0: Not using interface batadv_slave_0 (retrying later): interface not active
  batman_adv: batadv0: Interface activated: batadv_slave_0
  batman_adv: batadv0: Interface deactivated: batadv_slave_0
  batman_adv: batadv0: Removing interface: batadv_slave_0
  batman_adv: batadv0: adding TT local entry 33:33:00:00:00:01 to non-existent VLAN -1

messages are printed but the

  batadv_hardif_release+0x44/0xb0
  batadv_hard_if_event+0x349/0x410
  notifier_call_chain+0x41/0x100
  unregister_netdevice_many_notify+0x43a/0xac0
  default_device_exit_batch+0xed/0x120
  ops_undo_list+0x10d/0x3b0
  cleanup_net+0x1f8/0x370
  process_one_work+0x223/0x590
  worker_thread+0x1cb/0x3a0
  kthread+0xff/0x240
  ret_from_fork+0x17f/0x1e0
  ret_from_fork_asm+0x1a/0x30

trace is not called (compared to when this problem does not happen).

I suspect that batadv_hard_if_event_meshif() has something to do upon
NETDEV_UNREGISTER event because batadv_hard_if_event_meshif() receives
NETDEV_POST_INIT / NETDEV_REGISTER / NETDEV_UNREGISTER / NETDEV_PRE_UNINIT
events when this reproducer is executed, but I don't know what to do...

---------- minimized C reproducer start ----------
#define _GNU_SOURCE
#include <arpa/inet.h>
#include <errno.h>
#include <net/if.h>
#include <sched.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/socket.h>
#include <unistd.h>
#include <linux/genetlink.h>
#include <linux/if_ether.h>
#include <linux/ip.h>
#include <linux/netlink.h>
#include <linux/rtnetlink.h>
#include <linux/veth.h>
#include <linux/batman_adv.h>

struct nlmsg {
	char* pos;
	int nesting;
	struct nlattr* nested[8];
	char buf[4096];
};

static void netlink_init(struct nlmsg* nlmsg, int typ, int flags,
                         const void* data, int size)
{
	memset(nlmsg, 0, sizeof(*nlmsg));
	struct nlmsghdr* hdr = (struct nlmsghdr*)nlmsg->buf;
	hdr->nlmsg_type = typ;
	hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | flags;
	memcpy(hdr + 1, data, size);
	nlmsg->pos = (char*)(hdr + 1) + NLMSG_ALIGN(size);
}

static void netlink_attr(struct nlmsg* nlmsg, int typ, const void* data,
                         int size)
{
	struct nlattr* attr = (struct nlattr*)nlmsg->pos;
	attr->nla_len = sizeof(*attr) + size;
	attr->nla_type = typ;
	if (size > 0)
		memcpy(attr + 1, data, size);
	nlmsg->pos += NLMSG_ALIGN(attr->nla_len);
}

static void netlink_nest(struct nlmsg* nlmsg, int typ)
{
	struct nlattr* attr = (struct nlattr*)nlmsg->pos;
	attr->nla_type = typ;
	nlmsg->pos += sizeof(*attr);
	nlmsg->nested[nlmsg->nesting++] = attr;
}

static void netlink_done(struct nlmsg* nlmsg)
{
	struct nlattr* attr = nlmsg->nested[--nlmsg->nesting];
	attr->nla_len = nlmsg->pos - (char*)attr;
}

static int netlink_send_ext(struct nlmsg* nlmsg, int sock, uint16_t reply_type,
                            int* reply_len, bool dofail)
{
	if (nlmsg->pos > nlmsg->buf + sizeof(nlmsg->buf) || nlmsg->nesting)
		exit(1);
	struct nlmsghdr* hdr = (struct nlmsghdr*)nlmsg->buf;
	hdr->nlmsg_len = nlmsg->pos - nlmsg->buf;
	struct sockaddr_nl addr;
	memset(&addr, 0, sizeof(addr));
	addr.nl_family = AF_NETLINK;
	ssize_t n = sendto(sock, nlmsg->buf, hdr->nlmsg_len, 0,
			   (struct sockaddr*)&addr, sizeof(addr));
	if (n != (ssize_t)hdr->nlmsg_len) {
		if (dofail)
			exit(1);
		return -1;
	}
	n = recv(sock, nlmsg->buf, sizeof(nlmsg->buf), 0);
	if (reply_len)
		*reply_len = 0;
	if (n < 0) {
		if (dofail)
			exit(1);
		return -1;
	}
	if (n < (ssize_t)sizeof(struct nlmsghdr)) {
		errno = EINVAL;
		if (dofail)
			exit(1);
		return -1;
	}
	if (hdr->nlmsg_type == NLMSG_DONE)
		return 0;
	if (reply_len && hdr->nlmsg_type == reply_type) {
		*reply_len = n;
		return 0;
	}
	if (n < (ssize_t)(sizeof(struct nlmsghdr) + sizeof(struct nlmsgerr))) {
		errno = EINVAL;
		if (dofail)
			exit(1);
		return -1;
	}
	if (hdr->nlmsg_type != NLMSG_ERROR) {
		errno = EINVAL;
		if (dofail)
			exit(1);
		return -1;
	}
	errno = -((struct nlmsgerr*)(hdr + 1))->error;
	return -errno;
}

static int netlink_send(struct nlmsg* nlmsg, int sock)
{
	return netlink_send_ext(nlmsg, sock, 0, NULL, true);
}

static int netlink_query_family_id(struct nlmsg* nlmsg, int sock,
                                   const char* family_name, bool dofail)
{
	struct genlmsghdr genlhdr;
	memset(&genlhdr, 0, sizeof(genlhdr));
	genlhdr.cmd = CTRL_CMD_GETFAMILY;
	netlink_init(nlmsg, GENL_ID_CTRL, 0, &genlhdr, sizeof(genlhdr));
	netlink_attr(nlmsg, CTRL_ATTR_FAMILY_NAME, family_name,
		     strnlen(family_name, GENL_NAMSIZ - 1) + 1);
	int n = 0;
	int err = netlink_send_ext(nlmsg, sock, GENL_ID_CTRL, &n, dofail);
	if (err < 0) {
		return -1;
	}
	uint16_t id = 0;
	struct nlattr* attr = (struct nlattr*)(nlmsg->buf + NLMSG_HDRLEN +
					       NLMSG_ALIGN(sizeof(genlhdr)));
	for (; (char*)attr < nlmsg->buf + n;
	     attr = (struct nlattr*)((char*)attr + NLMSG_ALIGN(attr->nla_len))) {
		if (attr->nla_type == CTRL_ATTR_FAMILY_ID) {
			id = *(uint16_t*)(attr + 1);
			break;
		}
	}
	if (!id) {
		errno = EINVAL;
		return -1;
	}
	recv(sock, nlmsg->buf, sizeof(nlmsg->buf), 0);
	return id;
}

static void netlink_add_device_impl(struct nlmsg* nlmsg, const char* type,
                                    const char* name, bool up)
{
	struct ifinfomsg hdr;
	memset(&hdr, 0, sizeof(hdr));
	if (up)
		hdr.ifi_flags = hdr.ifi_change = IFF_UP;
	netlink_init(nlmsg, RTM_NEWLINK, NLM_F_EXCL | NLM_F_CREATE, &hdr,
		     sizeof(hdr));
	if (name)
		netlink_attr(nlmsg, IFLA_IFNAME, name, strlen(name));
	netlink_nest(nlmsg, IFLA_LINKINFO);
	netlink_attr(nlmsg, IFLA_INFO_KIND, type, strlen(type));
}

static void netlink_add_device(struct nlmsg* nlmsg, int sock, const char* type,
                               const char* name)
{
	netlink_add_device_impl(nlmsg, type, name, false);
	netlink_done(nlmsg);
	int err = netlink_send(nlmsg, sock);
	if (err < 0) {
	}
}

static void netlink_add_veth(struct nlmsg* nlmsg, int sock, const char* name,
                             const char* peer)
{
	netlink_add_device_impl(nlmsg, "veth", name, false);
	netlink_nest(nlmsg, IFLA_INFO_DATA);
	netlink_nest(nlmsg, VETH_INFO_PEER);
	nlmsg->pos += sizeof(struct ifinfomsg);
	netlink_attr(nlmsg, IFLA_IFNAME, peer, strlen(peer));
	netlink_done(nlmsg);
	netlink_done(nlmsg);
	netlink_done(nlmsg);
	int err = netlink_send(nlmsg, sock);
	if (err < 0) {
	}
}

static void netlink_device_change(struct nlmsg* nlmsg, int sock,
                                  const char* name, bool up, const char* master,
                                  const void* mac, int macsize)
{
	struct ifinfomsg hdr;
	memset(&hdr, 0, sizeof(hdr));
	if (up)
		hdr.ifi_flags = hdr.ifi_change = IFF_UP;
	hdr.ifi_index = if_nametoindex(name);
	netlink_init(nlmsg, RTM_NEWLINK, 0, &hdr, sizeof(hdr));
	if (master) {
		int ifindex = if_nametoindex(master);
		netlink_attr(nlmsg, IFLA_MASTER, &ifindex, sizeof(ifindex));
	}
	if (macsize)
		netlink_attr(nlmsg, IFLA_ADDRESS, mac, macsize);
	int err = netlink_send(nlmsg, sock);
	if (err < 0) {
	}
}

int main(void)
{
	static struct nlmsg nlmsg = { };
	const uint64_t macaddr = 0x00aaaaaaaaaa + ((10ull) << 40);
	unshare(CLONE_NEWNET);
	// initialize netdevices
	const int sock = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
	netlink_add_device(&nlmsg, sock, "batadv", "batadv0");
	netlink_add_veth(&nlmsg, sock, "batadv_slave_0", "veth0_to_batadv");
	netlink_device_change(&nlmsg, sock, "batadv_slave_0", false, "batadv0", 0, 0);
	netlink_device_change(&nlmsg, sock, "batadv_slave_0", true, 0, &macaddr, ETH_ALEN);
	close(sock);
	// execute
	int r[3];
	char buf[64] = "batadv0";
	r[0] = socket(AF_NETLINK, SOCK_RAW, NETLINK_GENERIC);
	r[1] = netlink_query_family_id(&nlmsg, r[0], "batadv", false);
	ioctl(r[0], SIOCGIFINDEX, buf);
	r[2] = *(uint32_t*) (buf + 0x10);
	*(uint32_t*)buf = 0x1c; // len
	*(uint16_t*)(buf + 4) = r[1]; // type
	*(uint16_t*)(buf + 6) = NLM_F_REQUEST|NLM_F_ROOT|NLM_F_MATCH; // flags
	*(uint32_t*)(buf + 8) = 0; // seq
	*(uint32_t*)(buf + 12) = 0; // pid
	*(uint8_t*)(buf + 16) = BATADV_CMD_GET_NEIGHBORS; // cmd
	*(uint8_t*)(buf + 17) = 0; // version
	*(uint16_t*)(buf + 18) = 0; // reserved
	*(uint16_t*)(buf + 20) = 8; // nla_len
	*(uint16_t*)(buf + 22) = BATADV_ATTR_MESH_IFINDEX; // nla_type
	*(uint32_t*)(buf + 24) = r[2]; // payload
	send(r[0], buf, 28, 0);
	return 0;
}
---------- minimized C reproducer end ----------

Re: unregister_netdevice: waiting for batadv_slave_0 to become free. Usage count = 2

[Tetsuo Handa]

On 2025/09/22 23:09, Tetsuo Handa wrote:
> I suspect that batadv_hard_if_event_meshif() has something to do upon
> NETDEV_UNREGISTER event because batadv_hard_if_event_meshif() receives
> NETDEV_POST_INIT / NETDEV_REGISTER / NETDEV_UNREGISTER / NETDEV_PRE_UNINIT
> events when this reproducer is executed, but I don't know what to do...

With a change show bottom, the reproducer no longer triggers this problem.
But is this change correct?



Commit 9e6b5648bbc4 ("batman-adv: Fix duplicated OGMs on NETDEV_UP")
replaced batadv_iv_iface_activate() (which is called via iface.activate()
 from batadv_hardif_activate_interface()) with batadv_iv_iface_enabled()
(which is called via iface.enabled() from batadv_hardif_enable_interface()).
But that commit missed that batadv_hardif_activate_interface() is called from
both batadv_hardif_enable_interface() and batadv_hard_if_event().

Since batadv_iv_ogm_schedule_buff() updates if_status to BATADV_IF_ACTIVE
only when if_status was BATADV_IF_TO_BE_ACTIVATED, we need to call
batadv_iv_ogm_schedule_buff() from batadv_iv_ogm_schedule() from
batadv_iv_iface_enabled() via iface.enabled() with
if_status == BATADV_IF_TO_BE_ACTIVATED if we want iface.enabled() from
batadv_hardif_enable_interface() to update if_status to BATADV_IF_ACTIVE.

But when IFF_UP is not set upon creation, batadv_hardif_enable_interface()
does not call batadv_hardif_activate_interface(), which means that
if_status remains BATADV_IF_INACTIVE despite
batadv_iv_ogm_schedule_buff() is called via iface.enabled().

And when IFF_UP is set after creation, batadv_hard_if_event() calls
batadv_hardif_activate_interface(). But despite "Interface activated: %s\n"
message being printed, if_status remains BATADV_IF_TO_BE_ACTIVATED because
iface.activate == NULL due to above-mentioned commit.

Since we need to call iface.enabled() instead of iface.activate() so that
batadv_iv_ogm_schedule_buff() will update if_status to BATADV_IF_ACTIVE,
move iface.enabled() from batadv_hardif_enable_interface() to
batadv_hardif_activate_interface().



diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index bace57e4f9a5..403785f649ff 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -673,6 +673,8 @@ batadv_hardif_activate_interface(struct batadv_hard_iface *hard_iface)
 
 	if (bat_priv->algo_ops->iface.activate)
 		bat_priv->algo_ops->iface.activate(hard_iface);
+	if (bat_priv->algo_ops->iface.enabled)
+		bat_priv->algo_ops->iface.enabled(hard_iface);
 
 out:
 	batadv_hardif_put(primary_if);
@@ -770,9 +772,6 @@ int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
 
 	batadv_hardif_recalc_extra_skbroom(mesh_iface);
 
-	if (bat_priv->algo_ops->iface.enabled)
-		bat_priv->algo_ops->iface.enabled(hard_iface);
-
 out:
 	return 0;
 

Re: unregister_netdevice: waiting for batadv_slave_0 to become free. Usage count = 2

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 760 bytes --]

On Tuesday, 23 September 2025 16:45:48 CEST Tetsuo Handa wrote:
> On 2025/09/22 23:09, Tetsuo Handa wrote:
> > I suspect that batadv_hard_if_event_meshif() has something to do upon
> > NETDEV_UNREGISTER event because batadv_hard_if_event_meshif() receives
> > NETDEV_POST_INIT / NETDEV_REGISTER / NETDEV_UNREGISTER / NETDEV_PRE_UNINIT
> > events when this reproducer is executed, but I don't know what to do...
> 
> With a change show bottom, the reproducer no longer triggers this problem.
> But is this change correct?

Thanks for the debugging. I have some doubts that the move is correct - but I 
need to check this in detail when I have more time. Unfortunately, this will 
not happen before the end of the week. I hope this is ok for you.

Regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: unregister_netdevice: waiting for batadv_slave_0 to become free. Usage count = 2

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 6928 bytes --]

On Tuesday, 23 September 2025 16:45:48 CEST Tetsuo Handa wrote:
> On 2025/09/22 23:09, Tetsuo Handa wrote:
> > I suspect that batadv_hard_if_event_meshif() has something to do upon
> > NETDEV_UNREGISTER event because batadv_hard_if_event_meshif() receives
> > NETDEV_POST_INIT / NETDEV_REGISTER / NETDEV_UNREGISTER / NETDEV_PRE_UNINIT
> > events when this reproducer is executed, but I don't know what to do...
> 
> With a change show bottom, the reproducer no longer triggers this problem.
> But is this change correct?

Thanks for the writeup. Unfortunately, I tried to read this multiple times but 
you don't make it easy to figure out what the problem is.

Please don't take the following paragraphs the wrong way - I was just not able 
to really figure out what you meant. I have therefore just added my thoughts 
on each paragraph.

At the end of this mail, I tried to give a shorter summary about the interface 
states and what I think is the actual problem.

> Commit 9e6b5648bbc4 ("batman-adv: Fix duplicated OGMs on NETDEV_UP")
> replaced batadv_iv_iface_activate() (which is called via iface.activate()
>  from batadv_hardif_activate_interface()) with batadv_iv_iface_enabled()
> (which is called via iface.enabled() from batadv_hardif_enable_interface()).
> But that commit missed that batadv_hardif_activate_interface() is called from
> both batadv_hardif_enable_interface() and batadv_hard_if_event().

Why did it miss it? This is the plan in the mentioned commit.

What is the relevant effect which is creating the problem here?


> Since batadv_iv_ogm_schedule_buff() updates if_status to BATADV_IF_ACTIVE
> only when if_status was BATADV_IF_TO_BE_ACTIVATED, we need to call
> batadv_iv_ogm_schedule_buff() from batadv_iv_ogm_schedule() from
> batadv_iv_iface_enabled() via iface.enabled() with
> if_status == BATADV_IF_TO_BE_ACTIVATED if we want iface.enabled() from
> batadv_hardif_enable_interface() to update if_status to BATADV_IF_ACTIVE.

This basically says: Because of A <- B <- C, we need to have C to B to get A. 
But not really what effect this would have.

> But when IFF_UP is not set upon creation, batadv_hardif_enable_interface()
> does not call batadv_hardif_activate_interface(), which means that
> if_status remains BATADV_IF_INACTIVE despite
> batadv_iv_ogm_schedule_buff() is called via iface.enabled().

It must stay inactive when it is down. But the periodic OGM scheduling is 
still needed to have the OGM queued for active interfaces.


> And when IFF_UP is set after creation, batadv_hard_if_event() calls
> batadv_hardif_activate_interface(). But despite "Interface activated: %s\n"
> message being printed, if_status remains BATADV_IF_TO_BE_ACTIVATED because
> iface.activate == NULL due to above-mentioned commit.

This is not necessarily true. It will simply be switched to BATADV_IF_ACTIVATE 
with the next OGM because batadv_iv_ogm_schedule is rescheduled all the time.

> Since we need to call iface.enabled() instead of iface.activate() so that
> batadv_iv_ogm_schedule_buff() will update if_status to BATADV_IF_ACTIVE,
> move iface.enabled() from batadv_hardif_enable_interface() to
> batadv_hardif_activate_interface().

Why do we need to switch to this state in the first place? I didn't find this 
explanation here.

If I would add your change, we would suddenly have multiple scheduled OGMs 
again, right? Because it basically an oddly written revert of 
commit 9e6b5648bbc4 ("batman-adv: Fix duplicated OGMs on NETDEV_UP").


If this would actually the problem, why would the BATADV_CMD_GET_NEIGHBORS 
request be required? I would have expected that following would show the same 
problem when the switch of the state is the problem:

    rmmod batman-adv || true
    modprobe batman-adv
 
    ip link add batadv_slave_0 type veth peer name veth0_to_batadv
    
    ip link add batadv0 type batadv
    batctl meshif batadv0 it 1000000
    ip link set down master batadv0 dev batadv_slave_0
    ip link set up address 00a:a:aa:aa:aa:aa dev batadv_slave_0
    ip link del dev batadv_slave_0


Let me rephrase what happens with the state of an interface. Just so we are on 
the same page:

* an interface detected by batman-adv is starting with the state 
  BATADV_IF_NOT_IN_USE
* an active interface has the state BATADV_IF_ACTIVE
* when an interface is added to a batman-adv meshif, it is first set to the 
  state BATADV_IF_TO_BE_ACTIVATED
  - this is done by batadv_hardif_activate_interface()
  - there are some dependencies to the UP state of the netdev - see below
* the transition from BATADV_IF_TO_BE_ACTIVATED to BATADV_IF_ACTIVE is 
  algorithm specific
  - IV: scheduling of the OGM buffer
  - V: "directly" after BATADV_IF_TO_BE_ACTIVATED is set


The transition from state BATADV_IF_NOT_IN_USE to BATADV_IF_TO_BE_ACTIVATED 
only happens when the interface is actually UP when .ndo_add_slave is called. 
Otherwise, batman-adv postpones the call to batadv_hardif_activate_interface() 
until the NETDEV_UP event is received.

As mentioned earlier the B.A.T.M.A.N. IV algorithm implementation is (in your 
reproducer) responsible for switching the interface state from 
BATADV_IF_TO_BE_ACTIVATED to BATADV_IF_ACTIVE. And it will only do 
this when the interface was actually in the state BATADV_IF_TO_BE_ACTIVATED. 
There is a comment above the statement which explains the details.

And exactly this "delay" makes it more likely that operations are started 
while an interface is in the transition state of BATADV_IF_TO_BE_ACTIVATED. 
Simply because the periodic OGM scheduling isn't triggered immediately - it 
continues with its normal periodicity.
 

The question would now be: what is the actual problem? For example, with 
following change, the problem also seems to be gone:

diff --git i/net/batman-adv/originator.c w/net/batman-adv/originator.c
index c84420cb..f82dce20 100644
--- i/net/batman-adv/originator.c
+++ w/net/batman-adv/originator.c
@@ -763,7 +763,7 @@ int batadv_hardif_neigh_dump(struct sk_buff *msg, struct netlink_callback *cb)
 	bat_priv = netdev_priv(mesh_iface);
 
 	primary_if = batadv_primary_if_get_selected(bat_priv);
-	if (!primary_if || primary_if->if_status != BATADV_IF_ACTIVE) {
+	if (!primary_if) {
 		ret = -ENOENT;
 		goto out_put_mesh_iface;
 	}


And now we are most likely on the right path... primary_if is holding a 
reference to an hardif and therefore also a reference to the netdev. And the 
error handling is only taking care of releasing the reference to the meshif 
but not the primary_if.

I will later send a fix for this with you and 
syzbot+881d65229ca4f9ae8c84@syzkaller.appspotmail.com as Reported-by. Is this 
okay for you?



As said, earlier - I am really thankful that you worked on this. So please 
don't interpret this as a harsh criticism. I just had really big problems to 
figure out what you wanted to say in these paragraphs. I usually prefer 
something which is easier to consume.

Regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: unregister_netdevice: waiting for batadv_slave_0 to become free. Usage count = 2

[Tetsuo Handa]

Thank you for responding quickly.

On 2025/09/28 2:21, Sven Eckelmann wrote:
> The question would now be: what is the actual problem? 

Sorry, my explanation was not clear enough.

What I thought as a problem is the difference between

	netlink_device_change(&nlmsg, sock, "batadv_slave_0", true, "batadv0", 0, 0);
	//netlink_device_change(&nlmsg, sock, "batadv_slave_0", true, 0, &macaddr, ETH_ALEN);

and

	netlink_device_change(&nlmsg, sock, "batadv_slave_0", false, "batadv0", 0, 0);
	netlink_device_change(&nlmsg, sock, "batadv_slave_0", true, 0, &macaddr, ETH_ALEN);

. The former makes hard_iface->if_status == BATADV_IF_ACTIVE while the latter makes
hard_iface->if_status == BATADV_IF_TO_BE_ACTIVATED (because batadv_iv_ogm_schedule_buff()
is not called).

This difference makes operations that depend on hard_iface->if_status == BATADV_IF_ACTIVE
impossible to work properly. You can confirm this difference using diff show below.

--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -761,6 +761,7 @@ int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,

        batadv_check_known_mac_addr(hard_iface);

+       pr_info("step 1: %d\n", hard_iface->if_status);
        if (batadv_hardif_is_iface_up(hard_iface))
                batadv_hardif_activate_interface(hard_iface);
        else
@@ -768,10 +769,12 @@ int batadv_hardif_enable_interface(struct batadv_hard_iface *hard_iface,
                           "Not using interface %s (retrying later): interface not active\n",
                           hard_iface->net_dev->name);

+       pr_info("step 2: %d\n", hard_iface->if_status);
        batadv_hardif_recalc_extra_skbroom(mesh_iface);

        if (bat_priv->algo_ops->iface.enabled)
                bat_priv->algo_ops->iface.enabled(hard_iface);
+       pr_info("step 3: %d\n", hard_iface->if_status);

 out:
        return 0;
@@ -961,7 +964,9 @@ static int batadv_hard_if_event(struct notifier_block *this,

        switch (event) {
        case NETDEV_UP:
+               pr_info("step 4: %d\n", hard_iface->if_status);
                batadv_hardif_activate_interface(hard_iface);
+               pr_info("step 5: %d\n", hard_iface->if_status);
                break;
        case NETDEV_GOING_DOWN:
        case NETDEV_DOWN:

The former case:

  batman_adv: batadv0: Adding interface: batadv_slave_0
  batman_adv: batadv0: The MTU of interface batadv_slave_0 is too small (1500) to handle the transport of batman-adv packets. Packets going over this interface will be fragmented on layer2 which could impact the performance. Setting the MTU to 1560 would solve the problem.
  batman_adv: step 1: 2
  batman_adv: batadv0: Interface activated: batadv_slave_0
  batman_adv: step 2: 4
  batman_adv: step 3: 3
  batman_adv: batadv0: Interface deactivated: batadv_slave_0
  batman_adv: batadv0: Removing interface: batadv_slave_0

The latter case:

  batman_adv: step 1: 2
  batman_adv: batadv0: Not using interface batadv_slave_0 (retrying later): interface not actve
  batman_adv: step 2: 2
  batman_adv: step 3: 2
  batman_adv: step 4: 2
  batman_adv: batadv0: Interface activated: batadv_slave_0
  batman_adv: step 5: 4
  batman_adv: batadv0: Interface deactivated: batadv_slave_0
  batman_adv: batadv0: Removing interface: batadv_slave_0



> --- i/net/batman-adv/originator.c
> +++ w/net/batman-adv/originator.c
> @@ -763,7 +763,7 @@ int batadv_hardif_neigh_dump(struct sk_buff *msg, struct netlink_callback *cb)
>  	bat_priv = netdev_priv(mesh_iface);
>  
>  	primary_if = batadv_primary_if_get_selected(bat_priv);
> -	if (!primary_if || primary_if->if_status != BATADV_IF_ACTIVE) {
> +	if (!primary_if) {
>  		ret = -ENOENT;
>  		goto out_put_mesh_iface;
>  	}
> 
> 
> And now we are most likely on the right path... primary_if is holding a 
> reference to an hardif and therefore also a reference to the netdev. And the 
> error handling is only taking care of releasing the reference to the meshif 
> but not the primary_if.

Ah, indeed. Since batadv_primary_if_get_selected() is calling kref_get_unless_zero(),
primary_if->if_status != BATADV_IF_ACTIVE case needs to call kref_put().
Also, this matches my what I thought as a problem (BATADV_IF_TO_BE_ACTIVATED prevents
operations that depends on BATADV_IF_ACTIVE from working as expected).

> I will later send a fix for this with you and 
> syzbot+881d65229ca4f9ae8c84@syzkaller.appspotmail.com as Reported-by. Is this
> okay for you?

Yes, the reproducer no longer shows the problem.

Thank you.

Re: unregister_netdevice: waiting for batadv_slave_0 to become free. Usage count = 2

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 3915 bytes --]

On Sunday, 28 September 2025 03:06:05 CEST Tetsuo Handa wrote:
> Thank you for responding quickly.
> 
> On 2025/09/28 2:21, Sven Eckelmann wrote:
> > The question would now be: what is the actual problem? 
> 
> Sorry, my explanation was not clear enough.

It was long and contained a lot of things - but not what the actual problem is. 
It is necessary to read a lot of inline calltraces with subclauses - and then 
by reading between the lines, we must figure out what you actually wanted to 
say.

It is no problem to not know the underlying problem. But all these absolute 
statements, accusations and overly detailed statement made me think that I am 
just too stupid to get it and you must be right.

> What I thought as a problem is the difference between
> 
> 	netlink_device_change(&nlmsg, sock, "batadv_slave_0", true, "batadv0", 0, 0);
> 	//netlink_device_change(&nlmsg, sock, "batadv_slave_0", true, 0, &macaddr, ETH_ALEN);
> 
> and
> 
> 	netlink_device_change(&nlmsg, sock, "batadv_slave_0", false, "batadv0", 0, 0);
> 	netlink_device_change(&nlmsg, sock, "batadv_slave_0", true, 0, &macaddr, ETH_ALEN);
> 
> . The former makes hard_iface->if_status == BATADV_IF_ACTIVE while the latter makes
> hard_iface->if_status == BATADV_IF_TO_BE_ACTIVATED (because batadv_iv_ogm_schedule_buff()
> is not called).
> 
> This difference makes operations that depend on hard_iface->if_status == BATADV_IF_ACTIVE
> impossible to work properly. You can confirm this difference using diff show below.

This is again (in my opinion) this kind of (odd) absolute statement again. 
"impossible to work properly" - this sounds like BATADV_IF_TO_BE_ACTIVATED is 
an state which you cannot escape. And that functions/operations depend on 
BATADV_IF_ACTIVE. Both statements are not really true. 

BATADV_IF_TO_BE_ACTIVATED is a transient state and some algorithm depending 
code is responsible to automatically get it in the BATADV_IF_ACTIVE state. 
This is somewhat important here because the first time I read your second 
mail, I was under the impression that something in the reproducer showed that 
the state would be stuck. I searched rather hard in the code but couldn't find 
the reason for this. Only much later, I decided to ignore all this and look 
what the reproducer is actually doing. And also ignore commit 9e6b5648bbc4 
("batman-adv: Fix duplicated OGMs on NETDEV_UP") - because it was impossible 
for me to reproduce it on this commit.

And regarding the functions/operations which "impossible to work properly": 
called functions must "work properly" independent of the state. Just what 
they are doing as work can be different depending on the state. But maybe this 
is a case of "glass is half full" vs "glass is half empty".

The problem is therefore that some function broke this "promise". Your second 
mail (and the patch) was then basically saying "BATADV_IF_TO_BE_ACTIVATED" must 
not exist and we must directly go to BATADV_IF_ACTIVE. (Even if this is in my 
opinion not the right statement) it never said why it must not exist and what 
broke because of "BATADV_IF_TO_BE_ACTIVATED".

The inline calltraces with detailed statements in subclauses make it 
harder to digest. Some small high level statements like 

"I don't know exactly what the underlying problem is but skipping 
BATADV_IF_TO_BE_ACTIVATED in batadv_hardif_activate_interface() seems to work 
around the problem. I suspect that some function is not handling 
BATADV_IF_TO_BE_ACTIVATED correctly. Maybe some kind of race condition between 
switching to BATADV_IF_ACTIVE and executing some specific code. Here are my 
detailed notes:"

would have helped me not to get stuck too long in the interpretation of 
paragraphs. But at the same time, would have given a lot of pointers in the 
right direction. But maybe I would have been stuck anyway - no idea.

Anyway, I hope we found the problem now and thanks for the help.

Regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]