All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sven Eckelmann <sven@narfation.org>
To: Marek Lindner <marek.lindner@mailbox.org>,
	Simon Wunderlich <sw@simonwunderlich.de>,
	Antonio Quartulli <antonio@mandelbit.com>,
	Ibrahim Hashimov <security@auditcode.ai>
Cc: b.a.t.m.a.n@lists.open-mesh.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH net] batman-adv: bound BLA claim and backbone gateway table growth
Date: Fri, 10 Jul 2026 19:56:41 +0200	[thread overview]
Message-ID: <9098133.T7Z3S40VBb@sven-desktop> (raw)
In-Reply-To: <20260710165224.39411-1-security@auditcode.ai>

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

On Friday, 10 July 2026 18:52:24 CEST Ibrahim Hashimov wrote:
> batadv_bla_add_claim() and batadv_bla_get_backbone_gw() each kzalloc a
> new hash entry (struct batadv_bla_claim / struct batadv_bla_backbone_gw)
> for every distinct (mac, vid) / (orig, vid) pair carried in a
> BLA-group-conforming CLAIM frame, and insert it into
> bat_priv->bla.claim_hash / backbone_hash. There is no maximum-entry
> cap on either table -- entries are only ever removed by the
> timeout-driven periodic purge (batadv_bla_purge_claims() /
> batadv_bla_purge_backbone_gw(), BATADV_BLA_CLAIM_TIMEOUT /
> BATADV_BLA_BACKBONE_TIMEOUT, on the order of 100 s).
> 
> The BLA group a frame must carry is htons(crc16(primary_hardif_mac)),
> which is trivially observable/derivable by any node already on the
> mesh soft-interface (batadv_check_claim_group()). A single on-mesh
> sender can therefore emit CLAIM frames with an incrementing source MAC
> and force one kzalloc(GFP_ATOMIC) per frame on both hot paths, growing
> kernel memory without bound for as long as the attacker keeps sending
> -- uncontrolled resource allocation. Both allocations are
> GFP_ATOMIC with a NULL check, so this is a graceful memory-pressure
> DoS, not a crash: there is no OOB access.
> 
> batman-adv already has an established pattern for capping an
> attacker-/peer-influenced, unbounded-growth per-mesh-interface
> resource: the TP-meter session list bounds concurrent sessions with a
> fixed ceiling and an atomic_add_unless() admission check that rejects
> new allocations once the cap is hit, logging and freeing/decrementing
> on the abort path (net/batman-adv/tp_meter.c, BATADV_TP_MAX_NUM,
> bat_priv->tp_num, "Meter: too many ongoing sessions, aborting").
> 
> Apply the same pattern to BLA: add two atomic_t counters,
> bat_priv->bla.num_claims and bat_priv->bla.num_backbone_gws, each
> capped at a new BATADV_BLA_MAX_CLAIMS / BATADV_BLA_MAX_BACKBONE_GW
> limit (4096 / 256 -- generous for any real bridged LAN/VLAN
> population, several orders of magnitude below what would need to be
> sprayed to threaten memory availability). batadv_bla_add_claim() and
> batadv_bla_get_backbone_gw() reserve a slot with atomic_add_unless()
> before allocating; on cap-hit the frame is dropped (matching existing
> "drop silently, let the sender resync/backoff" BLA behaviour) instead
> of allocating. The reservation is released on every existing early-out
> (kzalloc failure, hash_add failure) and in the kref release paths
> (batadv_claim_release(), batadv_backbone_gw_release()), where the
> counters are decremented right before the objects are freed. No
> locking changes are needed: the counters are only ever touched via
> atomic ops, mirroring tp_num.
> 
> This does not change on-the-wire behaviour, hash table sizing, or
> timeout-based purging; it only stops a single on-mesh peer from
> growing the tables past a bounded ceiling.
> 
> Verified by code review rather than by driving either counter to its
> cap at runtime: the atomic_add_unless()/atomic_dec() pairing was
> checked against every existing early-out (kzalloc failure, hash_add
> failure) and against both kref release callbacks, confirming exactly
> one reservation and one release per entry, mirroring the same
> tp_num accounting in tp_meter.c. A loopback CLAIM-frame reproducer
> was used earlier to confirm the pre-fix unbounded growth itself
> (distinct claim_hash/backbone_hash entries scale linearly with the
> number of distinct (mac, vid) pairs sent), but reaching the new
> 4096 / 256 caps with that same reproducer is impractical: entries
> age out via the existing timeout-driven purge faster than a
> single-host reproducer can accumulate enough distinct pairs to hit
> the ceiling, so the cap-hit and slot-release paths were exercised
> by inspection, not by a live saturation run.
> 
> Fixes: 23721387c409 ("batman-adv: add basic bridge loop avoidance code")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
> Assisted-by: AuditCode-AI:2026.07
> ---
>  net/batman-adv/bridge_loop_avoidance.c | 38 ++++++++++++++++++++++++--
>  net/batman-adv/main.h                  |  3 ++
>  net/batman-adv/types.h                 |  6 ++++
>  3 files changed, 45 insertions(+), 2 deletions(-)


Consider this rejected:

* the target tree should be batadv and not "net"
* patch doesn't even apply to batadv.git

  $ git am 20260710165224.39411-1-security@auditcode.ai.mbx
  Applying: batman-adv: bound BLA claim and backbone gateway table growth
  error: patch failed: net/batman-adv/bridge_loop_avoidance.c:179
  error: net/batman-adv/bridge_loop_avoidance.c: patch does not apply
  Patch failed at 0001 batman-adv: bound BLA claim and backbone gateway table growth
  hint: Use 'git am --show-current-patch=diff' to see the failed patch
  hint: When you have resolved this problem, run "git am --continue".
  hint: If you prefer to skip this patch, run "git am --skip" instead.
  hint: To restore the original branch and stop patching, run "git am --abort".
  hint: Disable this message with "git config advice.mergeConflict false"

* there are already patches for these for review [1]
* breaks existing setups and doesn't allow users to adjust limits according to 
  their requirements
* own backbones must never be prevented
* I start to get tired by all these overlong AI generated patch summaries 
  which meander around completely irrelevant detail information - or 
  information which are not even relevant here

Regards,
	Sven

[1] https://patchwork.open-mesh.org/project/b.a.t.m.a.n./list/?series=726
    https://git.open-mesh.org/pub/ecsv/batman-adv.git/log/?h=b4/resource-limit

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

      reply	other threads:[~2026-07-10 17:57 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-10 16:52 [PATCH net] batman-adv: bound BLA claim and backbone gateway table growth Ibrahim Hashimov
2026-07-10 17:56 ` Sven Eckelmann [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9098133.T7Z3S40VBb@sven-desktop \
    --to=sven@narfation.org \
    --cc=antonio@mandelbit.com \
    --cc=b.a.t.m.a.n@lists.open-mesh.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marek.lindner@mailbox.org \
    --cc=security@auditcode.ai \
    --cc=stable@vger.kernel.org \
    --cc=sw@simonwunderlich.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.