From: Arnd Bergmann arnd@arndb.de
bacmp() is a wrapper around memcpy(), which contain compile-time checks for buffer overflow. Since the hci_conn_request_evt() also calls bt_dev_dbg() with an implicit NULL pointer check, the compiler is now aware of a case where 'hdev' is NULL and treats this as meaning that zero bytes are available:
In file included from net/bluetooth/hci_event.c:32: In function 'bacmp', inlined from 'hci_conn_request_evt' at net/bluetooth/hci_event.c:3276:7: include/net/bluetooth/bluetooth.h:364:16: error: 'memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread] 364 | return memcmp(ba1, ba2, sizeof(bdaddr_t)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Add another NULL pointer check before the bacmp() to ensure the compiler understands the code flow enough to not warn about it. Since the patch that introduced the warning is marked for stable backports, this one should also go that way to avoid introducing build regressions.
Fixes: d70e44fef8621 ("Bluetooth: Reject connection with the device which has same BD_ADDR") Cc: Kees Cook keescook@chromium.org Cc: "Lee, Chun-Yi" jlee@suse.com Cc: Luiz Augusto von Dentz luiz.von.dentz@intel.com Cc: Marcel Holtmann marcel@holtmann.org Cc: stable@vger.kernel.org Signed-off-by: Arnd Bergmann arnd@arndb.de --- v2: rewrite completely --- net/bluetooth/hci_event.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 6f4409b4c3648..9b34c9f8ee02c 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -3273,7 +3273,7 @@ static void hci_conn_request_evt(struct hci_dev *hdev, void *data, /* Reject incoming connection from device with same BD ADDR against * CVE-2020-26555 */ - if (!bacmp(&hdev->bdaddr, &ev->bdaddr)) { + if (hdev && !bacmp(&hdev->bdaddr, &ev->bdaddr)) { bt_dev_dbg(hdev, "Reject connection with same BD_ADDR %pMR\n", &ev->bdaddr); hci_reject_conn(hdev, &ev->bdaddr);
On Mon, Oct 09, 2023 at 10:31:31PM +0200, Arnd Bergmann wrote:
From: Arnd Bergmann arnd@arndb.de
bacmp() is a wrapper around memcpy(), which contain compile-time checks for buffer overflow. Since the hci_conn_request_evt() also calls bt_dev_dbg() with an implicit NULL pointer check, the compiler is now aware of a case where 'hdev' is NULL and treats this as meaning that zero bytes are available:
In file included from net/bluetooth/hci_event.c:32: In function 'bacmp', inlined from 'hci_conn_request_evt' at net/bluetooth/hci_event.c:3276:7: include/net/bluetooth/bluetooth.h:364:16: error: 'memcmp' specified bound 6 exceeds source size 0 [-Werror=stringop-overread] 364 | return memcmp(ba1, ba2, sizeof(bdaddr_t)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Add another NULL pointer check before the bacmp() to ensure the compiler understands the code flow enough to not warn about it. Since the patch that introduced the warning is marked for stable backports, this one should also go that way to avoid introducing build regressions.
Fixes: d70e44fef8621 ("Bluetooth: Reject connection with the device which has same BD_ADDR") Cc: Kees Cook keescook@chromium.org Cc: "Lee, Chun-Yi" jlee@suse.com Cc: Luiz Augusto von Dentz luiz.von.dentz@intel.com Cc: Marcel Holtmann marcel@holtmann.org Cc: stable@vger.kernel.org Signed-off-by: Arnd Bergmann arnd@arndb.de
A weird side-effect of the NULL check, but not unreasonable. :)
Reviewed-by: Kees Cook keescook@chromium.org
Hello:
This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz luiz.von.dentz@intel.com:
On Mon, 9 Oct 2023 22:31:31 +0200 you wrote:
From: Arnd Bergmann arnd@arndb.de
bacmp() is a wrapper around memcpy(), which contain compile-time checks for buffer overflow. Since the hci_conn_request_evt() also calls bt_dev_dbg() with an implicit NULL pointer check, the compiler is now aware of a case where 'hdev' is NULL and treats this as meaning that zero bytes are available:
[...]
Here is the summary with links: - [v2] Bluetooth: avoid memcmp() out of bounds warning https://git.kernel.org/bluetooth/bluetooth-next/c/b8ba8e65e84b
You are awesome, thank you!
linux-stable-mirror@lists.linaro.org