From patchwork Mon Sep 4 22:11:52 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Olivier L'Heureux X-Patchwork-Id: 13374304 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 75A60C71153 for ; Mon, 4 Sep 2023 22:12:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239388AbjIDWMI (ORCPT ); Mon, 4 Sep 2023 18:12:08 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37302 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229561AbjIDWMH (ORCPT ); Mon, 4 Sep 2023 18:12:07 -0400 Received: from mail-ej1-x631.google.com (mail-ej1-x631.google.com [IPv6:2a00:1450:4864:20::631]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 78C34A9 for ; Mon, 4 Sep 2023 15:12:02 -0700 (PDT) Received: by mail-ej1-x631.google.com with SMTP id a640c23a62f3a-99357737980so280984866b.2 for ; Mon, 04 Sep 2023 15:12:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mind.be; s=google; t=1693865521; x=1694470321; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=5FkFqWTL0Jd47n0sP0wy9R7v8p0d/aInwEtv/pW4v3Q=; b=DD/ClJZflDtp/2f8S/HrpUx7xSJIP8iOqQZ0wvwKK4kGbT6lS536InwVACFNssbD7o CIkQq8f4AWb0RDE9gobFDFlmTOMCdI95kwl8gXKt9lkDnuIZQkFlwqqSSNjbI0qccJCa hhOwsSxrOWB/zdaFS/E9RkL3mTiA3taQXjI3223ga2fJAoLDt2HFsgSYXqilErSgnPsY Tzut+/4McI7oLdLMSKBO2d+smJkPGl9RFUlLFAbHBd+SUt8ij1EUD2ABrSUqlab9l9/p fJIBmaIlYbU2thut2GkTlDSdq0v92Z4cjvZ2e56wi+p+JSjx1v5hKldHvyVEIBUZWboV mBjg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693865521; x=1694470321; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=5FkFqWTL0Jd47n0sP0wy9R7v8p0d/aInwEtv/pW4v3Q=; b=Ui5GgWYg330hYrWqOn8OwqIef4pff2Pk/RAgm5CG/Q+uE8HQLjUhLYjdiSAOx58ny9 DJKyw2GiMefipmqRJLvIKfqTYU+vvx9R1OzjRPKivpJPSKuaZB8fLw3dZ4oFkCoV691/ IwYC6/OghPjNZ8aOjq5Pr8YgXP6c0YfpKB6Gq+a+2DhmGAir0a68X1hVHPmW+tKlhsDA bzgVr91MmvHwQhFIFi9uo8AXrNVA3OTfQeCOU4qVJ0yXtcP6mi5s2K0VL4edapORoKby 3j7ifjIY9IuELCUC0wZdaUTQkssS8WdM9QbugKrzsJO7Es8/p8fV01goJCQC/GFqZxEF RGaw== X-Gm-Message-State: AOJu0YxmGrVMl6oVAyTe4e6moOubnRn9o4HKjhi52rGs6OW1CiaiJjDf ExsTVqJoYC+4UYnHlQeyKCWZAg== X-Google-Smtp-Source: AGHT+IGEnrtzRzPNnM00RY/+EBucUSgO/6PCzaxj80w8lTw5RfqEAF0mUsjC+sJHi2MdYAf8JlkcpA== X-Received: by 2002:a17:906:519d:b0:9a2:96d2:b1e8 with SMTP id y29-20020a170906519d00b009a296d2b1e8mr8404425ejk.54.1693865520950; Mon, 04 Sep 2023 15:12:00 -0700 (PDT) Received: from olheureu-ThinkPad-E560.local.ess-mail.com ([2a02:578:85b9:1300:6c89:e61f:b837:7d81]) by smtp.gmail.com with ESMTPSA id z16-20020a170906715000b00993cc1242d4sm6692673ejj.151.2023.09.04.15.12.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Sep 2023 15:12:00 -0700 (PDT) From: Olivier L'Heureux To: Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz Cc: linux-bluetooth@vger.kernel.org, Olivier L'Heureux Subject: [PATCH 1/7] ARM: dts: stm32: Add Bluetooth (usart2) feature on stm32mp157x Date: Tue, 5 Sep 2023 00:11:52 +0200 Message-Id: <20230904221158.35425-2-olivier.lheureux@mind.be> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230904221158.35425-1-olivier.lheureux@mind.be> References: <20230904221158.35425-1-olivier.lheureux@mind.be> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org From: Christophe Roullier Add BT (usart2) config on stm32mp157x. Signed-off-by: Christophe Roullier Change-Id: I41cc7d60900e05d8bd4e3281abe170ef3fbbaee8 Reviewed-on: https://gerrit.st.com/c/mpu/oe/st/linux-stm32/+/237115 Reviewed-by: CITOOLS Reviewed-by: Eric FOURMONT Tested-by: Eric FOURMONT Cherry-picked for v5.13: dropped part of patch for absent "stm32mp157f-dk2.dts". --- arch/arm/boot/dts/st/stm32mp157c-dk2.dts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/st/stm32mp157c-dk2.dts b/arch/arm/boot/dts/st/stm32mp157c-dk2.dts index 4bef2300ed7c..680391433992 100644 --- a/arch/arm/boot/dts/st/stm32mp157c-dk2.dts +++ b/arch/arm/boot/dts/st/stm32mp157c-dk2.dts @@ -102,5 +102,14 @@ &usart2 { pinctrl-0 = <&usart2_pins_c>; pinctrl-1 = <&usart2_sleep_pins_c>; pinctrl-2 = <&usart2_idle_pins_c>; - status = "disabled"; + uart-has-rtscts; + status = "okay"; + + bluetooth { + shutdown-gpios = <&gpioz 6 GPIO_ACTIVE_HIGH>; + compatible = "brcm,bcm43438-bt"; + max-speed = <3000000>; + vbat-supply = <&v3v3>; + vddio-supply = <&v3v3>; + }; }; From patchwork Mon Sep 4 22:11:53 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Olivier L'Heureux X-Patchwork-Id: 13374306 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0C39FC83F3E for ; Mon, 4 Sep 2023 22:12:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240564AbjIDWMK (ORCPT ); Mon, 4 Sep 2023 18:12:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37312 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240388AbjIDWMI (ORCPT ); Mon, 4 Sep 2023 18:12:08 -0400 Received: from mail-lj1-x22a.google.com (mail-lj1-x22a.google.com [IPv6:2a00:1450:4864:20::22a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 427BDCCB for ; Mon, 4 Sep 2023 15:12:03 -0700 (PDT) Received: by mail-lj1-x22a.google.com with SMTP id 38308e7fff4ca-2b9338e4695so28114111fa.2 for ; Mon, 04 Sep 2023 15:12:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mind.be; s=google; t=1693865521; x=1694470321; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=ozfJRQtpgwFC9ZhqHlBlU2b3MtVkpPgDvpRNwWgjyLY=; b=KjwtFLj6cgNExLJWiGhEKSEZiUmC9b/QwK3MV9uP+9GiNk2nn4OyjUPGtvti824SWP sOC3B/0Kzgb9jOw/iYJsYwnGzv3vs/xmifRy2Djw5pxPGZNVBewFDIxVTlg6b0yhf3UD K8GIx1ceEv2d8T0qvBqKprETgMdk/myDBhVOeqz0a/5POfjHV71HwwKGlNurEvLYgunm efAzf4zJbzqikK/v0ganM72dqjOF5TS4+CJp1XUxXBYaQSjqMsmg8UnfriqpmLoPWmEX TwPMjaxhrEXiAt4SiB8MUphDhoUs861LRKEVibRxu2iW1eRrKXfddBF5e2qCbDuR26QM lVWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693865521; x=1694470321; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ozfJRQtpgwFC9ZhqHlBlU2b3MtVkpPgDvpRNwWgjyLY=; b=B5jjj8iykSxgrr2eD3T91uRsTz2ERhSREFO/jQjId9hpeNNj265QAvEakhmuUZAX8s BIVXneUNJ6hgvXBACPySD2XikkNsBTNik2iBu4lRwnEO4MtteVzPmMJau/ffBBoRN/04 QsDvmjhadkUUhaDvYgN/x4M7rs/rGuAxB2z7SrLJ53TYPnem2TOBV1+wiSLfHL4XNBUe Ny2k3PmB5dlWnVxHM5KJGhX3F9VpdYQ9ecuuODrxEp6XJbq9ja1NvF0IowmZGK/A8iWU B2NK6/sgxtsmabUh0gzAsSBJiVeIQiViI3iyoOlnAy8Jm9KpS+Sw0yRao/MuWaMWxoEe Ozkg== X-Gm-Message-State: AOJu0YwVVbfALqnbzZCcjhfhWtOsJZJjqSMCAeyF4xGkCKo50d4s6MBH W8wKEBMr++ZFBegEKWPdd4qT5Q== X-Google-Smtp-Source: AGHT+IE/vvRh/sSr63twRD+V6ZY16R6C3ID54jH2VOvEZ+HwDyYucHCTDn7JCaPBvJAJ8vwK5JTOTA== X-Received: by 2002:a2e:9c1a:0:b0:2b6:e2c1:6cda with SMTP id s26-20020a2e9c1a000000b002b6e2c16cdamr8029681lji.46.1693865521537; Mon, 04 Sep 2023 15:12:01 -0700 (PDT) Received: from olheureu-ThinkPad-E560.local.ess-mail.com ([2a02:578:85b9:1300:6c89:e61f:b837:7d81]) by smtp.gmail.com with ESMTPSA id z16-20020a170906715000b00993cc1242d4sm6692673ejj.151.2023.09.04.15.12.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Sep 2023 15:12:01 -0700 (PDT) From: Olivier L'Heureux To: Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz Cc: linux-bluetooth@vger.kernel.org, Olivier L'Heureux Subject: [PATCH 2/7] Bluetooth: add many traces for allocation/free/refcounting Date: Tue, 5 Sep 2023 00:11:53 +0200 Message-Id: <20230904221158.35425-3-olivier.lheureux@mind.be> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230904221158.35425-1-olivier.lheureux@mind.be> References: <20230904221158.35425-1-olivier.lheureux@mind.be> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org When debugging a kernel memory leak occuring when a bluetooth device scans for a specific bluetooth device but can't find it [1], we suspect the following structures are never freed, causing memory leaks: 1. the "struct hci_conn" allocated with "kzalloc()" in "hci_conn_add()" 2. the "struct l2cap_conn" allocated with "kzalloc()" in "l2cap_conn_add()" The "struct hci_conn" are reference-counted with 2 reference counters: 1. "conn->dev" is a usual "struct device*" and it is used for the struct lifetime, with "hci_conn_get()" and "hci_conn_put()". 2. "conn->refcnt" is a "atomic_t" that is used to control the underlying connections, with "hci_conn_hold()" and "hci_conn_drop()". The "struct l2cap_conn" are reference-counted with a "struct kref ref" member. To debug [2], we add here some debug traces in the following functions: 1. "hci_conn_get()" and "hci_conn_put()", to trace the refcounts, 2. "hci_conn_hold()" and "hci_conn_drop()", to trace the other refcounts, 3. "hci_conn_add()", "bt_link_release()", to trace "struct hci_conn" allocation/free, 4. "hci_conn_del_sysfs()" and "hci_conn_cleanup()", to better follow, 4. "l2cap_conn_get()" and "l2cap_conn_put()", to trace the refcounts, 5. "l2cap_conn_add()" and "l2cap_conn_free()", to trace "struct l2cap_conn" allocation/free. 6. "l2cap_conn_add()", in case it does nothing and reuse the existing "conn". We explicitly mention the "kzalloc()" and "kfree()" in the debugging messages, to ease the allocation and free matching. References: - [1] "ble-memleak-repro" - [2] "Dynamic debug" Signed-off-by: Olivier L'Heureux Signed-off-by: Olivier L'Heureux --- include/net/bluetooth/hci_core.h | 6 ++++-- net/bluetooth/hci_conn.c | 4 ++++ net/bluetooth/hci_sysfs.c | 6 ++++++ net/bluetooth/l2cap_core.c | 12 ++++++++++-- 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index e01d52cb668c..d8badb2a28cd 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1393,18 +1393,20 @@ void hci_conn_failed(struct hci_conn *conn, u8 status); static inline struct hci_conn *hci_conn_get(struct hci_conn *conn) { + BT_DBG("hcon %p orig kobj.refcount %d", conn, kref_read(&conn->dev.kobj.kref)); get_device(&conn->dev); return conn; } static inline void hci_conn_put(struct hci_conn *conn) { + BT_DBG("hcon %p orig kobj.refcount %d", conn, kref_read(&conn->dev.kobj.kref)); put_device(&conn->dev); } static inline struct hci_conn *hci_conn_hold(struct hci_conn *conn) { - BT_DBG("hcon %p orig refcnt %d", conn, atomic_read(&conn->refcnt)); + BT_DBG("hcon %p orig conn->refcnt %d", conn, atomic_read(&conn->refcnt)); atomic_inc(&conn->refcnt); cancel_delayed_work(&conn->disc_work); @@ -1414,7 +1416,7 @@ static inline struct hci_conn *hci_conn_hold(struct hci_conn *conn) static inline void hci_conn_drop(struct hci_conn *conn) { - BT_DBG("hcon %p orig refcnt %d", conn, atomic_read(&conn->refcnt)); + BT_DBG("hcon %p orig conn->refcnt %d", conn, atomic_read(&conn->refcnt)); if (atomic_dec_and_test(&conn->refcnt)) { unsigned long timeo; diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 76222565e2df..23e635600717 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -143,6 +143,8 @@ static void hci_conn_cleanup(struct hci_conn *conn) { struct hci_dev *hdev = conn->hdev; + BT_DBG("%s hcon %p", hdev->name, conn); + if (test_bit(HCI_CONN_PARAM_REMOVAL_PEND, &conn->flags)) hci_conn_params_del(conn->hdev, &conn->dst, conn->dst_type); @@ -994,6 +996,8 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst, if (!conn) return NULL; + BT_DBG("hcon %p=kzalloc()", conn); + bacpy(&conn->dst, dst); bacpy(&conn->src, &hdev->bdaddr); conn->handle = HCI_CONN_HANDLE_UNSET; diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c index 15b33579007c..fc297b651881 100644 --- a/net/bluetooth/hci_sysfs.c +++ b/net/bluetooth/hci_sysfs.c @@ -13,6 +13,9 @@ static const struct class bt_class = { static void bt_link_release(struct device *dev) { struct hci_conn *conn = to_hci_conn(dev); + + BT_DBG("kfree(conn %p)", conn); + kfree(conn); } @@ -67,6 +70,8 @@ void hci_conn_del_sysfs(struct hci_conn *conn) { struct hci_dev *hdev = conn->hdev; + BT_DBG("conn %p", conn); + if (!device_is_registered(&conn->dev)) return; @@ -80,6 +85,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn) put_device(dev); } + BT_DBG("calling device_del %p", conn); device_del(&conn->dev); hci_dev_put(hdev); diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 17ca13e8c044..c749b434df97 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -1945,12 +1945,16 @@ static void l2cap_conn_free(struct kref *ref) { struct l2cap_conn *conn = container_of(ref, struct l2cap_conn, ref); + BT_DBG("kfree(conn) %p", conn); + hci_conn_put(conn->hcon); kfree(conn); } struct l2cap_conn *l2cap_conn_get(struct l2cap_conn *conn) { + BT_DBG("conn %p, refcount %d", conn, kref_read(&conn->ref)); + kref_get(&conn->ref); return conn; } @@ -1958,6 +1962,8 @@ EXPORT_SYMBOL(l2cap_conn_get); void l2cap_conn_put(struct l2cap_conn *conn) { + BT_DBG("conn %p, refcount %d", conn, kref_read(&conn->ref)); + kref_put(&conn->ref, l2cap_conn_free); } EXPORT_SYMBOL(l2cap_conn_put); @@ -7835,8 +7841,10 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon) struct l2cap_conn *conn = hcon->l2cap_data; struct hci_chan *hchan; - if (conn) + if (conn) { + BT_DBG("hcon %p reuse conn %p", hcon, conn); return conn; + } hchan = hci_chan_create(hcon); if (!hchan) @@ -7853,7 +7861,7 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon) conn->hcon = hci_conn_get(hcon); conn->hchan = hchan; - BT_DBG("hcon %p conn %p hchan %p", hcon, conn, hchan); + BT_DBG("hcon %p conn %p=kzalloc() hchan %p", hcon, conn, hchan); switch (hcon->type) { case LE_LINK: From patchwork Mon Sep 4 22:11:54 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Olivier L'Heureux X-Patchwork-Id: 13374305 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BA934C83F33 for ; Mon, 4 Sep 2023 22:12:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240542AbjIDWMJ (ORCPT ); Mon, 4 Sep 2023 18:12:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37326 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229561AbjIDWMI (ORCPT ); Mon, 4 Sep 2023 18:12:08 -0400 Received: from mail-ej1-x632.google.com (mail-ej1-x632.google.com [IPv6:2a00:1450:4864:20::632]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CACB8CDA for ; Mon, 4 Sep 2023 15:12:03 -0700 (PDT) Received: by mail-ej1-x632.google.com with SMTP id a640c23a62f3a-9a1de3417acso593221266b.0 for ; Mon, 04 Sep 2023 15:12:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mind.be; s=google; t=1693865522; x=1694470322; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=gArWs5Q5c1PsBa7PCApkxjz+wZ4/Au6Fx4OqD6X/u4U=; b=gote8BAH1dfry79ayUQUZp6/WJMCaNkkdJXsd4qoWFaWWiTsgylOK/iI+5K0apMzky HqNaG3WPM425tZJGFaqVr5iBI/BGxc3Z0XKPSvGyDnjQ3aZ5Oh/7sVQMpnEaFA+ffsSO WPF/HRzar9GFtOsecuU4+NwGt0I6MH+6770iZYvCLeTZtri9fO+OY0EzpMOVylFL5hRK a1ipqBmGro2yZBzAOLgCxRhvisICF1apxxF7q2sZ0UXtc1RUxbsPepKsrTFvZ2KHahMT jbhRkFHQlGCChNqQrw2gMs0RUBdkdVc3aM1I5TFNNDYti9rSXooiaLBHu38bA38L/Ykb pj/w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693865522; x=1694470322; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=gArWs5Q5c1PsBa7PCApkxjz+wZ4/Au6Fx4OqD6X/u4U=; b=USaRazkNKxcl6vgoxyYNs5rpy3ceJPfInlq8HjVQD4O0GzVqmP3yPrfylHmkHkStzC cWQkJ+cmPM7pzfQbrwYd69TCrXJZayRp7tA1ZCXVbKIUIjr+z8wpRSmYhO4effDPb1DL 2dNTwXV4rNRRcvnZggVh7VJ17e4csebYzh95UW8n+6/NaOb4I9vL+y/EY7WriqEbf/Gt r9LRhQQ2TYd04cKxP3/f7vx/QEFiXwnLYDohl2wF+h9wHf/Byo2htFGCiKfGnkgjcG0z B3GtAJ4miB0S6kjwYPxqETZCg/plIJbBaqOVMSIZjKIxai5Tt1MYKc/XzQvxd9iwIaLU DWmw== X-Gm-Message-State: AOJu0YxmQ8aktHSMCjGK0y/b4f54GMtb8PzAD7hcb49fc67JWOAiX6sU PTB4J7BhIljwi0vkPGTyQwck0w== X-Google-Smtp-Source: AGHT+IFaVoPIsdEXXEJGSK0ibzOr6EUvtMfDzXMWWIs50iC+huwvFJoVXIvugNjIAkxZWWVCqFSLpQ== X-Received: by 2002:a17:906:3010:b0:9a5:bceb:1cf8 with SMTP id 16-20020a170906301000b009a5bceb1cf8mr11138985ejz.3.1693865522265; Mon, 04 Sep 2023 15:12:02 -0700 (PDT) Received: from olheureu-ThinkPad-E560.local.ess-mail.com ([2a02:578:85b9:1300:6c89:e61f:b837:7d81]) by smtp.gmail.com with ESMTPSA id z16-20020a170906715000b00993cc1242d4sm6692673ejj.151.2023.09.04.15.12.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Sep 2023 15:12:01 -0700 (PDT) From: Olivier L'Heureux To: Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz Cc: linux-bluetooth@vger.kernel.org, Olivier L'Heureux Subject: [PATCH 3/7] Bluetooth: L2CAP: use refcount on struct l2cap_chan->conn Date: Tue, 5 Sep 2023 00:11:54 +0200 Message-Id: <20230904221158.35425-4-olivier.lheureux@mind.be> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230904221158.35425-1-olivier.lheureux@mind.be> References: <20230904221158.35425-1-olivier.lheureux@mind.be> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org We have detected a memory leak of "struct l2cap_conn" objects, and of "struct hci_conn" in the bluetooth subsystem [12]. The second is a consequence of the first. This memory leak was also detected by syzbot [13]. "struct l2cap_conn" is allocated in "l2cap_conn_add()" [2]. "struct l2cap_conn" is reference-counted [1] by its "ref" member [3]: every holder of a reference to it should increment the reference counter by calling "l2cap_conn_get()" [4], and every reference holder that stops keeping the reference should call "l2cap_conn_put()" [5]. The "struct l2cap_conn" is freed when the counter reaches 0, meaning there is no reference holder any more. This mechanism is already used by the "hidp_session_new()" [6] and "session_free()" [7] functions that create and delete the "struct hidp_session" [8] objects, which contains a "conn" reference to a "struct l2cap_conn". The same reference counting mechanism is not used for the "conn" reference kept in the "struct l2cap_chan" [9] object. There were two places where the reference counting was not applied: 1. In "__l2cap_chan_add()" [11], the "struct l2cap_conn" reference is stored in the "conn" member of the "struct l2cap_chan" [9]. 2. In "l2cap_chan_del()" [10], the "conn" member of the "struct l2cap_chan" is set to NULL. To apply the reference counting to the "conn" member of the "struct l2cap_chan", we use "l2cap_conn_get()" in "__l2cap_chan_add()", where we store the reference, and "l2cap_conn_put()" in "l2cap_chan_del()", where we drop the reference. Handling the "conn" member of the "struct l2cap_chan" with the existing reference counter will help to fix the kernel memory leak in a following commit. References: - [1] "Adding reference counters (krefs) to kernel objects" - [2] "l2cap_conn_add()" - [3] "struct l2cap_conn" - [4] "l2cap_conn_get()" - [5] "l2cap_conn_put()" - [6] "hidp_session_new()" - [7] "session_free()" - [8] "struct hidp_session" - [9] "struct l2cap_chan" - [10] "l2cap_chan_del()" - [11] "__l2cap_chan_add()" - [12] "ble-memleak-repro" - [13] "[syzbot] [bluetooth?] memory leak in hci_conn_add (2)" Signed-off-by: Olivier L'Heureux Signed-off-by: Olivier L'Heureux --- net/bluetooth/l2cap_core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index c749b434df97..768632fba6f8 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -587,7 +587,7 @@ void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) conn->disc_reason = HCI_ERROR_REMOTE_USER_TERM; - chan->conn = conn; + chan->conn = l2cap_conn_get(conn); switch (chan->chan_type) { case L2CAP_CHAN_CONN_ORIENTED: @@ -669,6 +669,8 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err) if (mgr && mgr->bredr_chan == chan) mgr->bredr_chan = NULL; + + l2cap_conn_put(conn); } if (chan->hs_hchan) { From patchwork Mon Sep 4 22:11:55 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Olivier L'Heureux X-Patchwork-Id: 13374308 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 21638CA0FF3 for ; Mon, 4 Sep 2023 22:12:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240436AbjIDWML (ORCPT ); Mon, 4 Sep 2023 18:12:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37342 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240466AbjIDWMJ (ORCPT ); Mon, 4 Sep 2023 18:12:09 -0400 Received: from mail-lj1-x22d.google.com (mail-lj1-x22d.google.com [IPv6:2a00:1450:4864:20::22d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B5A77CDB for ; Mon, 4 Sep 2023 15:12:04 -0700 (PDT) Received: by mail-lj1-x22d.google.com with SMTP id 38308e7fff4ca-2bccda76fb1so30744881fa.2 for ; Mon, 04 Sep 2023 15:12:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mind.be; s=google; t=1693865523; x=1694470323; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=yyo1tkxni+p1MNEnypGuf8h5ed39NY3A4rmpbAGxJBI=; b=alyChdPRJ1wao6+64bBNjz/Xao6bmLgyVNnDu7K9eFROo5XWu9ZL1LVBVjHZi7h6pj rnRHjVSGAOooBOssxbtfDXBN+ylDClEVFw1mKkkeAYe7Ag1UcvUzo7gZuqpMWFWXXeUx Ne+UIoFcJtjAVPqm1HWA/QHs7Ia+oYGqS4mbolnFip8SrJN0bjFxmWlZIQvWdhkPqL0s jvWR5qLCpJogfnGqYbMTzeHljB8eUSgzfvY5PamLuE7W1IoX7UBHDb0lDn2GVawxk4bT P0ETcgrlCTDGyhNRL8ipiQXUAQ6JqLfgs5vPoT37mlFll2Ei4qsqbNJxjekmP8g8xgCb 3U9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693865523; x=1694470323; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=yyo1tkxni+p1MNEnypGuf8h5ed39NY3A4rmpbAGxJBI=; b=X/BXVPvL2GmtYgEBVZryeRlrJYNHYZNJyWnUPqSHiVZ7JVfeq3b0Z9PAp5YykBE8dd 0P2tBwm9oEyRv8iYZtFNBYrmrNzv2rd2u0vOpsxC/0ITjjH9f0+fNeUOE95ea6xptJCi lfPpV6cUKuoICqr3o4mKLxn27ns9I6L7SYjRi4y7jgyGwm+l6UVbaOUQkyHh+HmdbJ1t LfDHOiNyohsIOAxuHgyWnf7G0fzHAhWxZAj97nGRKgxX2IlQooxnImAMZInmZ5QzQ9NJ H9U8/lN2OwpYavkLW4VXxCSCSMKdM858MnGpmPWp4y0suKaFkhDgjp1o9JMfv2KcJyh8 xQvA== X-Gm-Message-State: AOJu0YyUZneXNwNgcthgw3Bx78JV5a8OzdRjOOkjWE2p5imjmytrhuem g61BR1p+0xN8V4jiMX/hMlCbfg== X-Google-Smtp-Source: AGHT+IH+eOSYCSlRmapK5DCQ7GhChDWqG4fs55gtG1thCCBWNdSeoTvB9JNSu1aKeqFpPG/Ql3M7mA== X-Received: by 2002:a2e:360e:0:b0:2bc:c28c:a2b5 with SMTP id d14-20020a2e360e000000b002bcc28ca2b5mr6889476lja.51.1693865522955; Mon, 04 Sep 2023 15:12:02 -0700 (PDT) Received: from olheureu-ThinkPad-E560.local.ess-mail.com ([2a02:578:85b9:1300:6c89:e61f:b837:7d81]) by smtp.gmail.com with ESMTPSA id z16-20020a170906715000b00993cc1242d4sm6692673ejj.151.2023.09.04.15.12.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Sep 2023 15:12:02 -0700 (PDT) From: Olivier L'Heureux To: Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz Cc: linux-bluetooth@vger.kernel.org, Olivier L'Heureux Subject: [PATCH 4/7] Bluetooth: L2CAP: free the leaking struct l2cap_conn Date: Tue, 5 Sep 2023 00:11:55 +0200 Message-Id: <20230904221158.35425-5-olivier.lheureux@mind.be> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230904221158.35425-1-olivier.lheureux@mind.be> References: <20230904221158.35425-1-olivier.lheureux@mind.be> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org There is a leak of "struct l2cap_conn" [2] objects, and of "struct hci_conn" objects, when a user-mode application scan for a BLE MAC address, with timeouts [7] [8]. The "struct hci_conn" leak is a consequence of the first, and we prove it below. The "struct l2cap_conn" memory leak comes from the "l2cap_chan_connect()" function. 1. "l2cap_chan_connect()" [3] calls "l2cap_conn_add(hcon)", which allocates a "struct l2cap_conn", sets is reference counter [1] to 1 and returns it. The "struct l2cap_conn*" is stored in "conn". 2. "l2cap_chan_connect()" calls "__l2cap_chan_add(conn, chan)" [4], which stores "conn" in the "conn" member of "chan": chan->conn = conn; Since a recent commit [6], "__l2cap_chan_add()" increases the "conn" reference counter to 2. 3. Nothing frees the allocated "struct l2cap_conn". The channel is eventually deleted by "l2cap_chan_del()" [5], which sets "chan->conn" to NULL and, since a recent commit [6], decreases "conn"'s reference counter back to 1. A recent commit [6] has added reference counting to "chan->conn": the counter is incremented in "__l2cap_chan_add(conn, chan)", when it is assigned to the "chan->conn" member, and the counter is decremented in "l2cap_chan_del()", when the "chan->conn" member is reset to NULL. The reference counting should free the "struct l2cap_conn", if correctly used. The memory allocation error in the current "l2cap_chan_connect()" comes from a lack of transfer of "struct l2cap_conn" ownership to the "chan" object. When "l2cap_chan_connect()" calls "l2cap_conn_add(hcon)", because the latter allocates a "struct l2cap_conn", the caller receives the responsibility to free it, we call this "ownership". Because "struct l2cap_conn" is reference counted, "l2cap_chan_connect()" should either call "l2cap_conn_put()", or transfer the ownership to a function or object. Calling "__l2cap_chan_add(conn, chan)" did not, but should transfer the responsibility to free "conn" to the "chan" channel. The channel can free "conn" because it stores a reference to it. This commit fixes the memory leak because it transfers the ownership from "l2cap_chan_connect()"'s local variable "conn" to the "conn" member of the "chan" channel. The ownership transfer is done by decrementing the reference counter in "l2cap_chan_connect()": "l2cap_chan_connect()" states so it does not use the "conn" reference any more and does not keep any reference to the "struct l2cap_conn". In the no-error case, "l2cap_chan_connect()" has called "__l2cap_chan_add()", which has taken its own reference and increased the ref-counter. "l2cap_chan_del()" will eventually drop the channel's reference and decrement the ref-counter. The reference count will eventually reach 0, "l2cap_conn_free()" will be called and it will free the "struct l2cap_conn". If an error happened between the memory allocation and "__l2cap_chan_add(conn, chan)", we need to free "conn", which happens because the "l2cap_conn_put(conn)" is after the "chan_unlock:" and "release_conn:" labels. Fixing the "struct l2cap_conn" memory leak fixes the "struct hci_conn" memory leak. Indeed, 1. "hci_conn_add()" allocates a "struct hci_conn", and it calls "hci_conn_init_sysfs()" that: 1. initialises the "conn->dev" reference counter to 1 with a "device_initialize(&conn->dev)". 2. registers the "bt_link_release()" callback that will free the "struct hci_conn", when the reference counter will reach 0. 2. Following the "<...>_add()" / "<...>_del()" convention, each call to "hci_conn_add()" should be matched by a "hci_conn_del()". However, this is not the case for us, because we only scan and our use case matches "hci_conn_del()"'s comment: /* Remove the connection from the list and cleanup its remaining * state. This is a separate function since for some cases like * BT_CONNECT_SCAN we *only* want the cleanup part without the * rest of hci_conn_del. */ hci_conn_cleanup(conn); 3. "hci_conn_cleanup()" will eventually be called, not from "hci_conn_del()" but from "le_scan_cleanup()". It will call "hci_conn_del_sysfs()" that deletes the device. 4. There are several "hci_conn_get()" with matching "hci_conn_put()": 1. "hci_chan_create()" calls "hci_conn_get()" because it sets the "conn" field of the "struct hci_chan" it is initialising. "hci_chan_create()" is matched by "hci_chan_del()", which calls "hci_conn_put()". 2. "hci_connect_le_scan_remove()" calls "hci_conn_get()". An explicit comment explains some cleaning work is deferred to the "le_scan_cleanup()" callback, which indeed calls "hci_conn_put()". 3. "l2cap_conn_add()" calls "hci_conn_get()" because it sets the "hcon" field of the "struct hci_chan" it is initialising. Following the "<...>_add()" / "<...>_del()" convention, "l2cap_conn_del()" should match "l2cap_conn_add()". In our use case, "l2cap_conn_del()" is never called, and it does not contain any "hci_conn_put()" either. The "hci_conn_put()" is deferred in the "l2cap_conn_free()" that is eventually called when the reference counter in the "struct kref ref" member of the "struct l2cap_conn" reaches 0. In our sources, "l2cap_conn_del()" is called only in "l2cap_connect_cfm()" and "l2cap_disconn_cfm()", but those callbacks are never called in our use case. References: - [1] "Adding reference counters (krefs) to kernel objects" - [2] "struct l2cap_conn" - [3] "l2cap_chan_connect()" - [4] "__l2cap_chan_add()" - [5] "l2cap_chan_del()" - [6] commit b7556839b38b ("Bluetooth: L2CAP: use refcount on struct l2cap_chan->conn") - [7] "ble-memleak-repro" - [8] "[syzbot] [bluetooth?] memory leak in hci_conn_add (2)" Signed-off-by: Olivier L'Heureux Signed-off-by: Olivier L'Heureux --- net/bluetooth/l2cap_core.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 768632fba6f8..f5dcb4a4fb15 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -8080,7 +8080,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, if (data.count > L2CAP_ECRED_CONN_SCID_MAX) { hci_conn_drop(hcon); err = -EPROTO; - goto done; + goto release_conn; } } @@ -8126,6 +8126,18 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, chan_unlock: l2cap_chan_unlock(chan); mutex_unlock(&conn->chan_lock); + +release_conn: + /* Transfer the "conn" ownership to "chan->conn". + * l2cap_conn_add() above has created "conn" with a + * ref-counter at 1. "__l2cap_chan_add()" stored a "conn" + * reference in "chan->conn" and incremented the ref-counter. + * Before "conn" goes out of scope, decrement here the "conn" + * ref-counter, so that when "l2cap_chan_del()" will + * eventually decrement the ref-counter, the "conn" will be + * freed. + */ + l2cap_conn_put(conn); done: hci_dev_unlock(hdev); hci_dev_put(hdev); From patchwork Mon Sep 4 22:11:56 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Olivier L'Heureux X-Patchwork-Id: 13374309 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9C9CEC71153 for ; Mon, 4 Sep 2023 22:12:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240466AbjIDWML (ORCPT ); Mon, 4 Sep 2023 18:12:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37350 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240531AbjIDWMJ (ORCPT ); Mon, 4 Sep 2023 18:12:09 -0400 Received: from mail-lf1-x12b.google.com (mail-lf1-x12b.google.com [IPv6:2a00:1450:4864:20::12b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 67A97A9 for ; Mon, 4 Sep 2023 15:12:05 -0700 (PDT) Received: by mail-lf1-x12b.google.com with SMTP id 2adb3069b0e04-501be2d45e0so30366e87.3 for ; Mon, 04 Sep 2023 15:12:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mind.be; s=google; t=1693865523; x=1694470323; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=wYo3dq3VK96vstXSFrFSQooJJuj3c4z/z/PjKdvJaEQ=; b=MN6ABb25xKNcMu+0lza8J4u3BdgeBRaC6sdDtvbB7FUsImIIE0/i0ViJbF+mCRdWVo Aho8Wqyu2bu84/YAxrGDoEN8bw7QvJCJff/NlIPncWrRtC9s06+oxMk2hCj6lsWko4Jm pyw58+i6d7OisLTkm9uR0ulyl+ws+HT0p7sn4zPRdBu83KgTQBIRdUWuDtaC0VtZzetK hr82Fm42ytbQq1ixRVphE6NFtz6271n2QxnCU6NlrhxVi0zHXtGnEB96fB2jwcM9lFVq S8/VP+7yFjXZxyzL1ECCRFfXZvGMKPc/hm4PfAVQZxtIK9g6uR9dPhhQbtzWAJSmycH2 Iz6g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693865523; x=1694470323; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=wYo3dq3VK96vstXSFrFSQooJJuj3c4z/z/PjKdvJaEQ=; b=iszIZ00gDqrcIxqe8QYUWY/t+74Vdbbyv0qFIKfDySrc9uWBxQ0A4hvxbo73LTEQyI LsVO+U1XPN7nQRagTWhfajp+K74xwM3qJcBOYkzYZYvZ0J8d+u74LVd4n6vYQMnr+lFX IwrICwlwKzTCU6mE9TdoTWUIL0jIjSs9hdm6OxM+wmVtaas8mBWDVyQsvVREN/RhVIHt j3IBZY8U9mC/azpBwVLrRT+O2a8V7+g++dyX8xwYMY8sZZ0mtewUPOcKaRhqJDQ+dM16 Dy4C79vzaOhdqA1G4rPXGYHYgdCmrIL/R+YIkWfbKkyYoU9nnrGvs4wWgZlhVAiUNHvd B4Ng== X-Gm-Message-State: AOJu0Yy8xQhGid3whdycg45WMFIMoTHpAQU544XM8/OjGPqfy0LJ299/ GYb5M50FC3VpGtyMI6goYhlH3g== X-Google-Smtp-Source: AGHT+IGWpcMIj9ryFTm84vfGakvVZw7X7Rz6JS5V+2fQaa3kiE//kN9E1jcd5aVC4tZ5XaUUqKgwTQ== X-Received: by 2002:a05:6512:2114:b0:4fd:d213:dfd4 with SMTP id q20-20020a056512211400b004fdd213dfd4mr6872963lfr.20.1693865523609; Mon, 04 Sep 2023 15:12:03 -0700 (PDT) Received: from olheureu-ThinkPad-E560.local.ess-mail.com ([2a02:578:85b9:1300:6c89:e61f:b837:7d81]) by smtp.gmail.com with ESMTPSA id z16-20020a170906715000b00993cc1242d4sm6692673ejj.151.2023.09.04.15.12.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Sep 2023 15:12:03 -0700 (PDT) From: Olivier L'Heureux To: Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz Cc: linux-bluetooth@vger.kernel.org, Olivier L'Heureux Subject: [PATCH 5/7] Bluetooth: introduce hci_conn_free() for better structure Date: Tue, 5 Sep 2023 00:11:56 +0200 Message-Id: <20230904221158.35425-6-olivier.lheureux@mind.be> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230904221158.35425-1-olivier.lheureux@mind.be> References: <20230904221158.35425-1-olivier.lheureux@mind.be> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org The bluetooth subsystem uses different sources for different layers or objects. In particular, the "hci_conn.c" source regroups the handling of the "struct hci_conn" objects, amongst other things. "hci_conn.c" contains "hci_conn_add()" to allocate the "struct hci_conn", "hci_conn_del()" to delete them etc. One function is lacking: a "hci_conn_free()" to free the "struct hci_conn". The "kfree()" is in the "bt_link_release()" [1], in "hci_sysfs.c". "bt_link_release()" is the callback called when the "struct device" reference count reaches 0. It makes sense that "bt_link_release()" is in "hci_sysfs.c", with the other functions related to "struct device" and sysfs, but to respect the structure of the bluetooth subsystem, "bt_link_release()" should not directly call "kfree()" on the "struct hci_conn" object. It should call a freeing function located in "hci_conn.c", so that "hci_conn.c" contains both the allocation and free of "struct hci_conn" objects. This improved structure becomes necessary if we want to do more than just calling "kfree()" in "bt_link_release()". We want to access the "struct l2cap_conn" associated to the "struct hci_conn", we can do this in "hci_conn.c", which includes "l2cap.h", while we can't do this in "hci_sysfs.c", for which "struct l2cap_conn" is opaque. For those structural reasons: 1. We create a new "hci_conn_free()" function in "hci_conn.c", whose purpose is to free the "struct hci_conn". 2. We export it by declaring it in "include/net/bluetooth/hci_core.h". 3. Instead of freeing the "struct hci_conn" in "bt_link_release()", we call "hci_conn_free()" where we have moved the content of "bt_link_release()". References: - [1] "bt_link_release" Signed-off-by: Olivier L'Heureux Signed-off-by: Olivier L'Heureux --- include/net/bluetooth/hci_core.h | 1 + net/bluetooth/hci_conn.c | 7 +++++++ net/bluetooth/hci_sysfs.c | 4 ++-- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index d8badb2a28cd..d5a9ef8909d4 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -1328,6 +1328,7 @@ int hci_le_create_cis(struct hci_conn *conn); struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst, u8 role); +void hci_conn_free(struct hci_conn *conn); void hci_conn_del(struct hci_conn *conn); void hci_conn_hash_flush(struct hci_dev *hdev); void hci_conn_check_pending(struct hci_dev *hdev); diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 23e635600717..755125403331 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1134,6 +1134,13 @@ static void hci_conn_unlink(struct hci_conn *conn) conn->link = NULL; } +void hci_conn_free(struct hci_conn *conn) +{ + BT_DBG("kfree(conn %p)", conn); + + kfree(conn); +} + void hci_conn_del(struct hci_conn *conn) { struct hci_dev *hdev = conn->hdev; diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c index fc297b651881..b0d841dcf860 100644 --- a/net/bluetooth/hci_sysfs.c +++ b/net/bluetooth/hci_sysfs.c @@ -14,9 +14,9 @@ static void bt_link_release(struct device *dev) { struct hci_conn *conn = to_hci_conn(dev); - BT_DBG("kfree(conn %p)", conn); + BT_DBG("dev %p conn %p", dev, conn); - kfree(conn); + hci_conn_free(conn); } static const struct device_type bt_link = { From patchwork Mon Sep 4 22:11:57 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Olivier L'Heureux X-Patchwork-Id: 13374310 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6A661C83F33 for ; Mon, 4 Sep 2023 22:12:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240582AbjIDWMM (ORCPT ); Mon, 4 Sep 2023 18:12:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37352 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240559AbjIDWMK (ORCPT ); Mon, 4 Sep 2023 18:12:10 -0400 Received: from mail-ej1-x62d.google.com (mail-ej1-x62d.google.com [IPv6:2a00:1450:4864:20::62d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 62CDBCDE for ; Mon, 4 Sep 2023 15:12:06 -0700 (PDT) Received: by mail-ej1-x62d.google.com with SMTP id a640c23a62f3a-99bcf2de59cso277303766b.0 for ; Mon, 04 Sep 2023 15:12:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mind.be; s=google; t=1693865525; x=1694470325; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=7D6jDdghy5xf6pMbgp+7uQJ09JEXfvhk/SVi6T9v3Ek=; b=WsoowIIwsTH6gDRPIEkDlBzfhAef6vHyYooPMNQ0/R7UHg4NZ3ymFQHYAF5qH6nN5k RMzOn18PqoR46Z5HBTFhZwZHEwfgye1gkuDaY5TdiTCBOR5/hueQ/nFQc96gILOg/+BE J/ziIi0f4S8Y/OLPHCiTngez4jqK/prKEx1c12G9LbFLk6k9s6avb4sJu/HB9afb6NfL rV48703CDaOAR6Z5etb7v2QeVvD+Zfi2xle3cHwOSUI6HKA9QzHw+TebZab6arybPAoj Vp1VlVPQ+imypGgoZy4mXU1KcJO2AV0tne/ixYIuteRXYBiOJOEV8SVteFSBiOiifDB9 DvsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693865525; x=1694470325; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=7D6jDdghy5xf6pMbgp+7uQJ09JEXfvhk/SVi6T9v3Ek=; b=QxaBH2cWzIP/dQChuiUkr95ca0BCYV46IMBGc0C3On4todXl+ei3FRfB67kAGZTDlN 6NNiKf6Q2kz7OJbtULx+D90VHelzWeE4NOtmFrGxSpf6RyIH8QesVCz/maPnptiW5fC7 ZkEHUxHQJbMnO7CKDc9/yQns37OzZFmvfCyfuYItePlHxzVJImG0MOJMFP1Gm/M+yzpe JwTQYbpk0TKOaKa9Q/NoyX/IB9mwFvqstZoBeaeV3X13j1yzai0hQAyoDlTo5P3lXlti iS9Y4T7rBYFhltYfWDOG0WEfPvzFlk2bzvtjTdSA6OvShG4xg7WFj1rHIe1GfxOhAy3q lpng== X-Gm-Message-State: AOJu0YxQK2IpjxYZXzCBJ7Luzq7MMS/cGP1z9sB1loS0sAMWc/p55Mls SvZt/q27MfhK5ARj0HgTwtv87Q== X-Google-Smtp-Source: AGHT+IFL5FtEPkDGuZTZWB/Nd8VBwJtn8Tyx+gwnrjOR4xJEYJCQ0t5YGJjf7AMvoxlFwcyUvL3I4A== X-Received: by 2002:a17:906:3149:b0:9a5:cade:8044 with SMTP id e9-20020a170906314900b009a5cade8044mr7760863eje.21.1693865524929; Mon, 04 Sep 2023 15:12:04 -0700 (PDT) Received: from olheureu-ThinkPad-E560.local.ess-mail.com ([2a02:578:85b9:1300:6c89:e61f:b837:7d81]) by smtp.gmail.com with ESMTPSA id z16-20020a170906715000b00993cc1242d4sm6692673ejj.151.2023.09.04.15.12.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Sep 2023 15:12:04 -0700 (PDT) From: Olivier L'Heureux To: Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz Cc: linux-bluetooth@vger.kernel.org, Olivier L'Heureux Subject: [PATCH 6/7] Bluetooth: L2CAP: inc refcount if reuse struct l2cap_conn Date: Tue, 5 Sep 2023 00:11:57 +0200 Message-Id: <20230904221158.35425-7-olivier.lheureux@mind.be> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230904221158.35425-1-olivier.lheureux@mind.be> References: <20230904221158.35425-1-olivier.lheureux@mind.be> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org Now that the "struct l2cap_conn" memory leak [1] is fixed, we observe use-after-free errors. Arnout Vandecappelle has found the root cause: the if (conn) return conn; at the beginning of "l2cap_conn_add()". It reuses an existing "struct l2cap_conn *" instead of allocating a new one. But there is an incoherence in the reference counting: 1. In the normal case (no reuse), "l2cap_conn_add()" allocates a new "struct l2cap_conn", initiates its "ref" reference counter to 1 and returns it. The caller is responsible to eventually call "l2cap_conn_put()" to free the returned "struct l2cap_conn". 2. If "l2cap_conn_add()" reuses an existing "struct l2cap_conn", it will return it immediately. But the caller, which can not know if "l2cap_conn_add()" reused an existing "struct l2cap_conn" or not, will eventually call "l2cap_conn_put()", for which there were no corresponding "l2cap_conn_get()". Tracing the reuse showed the reuse was indeed the reason for the use-after-free: [...] [ 960.331756] l2cap_conn_add:7719: hcon 1f5f8bdf reuse conn b69c7ec5 [ 960.331798] ------------[ cut here ]------------ [ 960.339480] WARNING: CPU: 0 PID: 173 at lib/refcount.c:25 l2cap_conn_get+0x8c/0x94 [ 960.353863] refcount_t: addition on 0; use-after-free. [ 960.362924] Modules linked in: [ 960.368036] CPU: 0 PID: 173 Comm: ble-memleak-rep Not tainted 5.13.0 #19 [ 960.380245] Hardware name: STM32 (Device Tree Support) [ 960.387449] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 960.400742] [] (show_stack) from [] (dump_stack+0xb4/0xc8) [ 960.413501] [] (dump_stack) from [] (__warn+0xb8/0x114) [ 960.425994] [] (__warn) from [] (warn_slowpath_fmt+0x78/0xac) [ 960.439016] [] (warn_slowpath_fmt) from [] (l2cap_conn_get+0x8c/0x94) [ 960.452752] [] (l2cap_conn_get) from [] (__l2cap_chan_add+0x3c/0x1e4) [ 960.466486] [] (__l2cap_chan_add) from [] (l2cap_chan_connect+0x514/0x9c8) [ 960.480656] [] (l2cap_chan_connect) from [] (l2cap_sock_connect+0x144/0x21c) [ 960.495023] [] (l2cap_sock_connect) from [] (__sys_connect+0xc8/0xe0) [ 960.508927] [] (__sys_connect) from [] (ret_fast_syscall+0x0/0x58) [...] The solution to the incoherence is to strictly apply the rules of reference counting [2]. By returning a reference to the reused "struct l2cap_conn", "l2cap_conn_add()" creates a new reference, and this reference should be reference-counted. The: if (conn) return conn; must thus become: if (conn) return l2cap_conn_get(conn); References: - [1] "ble-memleak-repro" - [2] "Adding reference counters (krefs) to kernel objects" Signed-off-by: Olivier L'Heureux Signed-off-by: Olivier L'Heureux Suggested-by: Arnout Vandecappelle --- net/bluetooth/l2cap_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index f5dcb4a4fb15..5e4dd293b2a4 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -7844,8 +7844,8 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon) struct hci_chan *hchan; if (conn) { - BT_DBG("hcon %p reuse conn %p", hcon, conn); - return conn; + BT_DBG("hcon %p reuse conn %p with l2cap_conn_get()", hcon, conn); + return l2cap_conn_get(conn); } hchan = hci_chan_create(hcon); From patchwork Mon Sep 4 22:11:58 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Olivier L'Heureux X-Patchwork-Id: 13374311 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id C9473CA0FF6 for ; Mon, 4 Sep 2023 22:12:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240614AbjIDWMM (ORCPT ); Mon, 4 Sep 2023 18:12:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37364 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240571AbjIDWMK (ORCPT ); Mon, 4 Sep 2023 18:12:10 -0400 Received: from mail-ej1-x635.google.com (mail-ej1-x635.google.com [IPv6:2a00:1450:4864:20::635]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 036179C for ; Mon, 4 Sep 2023 15:12:07 -0700 (PDT) Received: by mail-ej1-x635.google.com with SMTP id a640c23a62f3a-9a1de3417acso593231166b.0 for ; Mon, 04 Sep 2023 15:12:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mind.be; s=google; t=1693865525; x=1694470325; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=kpqCu60Uo5ndUxfDHlk2SXzECf518yYJiTvEcId+jnA=; b=XqGf84x4l5RD19E5+3QyNHR6O5PPfUBUXaWD9BqUBfbyVVq8O0NsqL86k4cTZfixbe YJtUY44domy0EXH6nj+AcWyIKYy9Da4hO+b32RzxEdxyrdCszcrIgkVUBQEeB2e2NMlT qZvgpWf9JHIxwrIEZJyNVq9vRcKDyiRPLDsXzTxJ3mDtPpzHHAcvVMxkcilvG0ogFhGQ xldRfYcN5bzIKpEkHla1ok5LitlbnVG8jKDOy/gLa6Eprd0He1MXe9zAFIP4mv97k92T zm1tywjdm/YVrQ9PNaYDHFW+Bf1hEIxl67zkTEvVms2gGt00n0p6tRZjlNG9YZlA1N5t 23kw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1693865525; x=1694470325; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=kpqCu60Uo5ndUxfDHlk2SXzECf518yYJiTvEcId+jnA=; b=EI+r1knWss1KMc1McoHd335S67UckHh4IHYGma6yDsjoU6E5i/V2Mesgo7zEDNJyz1 HkQQG9zchRlX0mTSnSoz4JWdCL2eAxmIrNbMYA/10qphawQD7oKjuOGIIxIIgSwsBNwH MeWFjqHk9fsLrR01+FEU1qsWO2SEqMWtRLu0P9wTD2u6XzlvBmGc2t37jKQjqdjMEBZ5 4rfp+X+ulSVLvQETNy6vW7mnaAy1C3/K/+HoIDTxqh9MuUU/e1RvLtvLMcqnxp+WkZZ6 SdoWsTRI3Vzfygxd05f+j2tp26Ce1FV5WqnyY4cmkzoCJQbEfGw/l3YcuhH+/y4wvqa/ EUAQ== X-Gm-Message-State: AOJu0YyGYrrexkcc3zdBoN0LuKhbi7RHfRjZeCvp12JHbPYND+rxWeTH rnilhLEEocWpoXxTmSy1zmqZfaVtYfr07kW0S+0= X-Google-Smtp-Source: AGHT+IFytbEuoX+aHbLBqnNp4Xby20bgkTcZIp7/+iC3zlDrNfJDcNflr9qVByVIv0wjXq/xJdWA9Q== X-Received: by 2002:a17:906:3010:b0:9a5:bceb:1cf8 with SMTP id 16-20020a170906301000b009a5bceb1cf8mr11139118ejz.3.1693865525645; Mon, 04 Sep 2023 15:12:05 -0700 (PDT) Received: from olheureu-ThinkPad-E560.local.ess-mail.com ([2a02:578:85b9:1300:6c89:e61f:b837:7d81]) by smtp.gmail.com with ESMTPSA id z16-20020a170906715000b00993cc1242d4sm6692673ejj.151.2023.09.04.15.12.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Sep 2023 15:12:05 -0700 (PDT) From: Olivier L'Heureux To: Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz Cc: linux-bluetooth@vger.kernel.org, Olivier L'Heureux Subject: [PATCH 7/7] Bluetooth: unlink objects to avoid use-after-free Date: Tue, 5 Sep 2023 00:11:58 +0200 Message-Id: <20230904221158.35425-8-olivier.lheureux@mind.be> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230904221158.35425-1-olivier.lheureux@mind.be> References: <20230904221158.35425-1-olivier.lheureux@mind.be> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org While stressing the Bluetooth subsystem with constant scanning [1], we have observed use-after-free in a "l2cap_conn_get()" called from "l2cap_chan_connect()". Our log could show "l2cap_conn_add()" was reusing an existing "struct l2cap_conn" that was freed, and which reference counter was at 0: [...] [ 782.314141] [166] l2cap_conn_put:1940: conn cb471e8c, refcount 1 [ 782.314162] [166] l2cap_conn_free:1923: kfree(conn) cb471e8c [...] [ 782.314693] [166] l2cap_chan_connect:7829: 00:00:00:00:00:00 -> 00:22:a3:07:0a:00 (type 1) psm 0x0080 mode 0x00 [ 782.314721] [166] hci_get_route:692: 00:00:00:00:00:00 -> 00:22:a3:07:0a:00 [ 782.314745] [166] hci_conn_hold:1152: hcon bb2debe3 orig conn->refcnt 0 [ 782.314766] [166] l2cap_conn_add:7719: hcon bb2debe3 reuse conn cb471e8c [ 782.314786] [166] l2cap_conn_get:1931: conn cb471e8c, refcount 0 [ 782.314802] ------------[ cut here ]------------ [ 782.322645] WARNING: CPU: 1 PID: 166 at lib/refcount.c:25 l2cap_conn_get+0x8c/0x94 [ 782.336633] [57] le_scan_cleanup:156: hci0 hcon bb2debe3 [ 782.336669] refcount_t: addition on 0; use-after-free. [ 782.344020] Modules linked in: [ 782.349187] CPU: 1 PID: 166 Comm: ble-memleak-rep Not tainted 5.13.0 #20 [ 782.361524] Hardware name: STM32 (Device Tree Support) [ 782.368778] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 782.382160] [] (show_stack) from [] (dump_stack+0xb4/0xc8) [ 782.395026] [] (dump_stack) from [] (__warn+0xb8/0x114) [ 782.407785] [] (__warn) from [] (warn_slowpath_fmt+0x78/0xac) [ 782.421203] [] (warn_slowpath_fmt) from [] (l2cap_conn_get+0x8c/0x94) [ 782.435352] [] (l2cap_conn_get) from [] (l2cap_chan_connect+0x278/0x984) [ 782.449802] [] (l2cap_chan_connect) from [] (l2cap_sock_connect+0x144/0x21c) [ 782.464717] [] (l2cap_sock_connect) from [] (__sys_connect+0xc8/0xe0) [ 782.479054] [] (__sys_connect) from [] (ret_fast_syscall+0x0/0x58) [...] Our proposed solution is to avoid such a situation: the "struct l2cap_conn" was reused via a dangling pointer, since the "struct l2cap_conn" was freed. The pointers are the double-linked pointers between "struct hci_conn" and "struct l2cap_conn". We can at least set the dangling pointers to NULL, just before we free an object. This will avoid the use-after-free. Done: just before freeing a "struct hci_conn", set the corresponding pointer to NULL in "struct l2cap_conn", and just before freeing a "struct l2cap_conn", set the corresponding pointer to NULL in "struct hci_conn". Note that we take care for NULL when dereferencing the pointers. In: struct hci_conn *hcon; struct l2cap_conn *lcon; expressions like "hcon->l2cap_data->hcon" or "lcon->hcon->l2cap_data" could dereference NULL pointers, because "hcon->l2cap_data" or "lcon->hcon" could be NULL. Indeed, "l2cap_conn_free()" and "hci_conn_free()" run in different threads, potentially in parallel. References: - [1] "ble-memleak-repro" Signed-off-by: Olivier L'Heureux Signed-off-by: Olivier L'Heureux --- net/bluetooth/hci_conn.c | 7 +++++++ net/bluetooth/l2cap_core.c | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 755125403331..86446f482b9f 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -1136,8 +1136,15 @@ static void hci_conn_unlink(struct hci_conn *conn) void hci_conn_free(struct hci_conn *conn) { + struct l2cap_conn *lcon = conn->l2cap_data; + BT_DBG("kfree(conn %p)", conn); + if (lcon && lcon->hcon == conn) { + BT_DBG("conn %p conn->l2cap_data->hcon = NULL", conn); + lcon->hcon = NULL; + } + kfree(conn); } diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 5e4dd293b2a4..9c2384c32d93 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -1949,6 +1949,11 @@ static void l2cap_conn_free(struct kref *ref) BT_DBG("kfree(conn) %p", conn); + if (conn->hcon && conn->hcon->l2cap_data == conn) { + BT_DBG("conn %p conn->hcon->l2cap_data = NULL", conn); + conn->hcon->l2cap_data = NULL; + } + hci_conn_put(conn->hcon); kfree(conn); }