| Message ID | 20230128005150.never.909-kees@kernel.org (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Bluetooth: hci_conn: Refactor hci_bind_bis() since it always succeeds | expand |
On Fri, Jan 27, 2023 at 04:51:54PM -0800, Kees Cook wrote: > The compiler thinks "conn" might be NULL after a call to hci_bind_bis(), > which cannot happen. Avoid any confusion by just making it not return a > value since it cannot fail. Fixes the warnings seen with GCC 13: > > In function 'arch_atomic_dec_and_test', > inlined from 'atomic_dec_and_test' at ../include/linux/atomic/atomic-instrumented.h:576:9, > inlined from 'hci_conn_drop' at ../include/net/bluetooth/hci_core.h:1391:6, > inlined from 'hci_connect_bis' at ../net/bluetooth/hci_conn.c:2124:3: > ../arch/x86/include/asm/rmwcc.h:37:9: warning: array subscript 0 is outside array bounds of 'atomic_t[0]' [-Warray-bounds=] > 37 | asm volatile (fullop CC_SET(cc) \ > | ^~~ > ... > In function 'hci_connect_bis': > cc1: note: source object is likely at address zero > > Fixes: eca0ae4aea66 ("Bluetooth: Add initial implementation of BIS connections") ... > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > net/bluetooth/hci_conn.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) Is this really a 'fix' ? In any case, the change looks good to me. Reviewed-by: Simon Horman <simon.horman@corigine.com>
Hello: This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Fri, 27 Jan 2023 16:51:54 -0800 you wrote: > The compiler thinks "conn" might be NULL after a call to hci_bind_bis(), > which cannot happen. Avoid any confusion by just making it not return a > value since it cannot fail. Fixes the warnings seen with GCC 13: > > In function 'arch_atomic_dec_and_test', > inlined from 'atomic_dec_and_test' at ../include/linux/atomic/atomic-instrumented.h:576:9, > inlined from 'hci_conn_drop' at ../include/net/bluetooth/hci_core.h:1391:6, > inlined from 'hci_connect_bis' at ../net/bluetooth/hci_conn.c:2124:3: > ../arch/x86/include/asm/rmwcc.h:37:9: warning: array subscript 0 is outside array bounds of 'atomic_t[0]' [-Warray-bounds=] > 37 | asm volatile (fullop CC_SET(cc) \ > | ^~~ > ... > In function 'hci_connect_bis': > cc1: note: source object is likely at address zero > > [...] Here is the summary with links: - Bluetooth: hci_conn: Refactor hci_bind_bis() since it always succeeds https://git.kernel.org/bluetooth/bluetooth-next/c/d57d873e6851 You are awesome, thank you!
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index acf563fbdfd9..61a34801e61e 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1981,16 +1981,14 @@ static void hci_iso_qos_setup(struct hci_dev *hdev, struct hci_conn *conn, qos->latency = conn->le_conn_latency; } -static struct hci_conn *hci_bind_bis(struct hci_conn *conn, - struct bt_iso_qos *qos) +static void hci_bind_bis(struct hci_conn *conn, + struct bt_iso_qos *qos) { /* Update LINK PHYs according to QoS preference */ conn->le_tx_phy = qos->out.phy; conn->le_tx_phy = qos->out.phy; conn->iso_qos = *qos; conn->state = BT_BOUND; - - return conn; } static int create_big_sync(struct hci_dev *hdev, void *data) @@ -2119,11 +2117,7 @@ struct hci_conn *hci_connect_bis(struct hci_dev *hdev, bdaddr_t *dst, if (IS_ERR(conn)) return conn; - conn = hci_bind_bis(conn, qos); - if (!conn) { - hci_conn_drop(conn); - return ERR_PTR(-ENOMEM); - } + hci_bind_bis(conn, qos); /* Add Basic Announcement into Peridic Adv Data if BASE is set */ if (base_len && base) {
The compiler thinks "conn" might be NULL after a call to hci_bind_bis(), which cannot happen. Avoid any confusion by just making it not return a value since it cannot fail. Fixes the warnings seen with GCC 13: In function 'arch_atomic_dec_and_test', inlined from 'atomic_dec_and_test' at ../include/linux/atomic/atomic-instrumented.h:576:9, inlined from 'hci_conn_drop' at ../include/net/bluetooth/hci_core.h:1391:6, inlined from 'hci_connect_bis' at ../net/bluetooth/hci_conn.c:2124:3: ../arch/x86/include/asm/rmwcc.h:37:9: warning: array subscript 0 is outside array bounds of 'atomic_t[0]' [-Warray-bounds=] 37 | asm volatile (fullop CC_SET(cc) \ | ^~~ ... In function 'hci_connect_bis': cc1: note: source object is likely at address zero Fixes: eca0ae4aea66 ("Bluetooth: Add initial implementation of BIS connections") Cc: Luiz Augusto von Dentz <luiz.von.dentz@intel.com> Cc: Marcel Holtmann <marcel@holtmann.org> Cc: Johan Hedberg <johan.hedberg@gmail.com> Cc: "David S. Miller" <davem@davemloft.net> Cc: Eric Dumazet <edumazet@google.com> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Paolo Abeni <pabeni@redhat.com> Cc: linux-bluetooth@vger.kernel.org Cc: netdev@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- net/bluetooth/hci_conn.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)