Re: [BlueZ, v2 2/3] unit: Adapt poc_*_oob.c test into a new test

On Wed, 2026-08-05 at 12:44 -0400, Luiz Augusto von Dentz wrote:
> Hi Bastien,
> 
> On Wed, Aug 5, 2026 at 3:59 AM Bastien Nocera <[email protected]>
> wrote:
> > 
> > On Tue, 2026-08-04 at 12:46 -0400, Luiz Augusto von Dentz wrote:
> > > Hi Bastien,
> > > 
> > > On Tue, Aug 4, 2026 at 10:31 AM Bastien Nocera
> > > <[email protected]>
> > > wrote:
> > > > 
> > > > Adapt poc_avrcp_oob.c and poc_folder_oob.c into unit tests.
> > > > 
> > > > Co-authored-by: Elman Shahbazov <[email protected]>
> > > > ---
> > > >  Makefile.am           |  9 +++++
> > > >  unit/test-avrcp-sec.c | 76
> > > > +++++++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 85 insertions(+)
> > > >  create mode 100644 unit/test-avrcp-sec.c
> > > > 
> > > > diff --git a/Makefile.am b/Makefile.am
> > > > index 19c468d3a504..e3baa4155c1f 100644
> > > > --- a/Makefile.am
> > > > +++ b/Makefile.am
> > > > @@ -660,6 +660,15 @@ unit_test_avrcp_SOURCES = unit/test-
> > > > avrcp.c \
> > > >  unit_test_avrcp_LDADD = lib/libbluetooth-internal.la \
> > > >                                 src/libshared-glib.la
> > > > $(GLIB_LIBS)
> > > > 
> > > > +unit_tests += unit/test-avrcp-sec
> > > > +
> > > > +unit_test_avrcp_sec_SOURCES = unit/test-avrcp-sec.c \
> > > > +                       profiles/audio/avrcp-parse.c \
> > > > +                       profiles/audio/avrcp-parse.h \
> > > > +                       src/log.h src/log.c
> > > > +unit_test_avrcp_sec_LDADD = lib/libbluetooth-internal.la \
> > > > +                               src/libshared-glib.la
> > > > $(GLIB_LIBS)
> > > 
> > > Any reason why this couldn't live inside test-avrcp.c?
> > 
> > Because most of the code in test-avrcp.c is mocked in unit/avrcp-
> > lib.[ch] and conflicts with code from profiles/audio/avrcp.c. I
> > really
> > did try...
> 
> Ok, well then perhaps we should start testing what we really use in
> the avrcp plugin rather than the removed Android code, or we could
> have a bigger task: converting this type of code to
> src/shared/avrcp.c
> (e.g. bt_avrcp) so it can be unit tested.

I figured that this code would actually test *something*, but it looks
like I was wrong.

I'll re-send the patch to remove the dead code for unit/avrcp.c, and
send a follow-up patch to remove this code here, and reinstate it with
this test.

Right now, "make coverage" says that profiles/audio has no coverage at
all...

> > I can change the name of the test if needed.
> 
> I'm not concerned about the name, I'm concerned about the number of
> unit test files which could otherwise be in a single file instead of
> duplicating a lot of context.
> 
> > > 
> > > >  unit_tests += unit/test-hfp
> > > > 
> > > >  unit_test_hfp_SOURCES = unit/test-hfp.c
> > > > diff --git a/unit/test-avrcp-sec.c b/unit/test-avrcp-sec.c
> > > > new file mode 100644
> > > > index 000000000000..a100b2d68e35
> > > > --- /dev/null
> > > > +++ b/unit/test-avrcp-sec.c
> > > > @@ -0,0 +1,76 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > > +/*
> > > > + *
> > > > + *  BlueZ - Bluetooth protocol stack for Linux
> > > > + *
> > > > + *  Copyright (C) 2026  Red Hat Inc.
> > > > + *
> > > > + *
> > > > + */
> > > > +
> > > > +#ifdef HAVE_CONFIG_H
> > > > +#include <config.h>
> > > > +#endif
> > > > +
> > > > +#include <glib.h>
> > > > +
> > > > +#include "src/shared/util.h"
> > > > +#include "src/shared/tester.h"
> > > > +#include "src/log.h"
> > > > +
> > > > +#include "profiles/audio/avrcp-parse.h"
> > > > +
> > > > +static void avrcp_element_name_oob(gconstpointer data)
> > > > +{
> > > > +       char name[255];
> > > > +       uint16_t namesize;
> > > > +       gboolean ret;
> > > > +
> > > > +       /* Crafting a malicious payload.
> > > > +        * Actual packet length (len) = 14 bytes */
> > > > +       uint8_t malicious_packet[14] = {0};
> > > > +
> > > > +       /* Specify namesize = 1000 (0x03E8 in Big Endian) at
> > > > offset
> > > > 11 */
> > > > +       malicious_packet[11] = 0x03;
> > > > +       malicious_packet[12] = 0xE8;
> > > > +
> > > > +       /* Launching the PoC. We transmit a 14-byte packet, but
> > > > namesize=1000... */
> > > > +       ret = parse_media_element_name(malicious_packet,
> > > > sizeof(malicious_packet),
> > > > +                                      name, &namesize);
> > > > +       if (ret)
> > > > +               tester_test_passed();
> > > > +       else
> > > > +               tester_test_failed();
> > > > +}
> > > > +
> > > > +static void avrcp_folder_name_oob(gconstpointer data)
> > > > +{
> > > > +       char name[255];
> > > > +       gboolean ret;
> > > > +
> > > > +       /* Crafting a malicious payload.
> > > > +        * Actual packet length (len) = 14 bytes */
> > > > +       uint8_t malicious_packet[14] = {0};
> > > > +
> > > > +       /* Specify namesize = 1000 (0x03E8 in Big Endian) at
> > > > offset
> > > > 12 */
> > > > +       malicious_packet[12] = 0x03;
> > > > +       malicious_packet[13] = 0xE8;
> > > > +
> > > > +       /* Launching the PoC. We transmit a 14-byte packet, but
> > > > namesize=1000... */
> > > > +       ret = parse_media_folder_name(malicious_packet,
> > > > sizeof(malicious_packet),
> > > > +                                     name);
> > > > +       if (ret)
> > > > +               tester_test_passed();
> > > > +       else
> > > > +               tester_test_failed();
> > > > +}
> > > > +
> > > > +int main(int argc, char *argv[])
> > > > +{
> > > > +       tester_init(&argc, &argv);
> > > > +
> > > > +       tester_add("/avrcp-element-name-oob", NULL, NULL,
> > > > avrcp_element_name_oob, NULL);
> > > > +       tester_add("/avrcp-folder-name-oob", NULL, NULL,
> > > > avrcp_folder_name_oob, NULL);
> > > > +
> > > > +       return tester_run();
> > > > +}
> > > > --
> > > > 2.55.0
> > > > 
> > > > 
> > > 
> 
>