Re: [PATCH] btusb: fix overflow return values

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Resend 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 = 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

On Fri, Jul 5, 2013 at 10:37 AM, Yang Bai <hamo.by@xxxxxxxxx> wrote:
> 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
>
>
>
> On Thu, Jul 4, 2013 at 8:43 PM, Adam Lee <adam.lee@xxxxxxxxxxxxx> wrote:
>>
>> 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.
>>
>> Signed-off-by: Adam Lee <adam.lee@xxxxxxxxxxxxx>
>> ---
>>  drivers/bluetooth/btusb.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>> index 7a7e5f8..0fb2799 100644
>> --- a/drivers/bluetooth/btusb.c
>> +++ b/drivers/bluetooth/btusb.c
>> @@ -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);
>>
>> --
>> 1.8.3.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> 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.
>     """
>
> Chinese Name: 白杨
> Nick Name: Hamo
> Homepage: http://hamobai.com/
> GPG KEY ID: 0xA4691A33
> Key fingerprint = 09D5 2D78 8E2B 0995 CF8E  4331 33C4 3D24 A469 1A33



-- 
    """
    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 1A33
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux