btusb: fix overflow return values
PATCH 1/1 btusb: fix overflow return values
PTR_ERR() returns a long type value, but btusb_setup_intel() and btusb_setup_intel_patching() should return an int type value.
This bug makes the judgement "if (ret < 0)" not working on x86_64 architecture systems, leading to failure as below, even panic.
[ 12.958920] Bluetooth: hci0 command 0xfc8e tx timeout [ 14.961765] Bluetooth: hci0 command 0xfc8e tx timeout [ 16.964688] Bluetooth: hci0 command 0xfc8e tx timeout [ 20.954501] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 22.957358] Bluetooth: hci0 command 0xfc8e tx timeout [ 30.948922] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 32.951780] Bluetooth: hci0 command 0xfc8e tx timeout [ 40.943359] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 42.946219] Bluetooth: hci0 command 0xfc8e tx timeout [ 50.937812] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 52.940670] Bluetooth: hci0 command 0xfc8e tx timeout [ 60.932236] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 62.935092] Bluetooth: hci0 command 0xfc8e tx timeout [ 70.926688] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 72.929545] Bluetooth: hci0 command 0xfc8e tx timeout [ 80.921111] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 82.923969] Bluetooth: hci0 command 0xfc2f tx timeout [ 90.915542] Bluetooth: hci0 sending Intel patch command (0xfc2f) failed (-110) [ 92.918406] Bluetooth: hci0 command 0xfc11 tx timeout [ 100.909955] Bluetooth: hci0 sending Intel patch command (0xfc11) failed (-110) [ 102.912858] Bluetooth: hci0 command 0xfc60 tx timeout [ 110.904394] Bluetooth: hci0 sending Intel patch command (0xfc60) failed (-110) [ 112.907293] Bluetooth: hci0 command 0xfc11 tx timeout [ 120.898831] Bluetooth: hci0 exiting Intel manufacturer mode failed (-110) [ 120.904757] bluetoothd[1030]: segfault at 4 ip 00007f8b2eb55236 sp 00007fff53ff6920 error 4 in bluetoothd[7f8b2eaff000+cb000]
For not affecting other modules, I choose to modify the return values but not extend btusb_setup_intel() and btusb_setup_intel_patching()'s return types. This is harmless, because the return values were only used to comparing number 0.
--- drivers/bluetooth/btusb.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/bluetooth/btusb.c +7 -7
@@ -1092,7 +1092,7 @@ static int btusb_setup_intel_patching(struct hci_dev *hdev,if (IS_ERR (skb)) {BT_ERR ("%s sending Intel patch command (0x%4.4x) failed (%ld)" , hdev->name, cmd->opcode,PTR_ERR (skb)); -return -PTR_ERR (skb); +return -EFAULT ; }/* It ensures that the returned event matches the event data read from
@@ -1144,7 +1144,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)if (IS_ERR (skb)) {BT_ERR ("%s sending initial HCI reset command failed (%ld)" , hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return -EFAULT ; }kfree_skb (skb);
@@ -1158,7 +1158,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)if (IS_ERR (skb)) {BT_ERR ("%s reading Intel fw version command failed (%ld)" , hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return -EFAULT ; }if (skb->len !=sizeof (*ver)) {
@@ -1216,7 +1216,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)BT_ERR ("%s entering Intel manufacturer mode failed (%ld)" , hdev->name,PTR_ERR (skb));release_firmware (fw); -return -PTR_ERR (skb); +return -EFAULT ; }if (skb->data[0 ]) {
@@ -1273,7 +1273,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)if (IS_ERR (skb)) {BT_ERR ("%s exiting Intel manufacturer mode failed (%ld)" , hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return -EFAULT ; }kfree_skb (skb);
@@ -1289,7 +1289,7 @@ exit_mfg_disable:if (IS_ERR (skb)) {BT_ERR ("%s exiting Intel manufacturer mode failed (%ld)" , hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return -EFAULT ; }kfree_skb (skb);
@@ -1307,7 +1307,7 @@ exit_mfg_deactivate:if (IS_ERR (skb)) {BT_ERR ("%s exiting Intel manufacturer mode failed (%ld)" , hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return -EFAULT ; }kfree_skb (skb);
Signature · 1 line
1.8.3.2
The return value of btusb_setup_intel is compared with 0. Code as:
drivers/bluetooth/btusb.c:
static int btusb_probe(struct usb_interface *intf, const struct usb_device_id *id)
if (id->driver_info & BTUSB_INTEL) hdev->setup = btusb_setup_intel;
net/bluetooth/hci_core.c:
int hci_dev_open(__u16 dev)
if (hdev->setup && test_bit(HCI_SETUP, &hdev->dev_flags))
ret = hdev->setup(hdev);
if (!ret) {
Thanks,
Yang
Quoted 133 lines · Adam Lee
PTR_ERR()returns a long type value, butbtusb_setup_intel()andbtusb_setup_intel_patching()should return an int type value.This bug makes the judgement "if (ret < 0)" not working on
x86_64architecture systems, leading to failure as below, even panic.[ 12.958920] Bluetooth: hci0 command 0xfc8e tx timeout [ 14.961765] Bluetooth: hci0 command 0xfc8e tx timeout [ 16.964688] Bluetooth: hci0 command 0xfc8e tx timeout [ 20.954501] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 22.957358] Bluetooth: hci0 command 0xfc8e tx timeout [ 30.948922] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 32.951780] Bluetooth: hci0 command 0xfc8e tx timeout [ 40.943359] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 42.946219] Bluetooth: hci0 command 0xfc8e tx timeout [ 50.937812] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 52.940670] Bluetooth: hci0 command 0xfc8e tx timeout [ 60.932236] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 62.935092] Bluetooth: hci0 command 0xfc8e tx timeout [ 70.926688] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 72.929545] Bluetooth: hci0 command 0xfc8e tx timeout [ 80.921111] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 82.923969] Bluetooth: hci0 command 0xfc2f tx timeout [ 90.915542] Bluetooth: hci0 sending Intel patch command (0xfc2f) failed (-110) [ 92.918406] Bluetooth: hci0 command 0xfc11 tx timeout [ 100.909955] Bluetooth: hci0 sending Intel patch command (0xfc11) failed (-110) [ 102.912858] Bluetooth: hci0 command 0xfc60 tx timeout [ 110.904394] Bluetooth: hci0 sending Intel patch command (0xfc60) failed (-110) [ 112.907293] Bluetooth: hci0 command 0xfc11 tx timeout [ 120.898831] Bluetooth: hci0 exiting Intel manufacturer mode failed (-110) [ 120.904757] bluetoothd[1030]: segfault at 4 ip 00007f8b2eb55236 sp
00007fff53ff6920error 4 in bluetoothd[7f8b2eaff000+cb000]For not affecting other modules, I choose to modify the return values but not extend
btusb_setup_intel()and btusb_setup_intel_patching()'s return types. This is harmless, because the return values were only used to comparing number 0.Signed-off-by Adam Lee <[email protected]>--- drivers/bluetooth/btusb.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)drivers/bluetooth/btusb.c +1
-1@@ -1092,7 +1092,7 @@ static int btusb_setup_intel_patching(struct hci_dev *hdev, if (IS_ERR(skb)) {BT_ERR ("%s sending Intel patch command (0x%4.4x) failed (%ld)", hdev->name, cmd->opcode,PTR_ERR (skb)); -return -PTR_ERR (skb); +return -EFAULT ; }/* It ensures that the returned event matches the event data readfrom
drivers/bluetooth/btusb.c +6
-7@@ -1144,7 +1144,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)if (IS_ERR (skb)) {BT_ERR ("%s sending initial HCI reset command failed (%ld)" , hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return -EFAULT ; }kfree_skb (skb);@@ -1158,7 +1158,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)if (IS_ERR (skb)) {BT_ERR ("%s reading Intel fw version command failed (%ld)" , hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return -EFAULT ; }if (skb->len !=sizeof (*ver)) {@@ -1216,7 +1216,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)BT_ERR ("%s entering Intel manufacturer mode failed (%ld)" , hdev->name,PTR_ERR (skb));release_firmware (fw); -return -PTR_ERR (skb); +return -EFAULT ; }if (skb->data[0 ]) {@@ -1273,7 +1273,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)if (IS_ERR (skb)) {BT_ERR ("%s exiting Intel manufacturer mode failed (%ld)" , hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return -EFAULT ; }kfree_skb (skb);@@ -1289,7 +1289,7 @@ exit_mfg_disable:if (IS_ERR (skb)) {BT_ERR ("%s exiting Intel manufacturer mode failed (%ld)" , hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return -EFAULT ; }kfree_skb (skb);@@ -1307,7 +1307,7 @@ exit_mfg_deactivate:if (IS_ERR (skb)) {BT_ERR ("%s exiting Intel manufacturer mode failed (%ld)" , hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return -EFAULT ; }kfree_skb (skb); --1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Signature · 9 lines
"""
Keep It Simple,Stupid.
"""
Chinese Name: 白杨
Nick Name: Hamo
Homepage: http://hamobai.com/
GPG KEY ID: 0xA4691A33
Key fingerprint = 09D5 2D78 8E2B 0995 CF8E 4331 33C4 3D24 A469 1A33Resend without HTML format.
The return value of btusb_setup_intel is compared with 0. Code as:
drivers/bluetooth/btusb.c:
static int btusb_probe(struct usb_interface *intf, const struct usb_device_id *id) {
if (id->driver_info & BTUSB_INTEL)hdev->setup =3D btusb_setup_intel;
}
net/bluetooth/hci_core.c:
int hci_dev_open(__u16 dev) {
if (hdev->setup && test_bit(HCI_SETUP, &hdev->dev_flags)) ret =3D hdev->setup(hdev);
if (!ret) {
...
}
}
Thanks,
Yang
Quoted 16 lines · Yang Bai
The return value of
btusb_setup_intelis compared with 0. Code as:
drivers/bluetooth/btusb.c:
static intbtusb_probe(struct usb_interface *intf, const struct usb_device_id *id)
if (id->driver_info&BTUSB_INTEL)hdev->setup=3Dbtusb_setup_intel;
net/bluetooth/hci_core.c:
inthci_dev_open(__u16 dev)
if (hdev->setup&&test_bit(HCI_SETUP, &hdev->dev_flags)) ret =3Dhdev->setup(hdev);if (!ret) {
Quoted 17 lines
Thanks,
YangQuoted 11 lines · Adam Lee
PTR_ERR()returns a long type value, butbtusb_setup_intel()andbtusb_setup_intel_patching()should return an int type value.This bug makes the judgement "if (ret < 0)" not working on
x86_64architecture systems, leading to failure as below, even panic.[ 12.958920] Bluetooth: hci0 command 0xfc8e tx timeout [ 14.961765] Bluetooth: hci0 command 0xfc8e tx timeout [ 16.964688] Bluetooth: hci0 command 0xfc8e tx timeout [ 20.954501] Bluetooth: hci0 sending Intel patch command (0xfc8e) fail=
ed
Quoted 3 lines
Quoted 3 lines
(-110)
[ 22.957358] Bluetooth: hci0 command 0xfc8e tx timeout [ 30.948922] Bluetooth: hci0 sending Intel patch command (0xfc8e) fail=
ed
Quoted 3 lines
Quoted 3 lines
(-110)
[ 32.951780] Bluetooth: hci0 command 0xfc8e tx timeout [ 40.943359] Bluetooth: hci0 sending Intel patch command (0xfc8e) fail=
ed
Quoted 3 lines
Quoted 3 lines
(-110)
[ 42.946219] Bluetooth: hci0 command 0xfc8e tx timeout [ 50.937812] Bluetooth: hci0 sending Intel patch command (0xfc8e) fail=
ed
Quoted 3 lines
Quoted 3 lines
(-110)
[ 52.940670] Bluetooth: hci0 command 0xfc8e tx timeout [ 60.932236] Bluetooth: hci0 sending Intel patch command (0xfc8e) fail=
ed
Quoted 3 lines
Quoted 3 lines
(-110)
[ 62.935092] Bluetooth: hci0 command 0xfc8e tx timeout [ 70.926688] Bluetooth: hci0 sending Intel patch command (0xfc8e) fail=
ed
Quoted 3 lines
Quoted 3 lines
(-110)
[ 72.929545] Bluetooth: hci0 command 0xfc8e tx timeout [ 80.921111] Bluetooth: hci0 sending Intel patch command (0xfc8e) fail=
ed
Quoted 3 lines
Quoted 3 lines
(-110)
[ 82.923969] Bluetooth: hci0 command 0xfc2f tx timeout [ 90.915542] Bluetooth: hci0 sending Intel patch command (0xfc2f) fail=
ed
Quoted 3 lines
Quoted 3 lines
(-110)
[ 92.918406] Bluetooth: hci0 command 0xfc11 tx timeout [ 100.909955] Bluetooth: hci0 sending Intel patch command (0xfc11) fail=
ed
Quoted 3 lines
Quoted 3 lines
(-110)
[ 102.912858] Bluetooth: hci0 command 0xfc60 tx timeout [ 110.904394] Bluetooth: hci0 sending Intel patch command (0xfc60) fail=
ed
Quoted 17 lines
Quoted 17 lines
(-110)
[ 112.907293] Bluetooth: hci0 command 0xfc11 tx timeout [ 120.898831] Bluetooth: hci0 exiting Intel manufacturer mode failed (-110) [ 120.904757] bluetoothd[1030]: segfault at 4 ip 00007f8b2eb55236 sp
00007fff53ff6920error 4 in bluetoothd[7f8b2eaff000+cb000]For not affecting other modules, I choose to modify the return values but not extend
btusb_setup_intel()and btusb_setup_intel_patching()'s return types. This is harmless, because the return values were only used to comparing number 0.Signed-off-by Adam Lee <[email protected]>--- drivers/bluetooth/btusb.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
Quoted 5 lines
drivers/bluetooth/btusb.c static int btusb_setup_intel_patching(struct hci_d=
Quoted 5 lines
drivers/bluetooth/btusb.c +0
-0@@ -1092,7 +1092,7 @@ static int btusb_setup_intel_patching(struct hci_d=
ev
Quoted 10 lines
Quoted 10 lines
drivers/bluetooth/btusb.c +1
-1*hdev, if (IS_ERR(skb)) {BT_ERR ("%s sending Intel patch command (0x%4.4x) failed (%ld)", hdev->name, cmd->opcode,PTR_ERR (skb)); -return -PTR_ERR (skb); +return -EFAULT ; }/* It ensures that the returned event matches the event data rea=
d
Quoted 11 lines
Quoted 11 lines
from
drivers/bluetooth/btusb.c +1
-1@@ -1144,7 +1144,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)if (IS_ERR (skb)) {BT_ERR ("%s sending initial HCI reset command failed (%ld)", hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return -EFAULT ; }kfree_skb (skb);
Quoted 3 lines
drivers/bluetooth/btusb.c static int btusb_setup_intel(struct hci_dev *hdev) · line 1159
Quoted 3 lines
drivers/bluetooth/btusb.c +0
-0@@ -1158,7 +1158,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)if (IS_ERR (skb)) { BT_ERR("%s reading Intel fw version command failed (%ld)=
",
Quoted 8 lines
drivers/bluetooth/btusb.c static int btusb_setup_intel(struct hci_dev *hdev) · line 1216
Quoted 8 lines
drivers/bluetooth/btusb.c +1
-1hdev->name, PTR_ERR(skb)); -return -PTR_ERR (skb); +return -EFAULT ; } if (skb->len !=3D sizeof(*ver)) {@@ -1216,7 +1216,7 @@ static int btusb_setup_intel(struct hci_dev *hdev) BT_ERR("%s entering Intel manufacturer mode failed (%ld)=
",
Quoted 10 lines
drivers/bluetooth/btusb.c static int btusb_setup_intel(struct hci_dev *hdev) · line 1274
Quoted 10 lines
drivers/bluetooth/btusb.c +1
-1hdev->name, PTR_ERR(skb));release_firmware (fw); -return -PTR_ERR (skb); +return -EFAULT ; }if (skb->data[0 ]) {@@ -1273,7 +1273,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)if (IS_ERR (skb)) { BT_ERR("%s exiting Intel manufacturer mode failed (%ld)"=
,
Quoted 9 lines
drivers/bluetooth/btusb.c exit_mfg_disable: · line 1290
Quoted 9 lines
drivers/bluetooth/btusb.c +1
-1hdev->name, PTR_ERR(skb)); -return -PTR_ERR (skb); +return -EFAULT ; }kfree_skb (skb);@@ -1289,7 +1289,7 @@ exit_mfg_disable:if (IS_ERR (skb)) { BT_ERR("%s exiting Intel manufacturer mode failed (%ld)"=
,
Quoted 9 lines
drivers/bluetooth/btusb.c exit_mfg_deactivate: · line 1308
Quoted 9 lines
drivers/bluetooth/btusb.c +1
-1hdev->name, PTR_ERR(skb)); -return -PTR_ERR (skb); +return -EFAULT ; }kfree_skb (skb);@@ -1307,7 +1307,7 @@ exit_mfg_deactivate:if (IS_ERR (skb)) { BT_ERR("%s exiting Intel manufacturer mode failed (%ld)"=
,
Quoted 11 lines
Quoted 11 lines
drivers/bluetooth/btusb.c +1
-2hdev->name, PTR_ERR(skb)); -return -PTR_ERR (skb); +return -EFAULT ; }kfree_skb (skb); --1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" =
in
Quoted 12 lines
Quoted 3 lines
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/--
""" Keep It Simple,Stupid. """
Quoted 5 lines
Chinese Name: =E7=99=BD=E6=9D=A8
Nick Name: Hamo
Homepage: http://hamobai.com/
GPG KEY ID:0xA4691A33
Key fingerprint =3D 09D5 2D78 8E2B 0995 CF8E 4331 33C4 3D24 A469 1A33
--=20
"""
Keep It Simple,Stupid.
"""Chinese Name: =E7=99=BD=E6=9D=A8
Nick Name: Hamo
Homepage: http://hamobai.com/
GPG KEY ID: 0xA4691A33
Key fingerprint =3D 09D5 2D78 8E2B 0995 CF8E 4331 33C4 3D24 A469 1A33
Quoted 8 lines · Yang Bai
The return value of btusb_setup_intel is compared with 0. Code as:
drivers/bluetooth/btusb.c:
static intbtusb_probe(struct usb_interface *intf, const struct usb_device_id *id)
if (id->driver_info&BTUSB_INTEL)hdev->setup=btusb_setup_intel;
Quoted 6 lines
net/bluetooth/hci_core.c:
inthci_dev_open(__u16 dev)
if (hdev->setup&&test_bit(HCI_SETUP, &hdev->dev_flags)) ret =hdev->setup(hdev);if (!ret) {
Yes, for btusb_setup_intel(), the return value is compared with number "0", doesn't break the judgement.
But it still overflows stack without this fix.
Quoted 7 lines · Adam Lee
PTR_ERR()returns a long type value, butbtusb_setup_intel()andbtusb_setup_intel_patching()should return an int type value.This bug makes the judgement "if (ret < 0)" not working on
x86_64architecture systems, leading to failure as below, even panic.
Signature · 3 lines
Regards, Adam Lee Hardware Enablement
Quoted 14 lines · Adam Lee
Quoted 13 lines · Yang Bai
The return value of btusb_setup_intel is compared with 0. Code as:
drivers/bluetooth/btusb.c:
static intbtusb_probe(struct usb_interface *intf, const struct usb_device_id *id)
if (id->driver_info&BTUSB_INTEL)hdev->setup=btusb_setup_intel;
net/bluetooth/hci_core.c:
inthci_dev_open(__u16 dev)
if (hdev->setup&&test_bit(HCI_SETUP, &hdev->dev_flags)) ret =hdev->setup(hdev);
Quoted 6 lines
Quoted 1 line
if (!ret) {
Yes, for
btusb_setup_intel(), the return value is compared with number "0", doesn't break the judgement.But it still overflows stack without this fix.
And what if the lower 32bits of PTR_ERR() are all "0"? :D
Quoted 7 lines
Quoted 7 lines · Adam Lee
PTR_ERR()returns a long type value, butbtusb_setup_intel()andbtusb_setup_intel_patching()should return an int type value.This bug makes the judgement "if (ret < 0)" not working on
x86_64architecture systems, leading to failure as below, even panic.
Signature · 3 lines
Regards, Adam Lee Hardware Enablement
Marcel Holtmann
maintainerHi Adam,
Quoted 32 lines
PTR_ERR()returns a long type value, butbtusb_setup_intel()andbtusb_setup_intel_patching()should return an int type value.This bug makes the judgement "if (ret < 0)" not working on
x86_64architecture systems, leading to failure as below, even panic.[ 12.958920] Bluetooth: hci0 command 0xfc8e tx timeout [ 14.961765] Bluetooth: hci0 command 0xfc8e tx timeout [ 16.964688] Bluetooth: hci0 command 0xfc8e tx timeout [ 20.954501] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 22.957358] Bluetooth: hci0 command 0xfc8e tx timeout [ 30.948922] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 32.951780] Bluetooth: hci0 command 0xfc8e tx timeout [ 40.943359] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 42.946219] Bluetooth: hci0 command 0xfc8e tx timeout [ 50.937812] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 52.940670] Bluetooth: hci0 command 0xfc8e tx timeout [ 60.932236] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 62.935092] Bluetooth: hci0 command 0xfc8e tx timeout [ 70.926688] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 72.929545] Bluetooth: hci0 command 0xfc8e tx timeout [ 80.921111] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 82.923969] Bluetooth: hci0 command 0xfc2f tx timeout [ 90.915542] Bluetooth: hci0 sending Intel patch command (0xfc2f) failed (-110) [ 92.918406] Bluetooth: hci0 command 0xfc11 tx timeout [ 100.909955] Bluetooth: hci0 sending Intel patch command (0xfc11) failed (-110) [ 102.912858] Bluetooth: hci0 command 0xfc60 tx timeout [ 110.904394] Bluetooth: hci0 sending Intel patch command (0xfc60) failed (-110) [ 112.907293] Bluetooth: hci0 command 0xfc11 tx timeout [ 120.898831] Bluetooth: hci0 exiting Intel manufacturer mode failed (-110) [ 120.904757] bluetoothd[1030]: segfault at 4 ip 00007f8b2eb55236 sp 00007fff53ff6920 error 4 in bluetoothd[7f8b2eaff000+cb000]
Quoted 4 lines
For not affecting other modules, I choose to modify the return values but not extend
btusb_setup_intel()and btusb_setup_intel_patching()'s return types. This is harmless, because the return values were only used to comparing number 0.
there are tons of examples in various subsystems and drivers where we return PTR_ERR from a function calls returning int.
So I wonder what is actually going wrong here. If this is x86_64 specific problem with PTR_ERR vs int, then we should have this problem everywhere in the kernel.
Regards
Marcel
Quoted 13 lines · Marcel Holtmann
Hi Adam,
Quoted 10 lines
PTR_ERR()returns a long type value, butbtusb_setup_intel()andbtusb_setup_intel_patching()should return an int type value.This bug makes the judgement "if (ret < 0)" not working on
x86_64architecture systems, leading to failure as below, even panic.
...
For not affecting other modules, I choose to modify the return values but not extendbtusb_setup_intel()and btusb_setup_intel_patching()'s return types. This is harmless, because the return values were only used to comparing number 0.
Quoted 6 lines
there are tons of examples in various subsystems and drivers where we return
PTR_ERRfrom a function calls returning int.So I wonder what is actually going wrong here. If this is
x86_64specific problem withPTR_ERRvs int, then we should have this problem everywhere in the kernel.
Hi, Marcel
I see you point, the difference between here and other subsystems are:
1, it returns -PTR_ERR() here but all other places return PTR_ERR(), I checked.
2, the judgement is "if (ret < 0)" here but other places are "if (ret)".
I'm not saying other subsystems are 100% right, but here, returning -PTR_ERR() and checking "if (ret < 0)" make the judgement broken much much more easily.
I attached a testing C file, run it on x86_64, you will see the bug.
PS, about other subsystems, I also think returning PTR_ERR() from a function calls returning int considered harmful sometimes, will talk about that in other thread.
Great thanks.
Signature · 3 lines
Regards, Adam Lee Hardware Enablement
#include <stdio.h>
static inline long PTR_ERR(const void *ptr)
{
return (long) ptr;
}int main(int argc, const char *argv[])
{
printf("sizeof(char) = %lu, sizeof(int) = %lu, sizeof(long) = %lu\n\n",
sizeof(char), sizeof(int), sizeof(long));
/*This address is in kernel space, check Documentation/x86/x86_64/mm.txt*/
void *ptr = (void *)0Xffff8900f0000000;
printf("ptr = %p, PTR_ERR(ptr) = %lx, (int)(-PTR_ERR(ptr)) = %d\n\n",
ptr, PTR_ERR(ptr), (int)(-PTR_ERR(ptr)));
if ((int)(-PTR_ERR(ptr)) < 0)
printf("That's what the codes want.\n");
else
printf("Bug happens!\n");
return 0;
}Quoted 33 lines · Adam Lee
Quoted 13 lines · Marcel Holtmann
Hi Adam,
Quoted 10 lines
PTR_ERR()returns a long type value, butbtusb_setup_intel()andbtusb_setup_intel_patching()should return an int type value.This bug makes the judgement "if (ret < 0)" not working on
x86_64architecture systems, leading to failure as below, even panic.
...
For not affecting other modules, I choose to modify the return values but not extendbtusb_setup_intel()and btusb_setup_intel_patching()'s return types. This is harmless, because the return values were only used to comparing number 0.Quoted 6 lines
there are tons of examples in various subsystems and drivers where we return
PTR_ERRfrom a function calls returning int.So I wonder what is actually going wrong here. If this is
x86_64specific problem withPTR_ERRvs int, then we should have this problem everywhere in the kernel.Hi, Marcel
I see you point, the difference between here and other subsystems are:
1, it returns -PTR_ERR() here but all other places return
PTR_ERR(), I checked.
2, the judgement is "if (ret < 0)" here but other places are "if (ret)".I'm not saying other subsystems are 100% right, but here, returning -PTR_ERR() and checking "if (ret < 0)" make the judgement broken much much more easily.
Quoted 5 lines
I attached a testing C file, run it on
x86_64, you will see the bug.PS, about other subsystems, I also think returning
PTR_ERR()from a function calls returning int considered harmful sometimes, will talk about that in other thread.
Hi, all
After diving into the err.h, I realized this patch contains some modifications which are actually not necessary. Will submit a v2 version and explain.
PS, other subsystems are using it right, this not.
Signature · 3 lines
Regards, Adam Lee Hardware Enablement
PATCH v2 1/1 btusb: fix wrong use of PTR_ERR()
PTR_ERR() returns a signed long type value which is limited by IS_ERR(), it must be a negative number and bigger than -MAX_ERRNO(-4095).
The bug here, btusb_setup_intel_patching()'s return value might come from -PTR_ERR() which must be a positive number, we can't judge if it's an error by "if (ret < 0)", the wrong use here leads to failure as below, even panic.
Error log:
[ 12.958920] Bluetooth: hci0 command 0xfc8e tx timeout [ 14.961765] Bluetooth: hci0 command 0xfc8e tx timeout [ 16.964688] Bluetooth: hci0 command 0xfc8e tx timeout [ 20.954501] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 22.957358] Bluetooth: hci0 command 0xfc8e tx timeout [ 30.948922] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 32.951780] Bluetooth: hci0 command 0xfc8e tx timeout [ 40.943359] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 42.946219] Bluetooth: hci0 command 0xfc8e tx timeout [ 50.937812] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 52.940670] Bluetooth: hci0 command 0xfc8e tx timeout [ 60.932236] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 62.935092] Bluetooth: hci0 command 0xfc8e tx timeout [ 70.926688] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 72.929545] Bluetooth: hci0 command 0xfc8e tx timeout [ 80.921111] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 82.923969] Bluetooth: hci0 command 0xfc2f tx timeout [ 90.915542] Bluetooth: hci0 sending Intel patch command (0xfc2f) failed (-110) [ 92.918406] Bluetooth: hci0 command 0xfc11 tx timeout [ 100.909955] Bluetooth: hci0 sending Intel patch command (0xfc11) failed (-110) [ 102.912858] Bluetooth: hci0 command 0xfc60 tx timeout [ 110.904394] Bluetooth: hci0 sending Intel patch command (0xfc60) failed (-110) [ 112.907293] Bluetooth: hci0 command 0xfc11 tx timeout [ 120.898831] Bluetooth: hci0 exiting Intel manufacturer mode failed (-110) [ 120.904757] bluetoothd[1030]: segfault at 4 ip 00007f8b2eb55236 sp 00007fff53ff6920 error 4 in bluetoothd[7f8b2eaff000+cb000]
--- drivers/bluetooth/btusb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/bluetooth/btusb.c +1 -1
@@ -1256,7 +1256,7 @@ static int btusb_setup_intel(struct hci_dev *hdev) ret =btusb_setup_intel_patching (hdev, fw, &fw_ptr, &disable_patch); -if (ret <0 ) +if (!ret) goto exit_mfg_deactivate; }
Signature · 1 line
1.8.3.2
#include <stdio.h>
#define MAX_ERRNO 4095
#define IS_ERR_VALUE(x) ((x) >= (unsigned long)-MAX_ERRNO)
static inline long IS_ERR(const void *ptr)
{
return IS_ERR_VALUE((unsigned long)ptr);
}static inline long PTR_ERR(const void *ptr)
{
return (long) ptr;
}int main(int argc, const char *argv[])
{
void *ptr = (void *)-1;
printf("ptr = %p, -PTR_ERR(ptr) = %ld\n\n", ptr, -PTR_ERR(ptr));
if(IS_ERR(ptr)) {
if (-PTR_ERR(ptr) < 0)
printf("That's what the codes want.\n");
else
printf("Bug happens!\n");
}
return 0;
}Hi Adam,
Quoted 21 lines · Adam Lee
Quoted 13 lines · Marcel Holtmann
Hi Adam,
Quoted 10 lines
PTR_ERR()returns a long type value, butbtusb_setup_intel()andbtusb_setup_intel_patching()should return an int type value.This bug makes the judgement "if (ret < 0)" not working on
x86_64architecture systems, leading to failure as below, even panic.
...
For not affecting other modules, I choose to modify the return values but not extendbtusb_setup_intel()and btusb_setup_intel_patching()'s return types. This is harmless, because the return values were only used to comparing number 0.Quoted 6 lines
there are tons of examples in various subsystems and drivers where we return
PTR_ERRfrom a function calls returning int.So I wonder what is actually going wrong here. If this is
x86_64specific problem withPTR_ERRvs int, then we should have this problem everywhere in the kernel.
Quoted 6 lines
Hi, Marcel
I see you point, the difference between here and other subsystems are:
1, it returns -PTR_ERR() here but all other places return
PTR_ERR(), I checked.
Please sending a patch fixing this. We got this right in other parts of the bluetooth subsystems but somehow we failed to check this when this code came in. And then another updating the checks if needed.
Gustavo
PATCH v3 1/1 btusb: fix wrong use of PTR_ERR()
PTR_ERR() returns a signed long type value which is limited by IS_ERR(), it must be a negative number whose range is [-MAX_ERRNO, 0).
The bug here returns negative numbers as error codes, then check it by "if (ret < 0)", but -PTR_ERR() is actually positive. The wrong use here leads to failure as below, even panic.
[ 12.958920] Bluetooth: hci0 command 0xfc8e tx timeout [ 14.961765] Bluetooth: hci0 command 0xfc8e tx timeout [ 16.964688] Bluetooth: hci0 command 0xfc8e tx timeout [ 20.954501] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 22.957358] Bluetooth: hci0 command 0xfc8e tx timeout [ 30.948922] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 32.951780] Bluetooth: hci0 command 0xfc8e tx timeout [ 40.943359] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 42.946219] Bluetooth: hci0 command 0xfc8e tx timeout [ 50.937812] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 52.940670] Bluetooth: hci0 command 0xfc8e tx timeout [ 60.932236] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 62.935092] Bluetooth: hci0 command 0xfc8e tx timeout [ 70.926688] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 72.929545] Bluetooth: hci0 command 0xfc8e tx timeout [ 80.921111] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 82.923969] Bluetooth: hci0 command 0xfc2f tx timeout [ 90.915542] Bluetooth: hci0 sending Intel patch command (0xfc2f) failed (-110) [ 92.918406] Bluetooth: hci0 command 0xfc11 tx timeout [ 100.909955] Bluetooth: hci0 sending Intel patch command (0xfc11) failed (-110) [ 102.912858] Bluetooth: hci0 command 0xfc60 tx timeout [ 110.904394] Bluetooth: hci0 sending Intel patch command (0xfc60) failed (-110) [ 112.907293] Bluetooth: hci0 command 0xfc11 tx timeout [ 120.898831] Bluetooth: hci0 exiting Intel manufacturer mode failed (-110) [ 120.904757] bluetoothd[1030]: segfault at 4 ip 00007f8b2eb55236 sp 00007fff53ff6920 error 4 in bluetoothd[7f8b2eaff000+cb000]
--- drivers/bluetooth/btusb.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/bluetooth/btusb.c +7 -7
@@ -1092,7 +1092,7 @@ static int btusb_setup_intel_patching(struct hci_dev *hdev,if (IS_ERR (skb)) {BT_ERR ("%s sending Intel patch command (0x%4.4x) failed (%ld)" , hdev->name, cmd->opcode,PTR_ERR (skb)); -return -PTR_ERR (skb); +return PTR_ERR (skb); }/* It ensures that the returned event matches the event data read from
@@ -1144,7 +1144,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)if (IS_ERR (skb)) {BT_ERR ("%s sending initial HCI reset command failed (%ld)" , hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return PTR_ERR (skb); }kfree_skb (skb);
@@ -1158,7 +1158,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)if (IS_ERR (skb)) {BT_ERR ("%s reading Intel fw version command failed (%ld)" , hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return PTR_ERR (skb); }if (skb->len !=sizeof (*ver)) {
@@ -1216,7 +1216,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)BT_ERR ("%s entering Intel manufacturer mode failed (%ld)" , hdev->name,PTR_ERR (skb));release_firmware (fw); -return -PTR_ERR (skb); +return PTR_ERR (skb); }if (skb->data[0 ]) {
@@ -1273,7 +1273,7 @@ static int btusb_setup_intel(struct hci_dev *hdev)if (IS_ERR (skb)) {BT_ERR ("%s exiting Intel manufacturer mode failed (%ld)" , hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return PTR_ERR (skb); }kfree_skb (skb);
@@ -1289,7 +1289,7 @@ exit_mfg_disable:if (IS_ERR (skb)) {BT_ERR ("%s exiting Intel manufacturer mode failed (%ld)" , hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return PTR_ERR (skb); }kfree_skb (skb);
@@ -1307,7 +1307,7 @@ exit_mfg_deactivate:if (IS_ERR (skb)) {BT_ERR ("%s exiting Intel manufacturer mode failed (%ld)" , hdev->name,PTR_ERR (skb)); -return -PTR_ERR (skb); +return PTR_ERR (skb); }kfree_skb (skb);
Signature · 1 line
1.8.3.2
Re: [PATCH v3] btusb: fix wrong use of PTR_ERR()
Hi Adam,
Quoted 33 lines · Adam Lee
PTR_ERR()returns a signed long type value which is limited byIS_ERR(), it must be a negative number whose range is [-MAX_ERRNO, 0).The bug here returns negative numbers as error codes, then check it by "if (ret < 0)", but -PTR_ERR() is actually positive. The wrong use here leads to failure as below, even panic.
[ 12.958920] Bluetooth: hci0 command 0xfc8e tx timeout [ 14.961765] Bluetooth: hci0 command 0xfc8e tx timeout [ 16.964688] Bluetooth: hci0 command 0xfc8e tx timeout [ 20.954501] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 22.957358] Bluetooth: hci0 command 0xfc8e tx timeout [ 30.948922] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 32.951780] Bluetooth: hci0 command 0xfc8e tx timeout [ 40.943359] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 42.946219] Bluetooth: hci0 command 0xfc8e tx timeout [ 50.937812] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 52.940670] Bluetooth: hci0 command 0xfc8e tx timeout [ 60.932236] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 62.935092] Bluetooth: hci0 command 0xfc8e tx timeout [ 70.926688] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 72.929545] Bluetooth: hci0 command 0xfc8e tx timeout [ 80.921111] Bluetooth: hci0 sending Intel patch command (0xfc8e) failed (-110) [ 82.923969] Bluetooth: hci0 command 0xfc2f tx timeout [ 90.915542] Bluetooth: hci0 sending Intel patch command (0xfc2f) failed (-110) [ 92.918406] Bluetooth: hci0 command 0xfc11 tx timeout [ 100.909955] Bluetooth: hci0 sending Intel patch command (0xfc11) failed (-110) [ 102.912858] Bluetooth: hci0 command 0xfc60 tx timeout [ 110.904394] Bluetooth: hci0 sending Intel patch command (0xfc60) failed (-110) [ 112.907293] Bluetooth: hci0 command 0xfc11 tx timeout [ 120.898831] Bluetooth: hci0 exiting Intel manufacturer mode failed (-110) [ 120.904757] bluetoothd[1030]: segfault at 4 ip 00007f8b2eb55236 sp 00007fff53ff6920 error 4 in bluetoothd[7f8b2eaff000+cb000]
Quoted 4 lines
Signed-off-by Adam Lee <[email protected]>--- drivers/bluetooth/btusb.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
Patch has been applied to bluetooth.git. Thanks.
Gustavo