Re: Bluetooth: mgmt: Fix heap overflow in mgmt_mesh_add

[Daniel Matsumoto]

> That seems valid, although it lacks a reproducer, since we need to
> protect the list mesh_pending.

Actually, looking at the locking semantics, 003ca042a386 is also flawed .

1. The list is already protected. The only caller of mgmt_mesh_add()
is mesh_send(), which explicitly acquires hci_dev_lock(hdev) before
the allocation and list insertion.
2. The patch uses the wrong mutex. It adds
guard(mutex)(&hdev->mgmt_pending_lock) to protect hdev->mesh_pending.
Isn't that lock meant for the mgmt_pending list? not the mesh lists?
3. The locking is asymmetric. The guard is added to the add() and
find() paths, but mgmt_mesh_remove() (which does list_del()) and
mgmt_mesh_next() are left untouched.
4. It replaces a fast path with a blocking lock. In mgmt_mesh_find(),
the patch removes a lockless list_empty() optimization, forcing the
code to acquire an expensive mutex just to iterate an empty list.

Since the execution path is already serialized by the primary device
lock, adding an orthogonal mutex here only introduces overhead and
potential deadlocks.
I suggest not merging that patch as well.


On Thu, Feb 19, 2026 at 2:49 PM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> Hi Maiquel,
>
> On Thu, Feb 19, 2026 at 12:08 PM Maiquel Paiva <maiquelpaiva@xxxxxxxxx> wrote:
> >
> > Thank you for the detailed follow-up.
> > The explanation about EXPORT_SYMBOL makes perfect sense.
> >
> > I was analyzing the function's limits in complete isolation,
> > and didn't realize the context of the trust limit within the module itself.
> >
> > I will certainly use this as a great learning experience,
> > (it's never too late to learn!)
> >
> > I fully agree with reverting commit ac0c6f1b6a58
> > ("Bluetooth: mgmt: Fix heap overflow in mgmt_mesh_add")
> > to avoid confusion and unnecessary code changes,
> > since the function that calls mesh_send already handles sanitization.
> >
> > Just to confirm: what will happen to the other commit in this series that addresses the blocking problem
> > (003ca042a386)? The handling of the mesh_pending list was indeed unprotected
> > that's exactly what guard(mutex) is for.
>
> That seems valid, although it lacks a reproducer, since we need to
> protect the list mesh_pending.
>
> > Thank you for the review.
> >
> > Thanks,
> > Maiquel Paiva
> >
> > Em qui., 19 de fev. de 2026 às 13:23, Daniel Matsumoto <insidetf2@xxxxxxxxx> escreveu:
> >>
> >> Hi Luiz,
> >>
> >> Makes perfect sense regarding EXPORT_SYMBOL. Thanks for taking a look
> >> and dropping it.
> >>
> >> Regards,
> >> Daniel
> >>
> >>
> >> On Thu, Feb 19, 2026 at 1:16 PM Luiz Augusto von Dentz
> >> <luiz.dentz@xxxxxxxxx> wrote:
> >> >
> >> > Hi Daniel,
> >> >
> >> > On Tue, Feb 17, 2026 at 1:09 PM Daniel Matsumoto <me@xxxxxxxx> wrote:
> >> > >
> >> > > Regarding commit ac0c6f1b6a58 ("Bluetooth: mgmt: Fix heap overflow in
> >> > > mgmt_mesh_add"):
> >> > >
> >> > > I reviewed the call path for this patch and the overflow condition
> >> > > appears to be unreachable in the current tree.
> >> > > The only caller of mgmt_mesh_add() is mesh_send() in
> >> > > net/bluetooth/mgmt_util.c. The length parameter is explicitly
> >> > > sanitized before the call:
> >> > >
> >> > > if (!hci_dev_test_flag(hdev, HCI_LE_ENABLED) ||
> >> > >    len <= MGMT_MESH_SEND_SIZE ||
> >> > >    len > (MGMT_MESH_SEND_SIZE + 31))
> >> > > return mgmt_cmd_status(sk, hdev->id, MGMT_OP_MESH_SEND,
> >> > >       MGMT_STATUS_REJECTED);
> >> > >
> >> > > Given that mgmt_mesh_add() allocates sizeof(*mesh_tx), which includes
> >> > > the param buffer sized for this maximum length, the bounds check
> >> > > introduced in the commit is redundant.
> >> > > While defensive programming is valid, tagging this as a fix for a heap
> >> > > overflow is misleading for backporters and security scanners, as the
> >> > > overflow cannot be triggered.
> >> >
> >> > Yeah, well I would say it would only be valid to apply defensive
> >> > programming if that function would be marked with EXPORT_SYMBOL so it
> >> > could be used outside of net/bluetooth context.
> >> >
> >> > > Please consider dropping this from the stable queue to avoid
> >> > > unnecessary code churn.
> >> >
> >> > +1, will drop it entirely, it seems I will need to ask for more
> >> > evidence as apparently people are relying too much on LLVM nowadays.
> >> >
> >> > --
> >> > Luiz Augusto von Dentz
> >> >
>
>
>
> --
> Luiz Augusto von Dentz