search

SmartThingsCommunity / st-device-sdk-c

SmartThings SDK for Direct Connected Devices for C
Other
118 stars 126 forks source link

Potential segmentation fault #62

Closed toddaustin07 closed 3 years ago

toddaustin07 commented 3 years ago

Hello! I was exchanging messages with Kwang-Hui on the SmartThings Community boards. I am working on a BSP for Raspberry Pi. In my testing so far I uncovered a condition which can cause a segmentation fault in the iot_easysetup_http module.

This is the condition: if an unexpected GET request is received and it does not contain an expected command, then line 414 will cause a segmentation fault due to attempting strlen() on 'payload', which does not get a value set in this case (it is a pointer with value 0).

Regarding progress on getting RPI working with the example app: I now have a SoftAP setup using hostapd and I can connect to it from my iphone. However, I have not been successful getting the ST mobile app to connect. I have confirmed that the SSID is configured to what you expect. Are there any other requirements I need to know about? I am running IOS 12.4.8 on iPhone6. Can it scan and find the SoftAP automatically while connected to my wireless router? Or do I have to manually change the wifi network? (I thought this was improved in more recent IOS versions) Does the ip address, wifi channel number, or auth mode matter?

I noticed that there are duplicate test devices showing in the ST mobile app even though there is only 1 in the developer workspace. Is this an issue? Also, I have seen mention in the documentation about the requirement of being a member of an organization for the APIs to work. I am an individual developer, however I was assign an mmId in the developer workspace. Could this be an issue?

If you would prefer that I use the community for these questions, I will post these questions there.

Thank you.

Todd Austin

Kwang-Hui commented 3 years ago

Hi @toddaustin07 , Thanks for report issue. The wrong usage of strlen() problem has fixed with https://github.com/SmartThingsCommunity/st-device-sdk-c/commit/ff6938c692bb2499b85eb159327faadb62f9d528#diff-8c0a6333d7c3674911908d993347d4c3e9355f20b806e633c7f22df0fdeda7d7 commit (it is still at develop branch. but will be merged master branch soon)

Regarding your problem on iPhone, iOS doesn't allow to get WiFi scanlist application privilege. ST app can't detect the existence of SoftAP device just like ST app at Android platform can do. So it is highly recommended to create QR code (Please refer https://github.com/SmartThingsCommunity/st-device-sdk-c/blob/master/doc/STDK_QR.md and great contribution from @grobebar https://github.com/SmartThingsCommunity/st-device-sdk-c/tree/master/tools/qrgen) and use ST app v1.6.51.XX or later.

  1. Turn on developer mode
  2. Select my testing device or scan QR
  3. ST app will ask user to get confirm connect to device's SoftAP.

Thank you.

Kwang-Hui Cho

toddaustin07 commented 3 years ago

Hi. I tried using the QR code when provisioning as you recommended, but I'm still unable to connect for some reason. I double-checked the SSID and it is correct. Here is the error screen I'm getting on the mobile app:

IMG_2335

I have version 1.6.52-437 of the ST mobile app. I can connect to the access point and use it normally through iphone settings, so I know Hostapd is working OK. But for some reason the ST app can't connect. The SoftAP is set up with SSID=Todds_RPI_Tes_E4fE4b0016q_iSl2FQ; broadcasting the SSID; wifi channel 6, open authorization (no password).

Any other thoughts?

Kwang-Hui commented 3 years ago

Hi @toddaustin07 ,

I think you can try with other SoftAP configuration refering https://github.com/SmartThingsCommunity/st-device-sdk-c/blob/56a509640153d7937d395e678a32c30ab9d34a0b/src/iot_api.c#L142

toddaustin07 commented 3 years ago

Yes! I changed authentication and used that password and now it connects.

But why is this password hardcoded in the SDK?? How would device manufacturers know to use it? I would suggest this requirement needs to be specified in the BSP porting documentation.

Thank you!

toddaustin07 commented 3 years ago

OK, next question (thank you for being patient with me - I'm making progress...)

There are several provisioning data files that the SDK appears to require on local storage: WifiProvStatus, IotAPSSID, ServerPort, etc. I was assuming that I did not have to create or initialize these - that your code manages them. However, I'm running into a file open error for ./IotAPPASS. It appears the other files are getting created but not this one.

My configuration is that my wireless router here does NOT require a password. Is this causing a problem? I'm not talking about the SoftAP (I've gotten past those connection issues); I'm talking about the access point you connect to in station mode. I can select the AP from the list displayed on the mobile app, so I know it gets that far. And it knows password is not required for that AP because the password field is disabled/greyed-out.

But then the SDK core code fails with the error. WifiProvStatus=DONE, but the password file is not there. I assume that it should exist, but be blank for my situation (open authorization). Perhaps this is a bug?

Kwang-Hui commented 3 years ago

Yes! I changed authentication and used that password and now it connects.

But why is this password hardcoded in the SDK?? How would device manufacturers know to use it? I would suggest this requirement needs to be specified in the BSP porting documentation.

Thank you!

Hi @toddaustin07 ,

We expected that porting layer handle it with given parameters of iot_bsp_wifi_set_mode()

I would suggest this requirement needs to be specified in the BSP porting documentation.

Thanks for your suggestion.

toddaustin07 commented 3 years ago

Hi Kwang-Hui -

Wanted to let you know that I successfully completed the provisioning process and the example device app (switch) is now working on my RPI!

To get around the bug I reported last night I temporarily changed my wireless router authentication to require a password. But I would assume you should also support authentication mode=OPEN, so hope you can make a correction to the code and handle the case where there is no password. I was looking at iot-nv_data.c and figured either to change the set wifi provisioning function to write a blank file if password length is 0, or have the get function ignore a missing password file and set it to null. But I wasn't sure of implications to the rest of the code, so I didn't try to change anything myself.

Kwang-Hui commented 3 years ago

Hi Kwang-Hui -

Wanted to let you know that I successfully completed the provisioning process and the example device app (switch) is now working on my RPI!

To get around the bug I reported last night I temporarily changed my wireless router authentication to require a password. But I would assume you should also support authentication mode=OPEN, so hope you can make a correction to the code and handle the case where there is no password. I was looking at iot-nv_data.c and figured either to change the set wifi provisioning function to write a blank file if password length is 0, or have the get function ignore a missing password file and set it to null. But I wasn't sure of implications to the rest of the code, so I didn't try to change anything myself.

Wow great. Congratulations!

I'll look into SDK code what you mentioned. Thanks again.

toddaustin07 commented 3 years ago

Hello again Kwang-Hui - I hope you are doing well. I am continuing to refine the wifi provisioning automation for my Raspberry Pi-based direct connect device. I have run into some error conditions that I'm trying to figure out, but I need some clarification on something: Can you please explain in detail what is expected when Scan mode is requested? What are the specific tasks you expect of the wifi module and what are the inputs and outputs? I don't think I'm clear on this and how it is related to the get_scan_result function which is separately called. I've looked at the esp32 code but it is equally unclear what "scan mode" does and how it is different from getting the scan results. The esp32 code for Scan mode seems to start station mode and nothing more, so it is confusing to me.

gamsahabnida

Kwang-Hui commented 3 years ago

Hi, @toddaustin07 iot_bsp_wifi_get_scan_result expects to get wifi ssid scan result from iot device (Raspberry-Pi in this case) perspective which would be used as base information to show available wifi list while device on-boarding at ST mobile application. because iot_bsp_wifi_get_scan_result could be called during soft-ap mode, it should be implemented carefully. some of BSPs/Chipsets can't support WiFi scan during soft-ap mode. the port layer should return cached - which is collected while station mode - wifi scan list as a fallback. I'm not sure the BCM wifi chipset could support wifi scanning during soft-ap mode. I think you can figure it out. Good luck !

Kwang-Hui commented 3 years ago

Hi Kwang-Hui - Wanted to let you know that I successfully completed the provisioning process and the example device app (switch) is now working on my RPI! To get around the bug I reported last night I temporarily changed my wireless router authentication to require a password. But I would assume you should also support authentication mode=OPEN, so hope you can make a correction to the code and handle the case where there is no password. I was looking at iot-nv_data.c and figured either to change the set wifi provisioning function to write a blank file if password length is 0, or have the get function ignore a missing password file and set it to null. But I wasn't sure of implications to the rest of the code, so I didn't try to change anything myself.

Wow great. Congratulations!

I'll look into SDK code what you mentioned. Thanks again.

I also fixed the problem you reported. Please refer below commits. Thank you again.

toddaustin07 commented 3 years ago

Thanks for your reply! I think I'm pretty clear on the scan_results function and I have that working fine. And yes, I understand that if you are in AP mode, it needs to be implemented carefully. But I'm still not clear what the purpose of a scan mode request is and what task it is supposed to perform. Is it simply to take the device out of AP mode so that a scan_results can subsequently be called?

And thank you for the fix to the open authorization bug!

Kwang-Hui commented 3 years ago

Thanks for your reply! I think I'm pretty clear on the scan_results function and I have that working fine. And yes, I understand that if you are in AP mode, it needs to be implemented carefully. But I'm still not clear what the purpose of a scan mode request is and what task it is supposed to perform. Is it simply to take the device out of AP mode so that a scan_results can subsequently be called?

And thank you for the fix to the open authorization bug!

Scan mode doesn't mean out of AP mode. it is changing WiFi supplicant to support Scanning and it depends on H/W's capability. Currently changing it to Scan mode at two points.

Mostly it isn't matter when scan mode is set at the beginning of provisioning. But called at during provisioning . it shouldn't stop soft AP mode. but make it possible to WiFi scan. if your H/W doesn't support wifi scan during soft AP mode. you can return cached scan result as alternatives.

Kwang-Hui commented 3 years ago

If you have further quires, Please open new issue. Thanks