From f2edd9f67b8bdbe8cc3bc5c0ba4992e511147642 Mon Sep 17 00:00:00 2001 From: Sebastian Reichel Date: Sat, 15 Apr 2017 23:54:13 +0200 Subject: Bluetooth: hci_ll: Fix NULL pointer deref on FW upload failure Avoid NULL pointer dereference occurring due to freeing skb containing an error pointer. It can easily be triggered by using the driver with broken uart (i.e. due to misconfigured pinmuxing). Fixes: 371805522f87 ("bluetooth: hci_uart: add LL protocol serdev driver support") Signed-off-by: Sebastian Reichel Signed-off-by: Marcel Holtmann --- drivers/bluetooth/hci_ll.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c index 485e8eb04542..adc444f309a3 100644 --- a/drivers/bluetooth/hci_ll.c +++ b/drivers/bluetooth/hci_ll.c @@ -537,8 +537,7 @@ static int read_local_version(struct hci_dev *hdev) if (IS_ERR(skb)) { bt_dev_err(hdev, "Reading TI version information failed (%ld)", PTR_ERR(skb)); - err = PTR_ERR(skb); - goto out; + return PTR_ERR(skb); } if (skb->len != sizeof(*ver)) { err = -EILSEQ; -- cgit v1.2.3 From 1fb78fb6c6ad3751cce18d37e773d07858c1ced9 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Wed, 19 Apr 2017 19:50:18 +0200 Subject: Bluetooth: try to improve CONFIG_SERIAL_DEV_BUS dependency With CONFIG_SERIAL_DEV_BUS=m, the hci_serdev.o file does not actually get built into hci_uart.o as the Makefile doesn't pick it up, leading to a link error with anything referring to it: ERROR: "hci_uart_register_device" [drivers/bluetooth/hci_nokia.ko] undefined! scripts/Makefile.modpost:91: recipe for target '__modpost' failed Changing this in the Makefile would cause another problem when hci_uart itself is built-in and cannot reference symbols from the serdev module. This tries to address both problems by introducing a new hidden Kconfig symbol that controls both the compilation of hci_serdev.o and whether the Nokia driver can be selected. This seems to address the problem for me, though there might be a better way to do it. Fixes: 7bb318680e86 ("Bluetooth: add nokia driver") Signed-off-by: Arnd Bergmann Signed-off-by: Marcel Holtmann --- drivers/bluetooth/Kconfig | 8 +++++++- drivers/bluetooth/Makefile | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig index 8aafbed9e160..737d93ef27c5 100644 --- a/drivers/bluetooth/Kconfig +++ b/drivers/bluetooth/Kconfig @@ -76,6 +76,12 @@ config BT_HCIUART Say Y here to compile support for Bluetooth UART devices into the kernel or say M to compile it as module (hci_uart). +config BT_HCIUART_SERDEV + bool + depends on SERIAL_DEV_BUS && BT_HCIUART + depends on SERIAL_DEV_BUS=y || SERIAL_DEV_BUS=BT_HCIUART + default y + config BT_HCIUART_H4 bool "UART (H4) protocol support" depends on BT_HCIUART @@ -89,7 +95,7 @@ config BT_HCIUART_H4 config BT_HCIUART_NOKIA tristate "UART Nokia H4+ protocol support" depends on BT_HCIUART - depends on SERIAL_DEV_BUS + depends on BT_HCIUART_SERDEV depends on PM help Nokia H4+ is serial protocol for communication between Bluetooth diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile index a7f237320f4b..e693ca6eeed9 100644 --- a/drivers/bluetooth/Makefile +++ b/drivers/bluetooth/Makefile @@ -31,7 +31,7 @@ btmrvl-y := btmrvl_main.o btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o hci_uart-y := hci_ldisc.o -hci_uart-$(CONFIG_SERIAL_DEV_BUS) += hci_serdev.o +hci_uart-$(CONFIG_BT_HCIUART_SERDEV) += hci_serdev.o hci_uart-$(CONFIG_BT_HCIUART_H4) += hci_h4.o hci_uart-$(CONFIG_BT_HCIUART_BCSP) += hci_bcsp.o hci_uart-$(CONFIG_BT_HCIUART_LL) += hci_ll.o -- cgit v1.2.3 From eee6044f6694ae21f6f4b8e5ce4f13dbda0c112b Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Wed, 19 Apr 2017 19:32:25 +0200 Subject: ieee802154: don't select COMMON_CLK A device driver must not select the COMMON_CLK subsystem, as that conflicts with platforms that provide a legacy implementation of the clk API: drivers/clk/clk.o: In function `clk_enable': clk.c:(.text.clk_enable+0x0): multiple definition of `clk_enable' arch/arm/mach-sa1100/clock.o:clock.c:(.text.clk_enable+0x0): first defined here drivers/clk/clk.o: In function `clk_round_rate': clk.c:(.text.clk_round_rate+0x0): multiple definition of `clk_round_rate' arch/arm/mach-sa1100/clock.o:clock.c:(.text.clk_round_rate+0x0): first defined here drivers/clk/clk.o: In function `clk_get_parent': clk.c:(.text.clk_get_parent+0x0): multiple definition of `clk_get_parent' arch/arm/mach-sa1100/clock.o:clock.c:(.text.clk_get_parent+0x0): first defined here drivers/clk/clk.o: In function `clk_get_rate': clk.c:(.text.clk_get_rate+0x0): multiple definition of `clk_get_rate' This changes the 'select' into 'depends on', as all other similar drivers do. Fixes: d931acd575d6 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver") Signed-off-by: Arnd Bergmann Signed-off-by: Marcel Holtmann --- drivers/net/ieee802154/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ieee802154/Kconfig b/drivers/net/ieee802154/Kconfig index ce4864dc3c6e..303ba4133920 100644 --- a/drivers/net/ieee802154/Kconfig +++ b/drivers/net/ieee802154/Kconfig @@ -86,8 +86,8 @@ config IEEE802154_ADF7242 config IEEE802154_CA8210 tristate "Cascoda CA8210 transceiver driver" depends on IEEE802154_DRIVERS && MAC802154 + depends on COMMON_CLK depends on SPI - select COMMON_CLK ---help--- Say Y here to enable the CA8210 SPI 802.15.4 wireless controller. -- cgit v1.2.3 From cb926520e18e6aecc63614b8aa2e40d431aa29cd Mon Sep 17 00:00:00 2001 From: Dean Jenkins Date: Thu, 20 Apr 2017 18:06:39 +0100 Subject: Bluetooth: hci_ldisc: Add missing return in hci_uart_init_work() If hci_register_dev() returns an error in hci_uart_init_work() then the HCI_UART_REGISTERED bit gets erroneously set due to a missing return statement. Therefore, add the missing return statement. The consequence of the missing return is that the HCI UART is not registered but HCI_UART_REGISTERED is set which allows the code to think that hu->hdev is safe to access but hu->hdev has been freed so could lead to a crash. Signed-off-by: Dean Jenkins Signed-off-by: Marcel Holtmann --- drivers/bluetooth/hci_ldisc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c index cec4438ede01..1166e3f5682d 100644 --- a/drivers/bluetooth/hci_ldisc.c +++ b/drivers/bluetooth/hci_ldisc.c @@ -187,6 +187,7 @@ static void hci_uart_init_work(struct work_struct *work) hci_free_dev(hu->hdev); hu->hdev = NULL; hu->proto->close(hu); + return; } set_bit(HCI_UART_REGISTERED, &hu->flags); -- cgit v1.2.3 From a225b8c70af9368cfcb52a3608bc862bc88a7801 Mon Sep 17 00:00:00 2001 From: Dean Jenkins Date: Thu, 20 Apr 2017 18:06:40 +0100 Subject: Bluetooth: hci_ldisc: Ensure hu->hdev set to NULL before freeing hdev When hci_register_dev() fails, hu->hdev should be set to NULL before freeing hdev. This avoids potential use of hu->hdev after it has been freed. This commit sets hu->hdev to NULL before calling hci_free_dev() in error handling scenarios in hci_uart_init_work() and hci_uart_register_dev(). Signed-off-by: Dean Jenkins Signed-off-by: Marcel Holtmann --- drivers/bluetooth/hci_ldisc.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c index 1166e3f5682d..b1096d1ab30e 100644 --- a/drivers/bluetooth/hci_ldisc.c +++ b/drivers/bluetooth/hci_ldisc.c @@ -177,6 +177,7 @@ static void hci_uart_init_work(struct work_struct *work) { struct hci_uart *hu = container_of(work, struct hci_uart, init_ready); int err; + struct hci_dev *hdev; if (!test_and_clear_bit(HCI_UART_INIT_PENDING, &hu->hdev_flags)) return; @@ -184,8 +185,9 @@ static void hci_uart_init_work(struct work_struct *work) err = hci_register_dev(hu->hdev); if (err < 0) { BT_ERR("Can't register HCI device"); - hci_free_dev(hu->hdev); + hdev = hu->hdev; hu->hdev = NULL; + hci_free_dev(hdev); hu->proto->close(hu); return; } @@ -603,6 +605,7 @@ static int hci_uart_register_dev(struct hci_uart *hu) if (hci_register_dev(hdev) < 0) { BT_ERR("Can't register HCI device"); + hu->hdev = NULL; hci_free_dev(hdev); return -ENODEV; } -- cgit v1.2.3 From d160b74da85a4ec072b076db056e27ba7246eba0 Mon Sep 17 00:00:00 2001 From: Dean Jenkins Date: Thu, 20 Apr 2017 18:06:41 +0100 Subject: Bluetooth: hci_ldisc: Add missing clear HCI_UART_PROTO_READY Ensure that HCI_UART_PROTO_READY is cleared before close(hu) is called which closes the Data Link protocol layer. Therefore, add the missing bit clear of HCI_UART_PROTO_READY to hci_uart_init_work() so that the flag is cleared when hci_register_dev fails. Without the fix, the functions of the Data Link protocol layer could potentially be accessed after that layer has been closed. This could lead to a crash as memory would have been freed in that layer. Signed-off-by: Dean Jenkins Signed-off-by: Marcel Holtmann --- drivers/bluetooth/hci_ldisc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c index b1096d1ab30e..c53513cb7654 100644 --- a/drivers/bluetooth/hci_ldisc.c +++ b/drivers/bluetooth/hci_ldisc.c @@ -188,6 +188,7 @@ static void hci_uart_init_work(struct work_struct *work) hdev = hu->hdev; hu->hdev = NULL; hci_free_dev(hdev); + clear_bit(HCI_UART_PROTO_READY, &hu->flags); hu->proto->close(hu); return; } -- cgit v1.2.3