Re: [PATCH bluetooth] Bluetooth: qca: fix NVM tag length underflow in TLV parser
From: Johan Hovold
Date: Thu Jul 09 2026 - 07:06:11 EST
On Wed, Jul 08, 2026 at 11:15:00AM -0400, Luiz Augusto von Dentz wrote:
> On Wed, Jul 8, 2026 at 10:21 AM Johan Hovold <johan@xxxxxxxxxx> wrote:
> > On Tue, Jul 07, 2026 at 10:55:23AM -0700, Xiang Mei wrote:
> > > On Sun, Jul 5, 2026 at 2:41 AM Johan Hovold <johan@xxxxxxxxxx> wrote:
> > > > On Sat, Jul 04, 2026 at 04:10:30PM -0700, Xiang Mei wrote:
> > > > > The loop body
> > > > > then reads a 12-byte struct tlv_type_nvm past the end of the short
> > > > > vmalloc'd firmware buffer (and the EDL_TAG_ID_* handlers can write past it).
> > > >
> > > > No, only four bytes are read before the loop exits without any further
> > > > consequences due to the following check:
> >
> > > Thanks Johan, and you're right. Only the 2-byte tag_id (and at most
> > > the 4 bytes of tag_id/tag_len) is read out of bounds. This commit
> > > message overstated the impact, and I should have caught that.
> > >
> > > Unfortunately, the bot merged this patch when I was preparing the v2
> > > this morning. If you think the inaccurate commit message is worth
> > > fixing on record, I can send a follow-up to the maintainers to correct
> > > the message
> >
> > It was Luiz who applied the patch -- perhaps he did not notice my
> > conditional Reviewed-by tag.
> >
> > I assume the Bluetooth tree is not rebased so I don't think there's
> > anything that can be done here and there is no need to send a v2 (unless
> > Luiz says otherwise).
>
> If only the commit message that needs updating then I guess we don't
> need to worry about if stating something slightly inaccurate.
It's mostly an issue for backporting and determining the impact of the
fix. The commit message as it stands suggests that the bug can result in
memory corruption when it really is just a 4-byte oob read.
And then the inclusion of my tag makes it look like I failed to spot
this during review.
But let's keep it this way this time.
Johan