Serasidis / STM32_HID_Bootloader

Driverless USB HID bootloader and flashing tool for STM32F10X devices
418 stars 150 forks source link

Fix HID device open failure handling #30

Open matthijskooijman opened 4 years ago

matthijskooijman commented 4 years ago

When the ACM device could be opened to restart into the bootloader, but the HID device was detected but failed to open (for example because of a permission problem), this would not be detected and the code would segfault because handle was NULL.

The code that was supposed to handle a NULL return value from hid_open actually triggered only on a non-null value, and apparently also only when the "looking for hid device" loop above had reached i = 10 (in which case some error handling above would have bailed out already).

This changes the code to just check for a NULL return value, and bail out in that case.

matthijskooijman commented 4 years ago

In the Arduino IDE (with the Arduino_Core_STM32 core), this looked like this:

+-----------------------------------------------------------------------+
|         HID-Flash v2.2.1 - STM32 HID Bootloader Flash Tool            |
|     (c)      2018 - Bruno Freitas       http://www.brunofreitas.com   |
|     (c) 2018-2019 - Vassilis Serasidis  https://www.serasidis.gr      |
|   Customized for STM32duino ecosystem   https://www.stm32duino.com    |
+-----------------------------------------------------------------------+

> Trying to open the [ttyACM0]...
> Toggling DTR...
> Searching for [1209:BEBA] device...
##
> [1209:BEBA] device is found !
> Sending <reset pages> command...
An error occurred while uploading the sketch

I noticed the segfault in dmesg:

[364192.188296] hid-flash[12130]: segfault at c ip 0000000000404687 sp 00007fff108c5db0 error 4 in hid-flash[402000+4000]
[364192.188318] Code: b6 00 0f b6 c0 89 45 f0 c7 45 ec 00 00 00 00 83 7d f0 00 75 11 48 83 45 d0 01 48 83 6d c8 01 c7 45 ec 01 00 00 00 48 8b 45 d8 <8b> 40 0c 
85 c0 7f 6a 48 8b 45 c8 0f b7 c8 48 8b 45 d8 8b 40 14 0f
jalius commented 4 years ago

Hi. Any idea what permission may be missing? My device shows up properly in /dev/ttyACM0, but at hid_open there is a null handle.

rogerclarkmelbourne commented 4 years ago

@matthijskooijman

Does the F4 version build for you ?

I'm getting errors about missing headers :-(

BTW. This is nothing to do with your PR, but you seem to be the only one actively working on this, and I need to recompile it as I need to change the jump address to mimic another bootloader

matthijskooijman commented 4 years ago

@rogerclarkmelbourne, I haven't tried compiling the bootloader itself, sorry. I just used the prebuilt version and noticed a bug in the hid-flash utility.

but you seem to be the only one actively working on this

That's overstating things, I was just testing a little bit to see if the core changes I made did not break the HID bootloader upload, I'm really not working on the bootloader itself (and have little interest in doing so, we use the builtin hardware bootloader).

rogerclarkmelbourne commented 4 years ago

@matthijskooijman

No worries. I contacted @Serasidis and he has pushed and update to fix the problem.

jerkey commented 1 year ago

I don't understand, why hasn't this been merged yet?