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;
Message-ID: <749ed578-bc9d-b699-3602-bee444fff9bb@samsung.com>
Date:   Tue, 9 Aug 2022 10:54:59 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0)
        Gecko/20100101 Thunderbird/91.12.0
Subject: Re: [PATCH v7 2/7] Bluetooth: Add initial implementation of CIS
 connections
Content-Language: en-US
To:     Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
        linux-bluetooth@vger.kernel.org
From:   Marek Szyprowski <m.szyprowski@samsung.com>
In-Reply-To: <20220711233535.3134546-2-luiz.dentz@gmail.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <20220711233535.3134546-1-luiz.dentz@gmail.com>
        <20220711233535.3134546-2-luiz.dentz@gmail.com>
        <CGME20220809085500eucas1p18cb2752164ca55832787e7d3843f48ab@eucas1p1.samsung.com>
Precedence: bulk

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

$

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