Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220711233535.3134546-1-luiz.dentz@gmail.com>
 <CGME20220809085500eucas1p18cb2752164ca55832787e7d3843f48ab@eucas1p1.samsung.com>
 <20220711233535.3134546-2-luiz.dentz@gmail.com> <749ed578-bc9d-b699-3602-bee444fff9bb@samsung.com>
In-Reply-To: <749ed578-bc9d-b699-3602-bee444fff9bb@samsung.com>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Tue, 9 Aug 2022 12:24:54 -0700
Message-ID: <CABBYNZLsR0KT1-OVVjnUzLp0+U0eHGi0PNqKbfntp_5AHNDawA@mail.gmail.com>
Subject: Re: [PATCH v7 2/7] Bluetooth: Add initial implementation of CIS connections
To:     Marek Szyprowski <m.szyprowski@samsung.com>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Marek,

On Tue, Aug 9, 2022 at 1:55 AM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>
> Hi Luiz,
>
> On 12.07.2022 01:35, Luiz Augusto von Dentz wrote:
> > From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> >
> > This adds the initial implementation of CIS connections and introduces
> > the ISO packets/links.
> >
> > == Central: Set CIG Parameters, create a CIS and Setup Data Path ==
> >
> >> tools/isotest -s <address>
> > < HCI Command: LE Extended Create... (0x08|0x0043) plen 26
> > ...
> >> HCI Event: Command Status (0x0f) plen 4
> >        LE Extended Create Connection (0x08|0x0043) ncmd 1
> >          Status: Success (0x00)
> >> HCI Event: LE Meta Event (0x3e) plen 31
> >        LE Enhanced Connection Complete (0x0a)
> >        ...
> > < HCI Command: LE Create Connected... (0x08|0x0064) plen 5
> > ...
> >> HCI Event: Command Status (0x0f) plen 4
> >        LE Create Connected Isochronous Stream (0x08|0x0064) ncmd 1
> >          Status: Success (0x00)
> >> HCI Event: LE Meta Event (0x3e) plen 29
> >        LE Connected Isochronous Stream Established (0x19)
> >        ...
> > < HCI Command: LE Setup Isochronou.. (0x08|0x006e) plen 13
> > ...
> >> HCI Event: Command Complete (0x0e) plen 6
> >        LE Setup Isochronous Data Path (0x08|0x006e) ncmd 1
> >          Status: Success (0x00)
> >          Handle: 257
> > < HCI Command: LE Setup Isochronou.. (0x08|0x006e) plen 13
> > ...
> >> HCI Event: Command Complete (0x0e) plen 6
> >        LE Setup Isochronous Data Path (0x08|0x006e) ncmd 1
> >          Status: Success (0x00)
> >          Handle: 257
> >
> > == Peripheral: Accept CIS and Setup Data Path ==
> >
> >> tools/isotest -d
> >   HCI Event: LE Meta Event (0x3e) plen 7
> >        LE Connected Isochronous Stream Request (0x1a)
> > ...
> > < HCI Command: LE Accept Co.. (0x08|0x0066) plen 2
> > ...
> >> HCI Event: LE Meta Event (0x3e) plen 29
> >        LE Connected Isochronous Stream Established (0x19)
> > ...
> > < HCI Command: LE Setup Is.. (0x08|0x006e) plen 13
> > ...
> >> HCI Event: Command Complete (0x0e) plen 6
> >        LE Setup Isochronous Data Path (0x08|0x006e) ncmd 1
> >          Status: Success (0x00)
> >          Handle: 257
> > < HCI Command: LE Setup Is.. (0x08|0x006e) plen 13
> > ...
> >> HCI Event: Command Complete (0x0e) plen 6
> >        LE Setup Isochronous Data Path (0x08|0x006e) ncmd 1
> >          Status: Success (0x00)
> >          Handle: 257
> >
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This patch landed recently in linux-next as commit 26afbd826ee3
> ("Bluetooth: Add initial implementation of CIS connections").
> Unfortunately it causes a regression on my test systems. On almost all
> I've observed that calling a simple 'hcitool scan' command in a shell
> fails in an unexpected way:
>
> $ hcitool scan
> *** stack smashing detected ***: <unknown> terminated
> Aborted

Not really sure how it would be related to ISO changes though, have
you even enabled ISO sockets UUID? Perhaps check if there is something
on dmesg indicating what is going on since I tried here and that
doesn't seem to cause any problem:

tools/hcitool scan
Scanning ...

Perhaps it is a combination of using old userspace tools with new
kernel, but then again these changes should affect something like
hcitool.

> $
>
> The above 'stack smashing' issue I've observed on Raspberry Pi4 with ARM
> 32bit kernel.
>
> Bisecting this lead me to this commit. Reverting it on top of
> next-20220808, together with the following commits due to dependencies,
> fixes the issue:
>
> 26afbd826ee326e63a334c37fd45e82e50a615ec "Bluetooth: Add initial
> implementation of CIS connections"
> e78165d2ce798451d08ac09d704cd72caa962b81 "Bluetooth: hci_conn: Fix
> updating ISO QoS PHY"
> ccf74f2390d60a2f9a75ef496d2564abb478f46a "Bluetooth: Add BTPROTO_ISO
> socket type"
> f764a6c2c1e446f560faa3232271a0637369170b "Bluetooth: ISO: Add broadcast
> support"
> c85008a4e748051cddc6be6333f55df476f35362 "Bluetooth: ISO: unlock on
> error path in iso_sock_setsockopt()"
> 82e07ef4b474f466b4b2955bad76c52d6df98a30 "Bluetooth: ISO: Fix info leak
> in iso_sock_getsockopt()"
> c5d36b8a7dbdcf41da7316252b717c60c409992b "Bluetooth: ISO: Fix memory
> corruption"
> 8a3fd9bb4fac67ad5d44d6540c7ac20004767076 "Bluetooth: ISO: Fix
> iso_sock_getsockopt for BT_DEFER_SETUP"
> aac59090a068139550f9b73d46d1116e31cdd2d1 "Bluetooth: ISO: Fix not using
> the correct QoS"
> eca0ae4aea66914515e5e3098ea051b518ee5316 "Bluetooth: Add initial
> implementation of BIS connections"
>
> Let me know if I can help fixing this issue.
>
> > ---
> >   include/net/bluetooth/bluetooth.h |  33 ++-
> >   include/net/bluetooth/hci.h       |  28 +-
> >   include/net/bluetooth/hci_core.h  | 107 +++++++-
> >   include/net/bluetooth/hci_sock.h  |   2 +
> >   include/net/bluetooth/hci_sync.h  |   3 +
> >   net/bluetooth/Kconfig             |   1 +
> >   net/bluetooth/hci_conn.c          | 440 ++++++++++++++++++++++++++++++
> >   net/bluetooth/hci_core.c          | 230 ++++++++++++----
> >   net/bluetooth/hci_event.c         | 307 ++++++++++++++++++++-
> >   net/bluetooth/hci_sync.c          |  49 +++-
> >   10 files changed, 1145 insertions(+), 55 deletions(-)
> >
> > ...
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>


-- 
Luiz Augusto von Dentz