[PATCH net] batman-adv: Close OGM aggregation before transmission

[PATCH net] batman-adv: Close OGM aggregation before transmission

[Chengfeng Ye]

The OGM send worker leaves its forwarding packet on forw_bat_list until
after batadv_iv_ogm_emit() returns. Aggregation holds forw_bat_list_lock,
but emission reads and clones the packet without that lock.

CPU 0 can therefore start emitting a queued packet while CPU 1 takes the
list lock, finds the same packet and appends another OGM. The sender can
observe the new packet length before the corresponding direct-link flag
is set, or clone the skb while its length and payload are being updated.
This can transmit an OGM with incorrect flags or inconsistent data.

KCSAN reported:

  BUG: KCSAN: data-race in batadv_iv_ogm_queue_add / batadv_iv_send_outstanding_bat_ogm_packet
  write to 0xffff888100fedcf8 of 2 bytes by interrupt on cpu 1:
  read to 0xffff888100fedcf8 of 2 bytes by task 70 on cpu 2:
  value changed: 0x00c0 -> 0x00d8

Set num_packets to BATADV_MAX_AGGREGATION_PACKETS under the list lock
before emission. This waits for any ongoing append and makes the existing
aggregation limit check reject further appends before inspecting mutable
OGM flags. Emission walks packet_len rather than num_packets, so the
queued contents are still sent normally. Keep the packet on the list so
interface purging can still wait for the worker and retain its existing
ownership of the packet when freeing it.

Backports before Linux 6.15 need count-handling adaptation.

Fixes: 9b4aec647a92 ("batman-adv: fix rare race conditions on interface removal")
Cc: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 net/batman-adv/bat_iv_ogm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 53fbdbbe8f4f..59847299a123 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -1909,6 +1909,10 @@ static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
 		goto out;
 	}
 
+	spin_lock_bh(&bat_priv->forw_bat_list_lock);
+	forw_packet->num_packets = BATADV_MAX_AGGREGATION_PACKETS;
+	spin_unlock_bh(&bat_priv->forw_bat_list_lock);
+
 	batadv_iv_ogm_emit(forw_packet);
 
 	/* we have to have at least one packet in the queue to determine the

base-commit: 165768bb70265b5c38cf0b73fafd75be235f8b14
-- 
2.43.0

Re: [PATCH net] batman-adv: Close OGM aggregation before transmission

[Sven Eckelmann]

Thanks for the fix.

Please don't submit batman-adv changes to netdev directly. And please send the
patch also to the person which introduced the problem. You can use tools like
`b4 prep --auto-to-cc` or just the underlying ./scripts/get_maintainer.pl 

    $ b4 mbox 20260926172600.2394088-1-nicoyip.dev@gmail.com
    $ ./scripts/get_maintainer.pl 20260926172600.2394088-1-nicoyip.dev@gmail.com.mbx
    Marek Lindner <marek.lindner@mailbox.org> (maintainer:BATMAN ADVANCED)
    Simon Wunderlich <sw@simonwunderlich.de> (maintainer:BATMAN ADVANCED,blamed_fixes:1/1=100%)
    Antonio Quartulli <antonio@mandelbit.com> (maintainer:BATMAN ADVANCED)
    Sven Eckelmann <sven@narfation.org> (maintainer:BATMAN ADVANCED,blamed_fixes:1/1=100%)
    "Linus Lüssing" <linus.luessing@c0d3.blue> (blamed_fixes:1/1=100%)
    b.a.t.m.a.n@lists.open-mesh.org (moderated list:BATMAN ADVANCED)
    linux-kernel@vger.kernel.org (open list)

This change should not go directly to net.git. The target tree must be
batadv.git

    $ ./scripts/get_maintainer.pl --scm 20260926172600.2394088-1-nicoyip.dev@gmail.com.mbx
    [...]
    git https://git.open-mesh.org/batadv.git
    git git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

The last tree is irrelevant for you because it is really unlikely that Linus
Torvalds will apply your patch directly.

> The OGM send worker leaves its forwarding packet on forw_bat_list until
> after batadv_iv_ogm_emit() returns. Aggregation holds forw_bat_list_lock,
> but emission reads and clones the packet without that lock.
> 
> CPU 0 can therefore start emitting a queued packet while CPU 1 takes the
> list lock, finds the same packet and appends another OGM. The sender can
> observe the new packet length before the corresponding direct-link flag
> is set, or clone the skb while its length and payload are being updated.
> This can transmit an OGM with incorrect flags or inconsistent data.
> 
> KCSAN reported:
> 
>   BUG: KCSAN: data-race in batadv_iv_ogm_queue_add / batadv_iv_send_outstanding_bat_ogm_packet
>   write to 0xffff888100fedcf8 of 2 bytes by interrupt on cpu 1:
>   read to 0xffff888100fedcf8 of 2 bytes by task 70 on cpu 2:
>   value changed: 0x00c0 -> 0x00d8

Is there a reproducer and where can I find it?

> Set num_packets to BATADV_MAX_AGGREGATION_PACKETS under the list lock
> before emission. This waits for any ongoing append and makes the existing
> aggregation limit check reject further appends before inspecting mutable
> OGM flags. Emission walks packet_len rather than num_packets, so the
> queued contents are still sent normally. Keep the packet on the list so
> interface purging can still wait for the worker and retain its existing
> ownership of the packet when freeing it.

I might be wrong but this reads a little bit like it was written by an LLM. If
this is the case, please think about following the annotation style described
in https://docs.kernel.org/process/coding-assistants.html

> Backports before Linux 6.15 need count-handling adaptation.

This is wrong. You are depending on 434becf57bdc ("batman-adv: Limit number of
aggregated packets directly") and not the counting adaption. Of course, for
4.10.x-6.14.x, you also need to change the BATADV_MAX_AGGREGATION_PACKETS to
BITS_PER_TYPE(forw_packet->direct_link_flags)

And it doesn't belong in the commit message (like this). Please annotate the
dependencies as described in
https://docs.kernel.org/process/stable-kernel-rules.html#stable-kernel-rules

> Fixes: 9b4aec647a92 ("batman-adv: fix rare race conditions on interface removal")
> Cc: stable@vger.kernel.org
> Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
>
> diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
> index 53fbdbbe8f4f7..59847299a1237 100644
> --- a/net/batman-adv/bat_iv_ogm.c
> +++ b/net/batman-adv/bat_iv_ogm.c
> @@ -1909,6 +1909,10 @@ static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
>  		goto out;
>  	}
>  
> +	spin_lock_bh(&bat_priv->forw_bat_list_lock);
> +	forw_packet->num_packets = BATADV_MAX_AGGREGATION_PACKETS;
> +	spin_unlock_bh(&bat_priv->forw_bat_list_lock);
> +

Can you add a small comment above like "mark aggregate as full before forcing
emit" (or something similar).

-- 
Sven Eckelmann <sven@narfation.org>

Re: [PATCH net] batman-adv: Close OGM aggregation before transmission

[Chengfeng Ye]

Thanks for the quick review.

On Sun, Sep 27, 2026 at 4:19 PM Sven Eckelmann <sven@narfation.org> wrote:
>
> Thanks for the fix.
>
> Please don't submit batman-adv changes to netdev directly. And please send the
> patch also to the person which introduced the problem. You can use tools like
> `b4 prep --auto-to-cc` or just the underlying ./scripts/get_maintainer.pl
>
>     $ b4 mbox 20260926172600.2394088-1-nicoyip.dev@gmail.com
>     $ ./scripts/get_maintainer.pl 20260926172600.2394088-1-nicoyip.dev@gmail.com.mbx
>     Marek Lindner <marek.lindner@mailbox.org> (maintainer:BATMAN ADVANCED)
>     Simon Wunderlich <sw@simonwunderlich.de> (maintainer:BATMAN ADVANCED,blamed_fixes:1/1=100%)
>     Antonio Quartulli <antonio@mandelbit.com> (maintainer:BATMAN ADVANCED)
>     Sven Eckelmann <sven@narfation.org> (maintainer:BATMAN ADVANCED,blamed_fixes:1/1=100%)
>     "Linus Lüssing" <linus.luessing@c0d3.blue> (blamed_fixes:1/1=100%)
>     b.a.t.m.a.n@lists.open-mesh.org (moderated list:BATMAN ADVANCED)
>     linux-kernel@vger.kernel.org (open list)
>
> This change should not go directly to net.git. The target tree must be
> batadv.git

No problem, I will send the v2 to batadv.

>
>     $ ./scripts/get_maintainer.pl --scm 20260926172600.2394088-1-nicoyip.dev@gmail.com.mbx
>     [...]
>     git https://git.open-mesh.org/batadv.git
>     git git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>
> The last tree is irrelevant for you because it is really unlikely that Linus
> Torvalds will apply your patch directly.
>
> > The OGM send worker leaves its forwarding packet on forw_bat_list until
> > after batadv_iv_ogm_emit() returns. Aggregation holds forw_bat_list_lock,
> > but emission reads and clones the packet without that lock.
> >
> > CPU 0 can therefore start emitting a queued packet while CPU 1 takes the
> > list lock, finds the same packet and appends another OGM. The sender can
> > observe the new packet length before the corresponding direct-link flag
> > is set, or clone the skb while its length and payload are being updated.
> > This can transmit an OGM with incorrect flags or inconsistent data.
> >
> > KCSAN reported:
> >
> >   BUG: KCSAN: data-race in batadv_iv_ogm_queue_add / batadv_iv_send_outstanding_bat_ogm_packet
> >   write to 0xffff888100fedcf8 of 2 bytes by interrupt on cpu 1:
> >   read to 0xffff888100fedcf8 of 2 bytes by task 70 on cpu 2:
> >   value changed: 0x00c0 -> 0x00d8
>
> Is there a reproducer and where can I find it?

I will prepare one and send it right away. The KCSAN was triggered
with a kernel-side delay() instrumentation.

> > Set num_packets to BATADV_MAX_AGGREGATION_PACKETS under the list lock
> > before emission. This waits for any ongoing append and makes the existing
> > aggregation limit check reject further appends before inspecting mutable
> > OGM flags. Emission walks packet_len rather than num_packets, so the
> > queued contents are still sent normally. Keep the packet on the list so
> > interface purging can still wait for the worker and retain its existing
> > ownership of the packet when freeing it.
>
> I might be wrong but this reads a little bit like it was written by an LLM. If
> this is the case, please think about following the annotation style described
> in https://docs.kernel.org/process/coding-assistants.html

Yes, it was generated with gpt-6-Astra, and I will add the missing tag
on v2 following the doc.

> > Backports before Linux 6.15 need count-handling adaptation.
>
> This is wrong. You are depending on 434becf57bdc ("batman-adv: Limit number of
> aggregated packets directly") and not the counting adaption. Of course, for
> 4.10.x-6.14.x, you also need to change the BATADV_MAX_AGGREGATION_PACKETS to
> BITS_PER_TYPE(forw_packet->direct_link_flags)
> And it doesn't belong in the commit message (like this). Please annotate the
> dependencies as described in
> https://docs.kernel.org/process/stable-kernel-rules.html#stable-kernel-rules
>
> > Fixes: 9b4aec647a92 ("batman-adv: fix rare race conditions on interface removal")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
> >
> > diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
> > index 53fbdbbe8f4f7..59847299a1237 100644
> > --- a/net/batman-adv/bat_iv_ogm.c
> > +++ b/net/batman-adv/bat_iv_ogm.c
> > @@ -1909,6 +1909,10 @@ static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
> >               goto out;
> >       }
> >
> > +     spin_lock_bh(&bat_priv->forw_bat_list_lock);
> > +     forw_packet->num_packets = BATADV_MAX_AGGREGATION_PACKETS;
> > +     spin_unlock_bh(&bat_priv->forw_bat_list_lock);
> > +
>
> Can you add a small comment above like "mark aggregate as full before forcing
> emit" (or something similar).

No problem, the commit message and the comment will be adjusted accordingly.

> --
> Sven Eckelmann <sven@narfation.org>

[PATCH v2] batman-adv: Close OGM aggregation before transmission

[Chengfeng Ye]

The OGM send worker leaves its forwarding packet on forw_bat_list until
after batadv_iv_ogm_emit() returns. Aggregation holds forw_bat_list_lock,
but emission reads and clones the packet without that lock.

CPU 0 can therefore start emitting a queued packet while CPU 1 takes the
list lock, finds the same packet and appends another OGM. The sender can
observe the new packet length before the corresponding direct-link flag
is set, or clone the skb while its length and payload are being updated.
This can transmit an OGM with incorrect flags or inconsistent data.

KCSAN reported:

  BUG: KCSAN: data-race in batadv_iv_ogm_queue_add / batadv_iv_send_outstanding_bat_ogm_packet
  write to 0xffff888100fedcf8 of 2 bytes by interrupt on cpu 1:
  read to 0xffff888100fedcf8 of 2 bytes by task 70 on cpu 2:
  value changed: 0x00c0 -> 0x00d8

Mark the aggregate as full under forw_bat_list_lock before emission. This
waits for any ongoing append and prevents further aggregation. Emission
uses packet_len to walk the OGMs, so all queued packets are still sent.
Keep the packet on the list so interface purging can find the worker,
wait for it to finish and free the packet.

Fixes: 9b4aec647a92 ("batman-adv: fix rare race conditions on interface removal")
Assisted-by: GPT-6-Astra
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
Changes in v2:
- Rebase on batadv/net and route the patch to the batman-adv maintainers
  and mailing list, including the introducing author.
- Add a comment marking the aggregate as full before emission.
- Drop the stable-backport request and defer backport consideration
  pending maintainer assessment of practical impact.
- Add Assisted-by: GPT-6-Astra.

This revision is based on batadv.git, branch batadv/net.

The reproducer archive, batman-ogm-aggregation-race-reproducer.tar.gz,
was sent in a separate reply to this discussion. It contains the PoC,
prebuilt kernels, build scripts, configs and QEMU launcher. Reproduction
uses KCSAN and a conditional kernel-side mdelay(), capped at 50 ms, before
appending the ninth OGM. On Linux master fd179f8a05be, the vulnerable run
reported the target race; the fixed control reported none. Both runs
completed 240 cycles. These are instrumented runs.

The v2 batman-adv subsystem build and git diff --check passed. The code
change relative to v1 is the comment above the existing locking fix.
 net/batman-adv/bat_iv_ogm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index d8a6a0f64ce2..084970404d3e 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -1909,6 +1909,11 @@ static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
 		goto out;
 	}
 
+	/* Mark aggregate as full before forcing emit. */
+	spin_lock_bh(&bat_priv->forw_bat_list_lock);
+	forw_packet->num_packets = BATADV_MAX_AGGREGATION_PACKETS;
+	spin_unlock_bh(&bat_priv->forw_bat_list_lock);
+
 	batadv_iv_ogm_emit(forw_packet);
 
 	/* we have to have at least one packet in the queue to determine the

base-commit: abfe281aeab4d26b8e262ca9efc979a92c494aed
-- 
2.43.0

Re: [PATCH v2] batman-adv: Close OGM aggregation before transmission

[Sven Eckelmann]

On Sun, 27 Sep 2026 18:16:24 +0800, Chengfeng Ye wrote:
> batman-adv: Close OGM aggregation before transmission

Applied, thanks!

[1/1] batman-adv: Close OGM aggregation before transmission
      https://git.open-mesh.org/batadv/c/c31a4d235404

Best regards,
-- 
Sven Eckelmann <sven@narfation.org>

[PATCH v2] batman-adv: Close OGM aggregation before transmission

[Chengfeng Ye]

The OGM send worker leaves its forwarding packet on forw_bat_list until
after batadv_iv_ogm_emit() returns. Aggregation holds forw_bat_list_lock,
but emission reads and clones the packet without that lock.

CPU 0 can therefore start emitting a queued packet while CPU 1 takes the
list lock, finds the same packet and appends another OGM. The sender can
observe the new packet length before the corresponding direct-link flag
is set, or clone the skb while its length and payload are being updated.
This can transmit an OGM with incorrect flags or inconsistent data.

KCSAN reported:

  BUG: KCSAN: data-race in batadv_iv_ogm_queue_add / batadv_iv_send_outstanding_bat_ogm_packet
  write to 0xffff888100fedcf8 of 2 bytes by interrupt on cpu 1:
  read to 0xffff888100fedcf8 of 2 bytes by task 70 on cpu 2:
  value changed: 0x00c0 -> 0x00d8

Mark the aggregate as full under forw_bat_list_lock before emission. This
waits for any ongoing append and prevents further aggregation. Emission
uses packet_len to walk the OGMs, so all queued packets are still sent.
Keep the packet on the list so interface purging can find the worker,
wait for it to finish and free the packet.

Fixes: 9b4aec647a92 ("batman-adv: fix rare race conditions on interface removal")
Assisted-by: GPT-6-Astra
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
Changes in v2:
- Rebase on batadv/net and route the patch to the batman-adv maintainers
  and mailing list, including the introducing author.
- Add a comment marking the aggregate as full before emission.
- Drop the stable-backport request and defer backport consideration
  pending maintainer assessment of practical impact.
- Add Assisted-by: GPT-6-Astra.

This revision is based on batadv.git, branch batadv/net.

The reproducer archive, batman-ogm-aggregation-race-reproducer.tar.gz,
was sent in a separate reply to this discussion. It contains the PoC,
prebuilt kernels, build scripts, configs and QEMU launcher. Reproduction
uses KCSAN and a conditional kernel-side mdelay(), capped at 50 ms, before
appending the ninth OGM. On Linux master fd179f8a05be, the vulnerable run
reported the target race; the fixed control reported none. Both runs
completed 240 cycles. These are instrumented runs.

The v2 batman-adv subsystem build and git diff --check passed. The code
change relative to v1 is the comment above the existing locking fix.
 net/batman-adv/bat_iv_ogm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index d8a6a0f64ce2..084970404d3e 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -1909,6 +1909,11 @@ static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
 		goto out;
 	}
 
+	/* Mark aggregate as full before forcing emit. */
+	spin_lock_bh(&bat_priv->forw_bat_list_lock);
+	forw_packet->num_packets = BATADV_MAX_AGGREGATION_PACKETS;
+	spin_unlock_bh(&bat_priv->forw_bat_list_lock);
+
 	batadv_iv_ogm_emit(forw_packet);
 
 	/* we have to have at least one packet in the queue to determine the

base-commit: abfe281aeab4d26b8e262ca9efc979a92c494aed
-- 
2.43.0