Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HttpUpgrader crashes when tested with Host emulator #2724

Closed
mikee47 opened this issue Mar 7, 2024 · 0 comments
Closed

HttpUpgrader crashes when tested with Host emulator #2724

mikee47 opened this issue Mar 7, 2024 · 0 comments

Comments

@mikee47
Copy link
Contributor

mikee47 commented Mar 7, 2024

Further to #2720 I tested Basic_Ota with Host to further exercise the code logic. (It's necessary to comment-out COMPONENT_SOC in component.mk or add host.) In the test scenario neither rom0.bin nor rom1.bin exist so the ota server rejects the request. The application should fail gracefully, but it crashes:

Updating...
1011021 
Firmware download failed..
In callback...
Firmware update failed!
1011044 HTTP parser error: HPE_CB_message_complete

signal_handler: SIGSEGV - segmentation fault

Closed "out/Host/debug/firmware/flash.bin"
Goodbye!

Running make valgrind gives more detail:

Updating...
1047067 
Firmware download failed..
In callback...
Firmware update failed!
1055991 HTTP parser error: HPE_CB_message_complete
==1847804== Invalid read of size 4
==1847804==    at 0x80765AA: HttpClientConnection::onBody(char const*, unsigned int) (HttpClientConnection.cpp:224)
==1847804==    by 0x807E3E7: http_parser_execute (http_parser.c:2126)
==1847804==    by 0x8075B70: HttpConnection::onTcpReceive(TcpClient&, char*, int) (HttpConnection.cpp:179)
==1847804==    by 0x8078A55: operator() (std_function.h:591)
==1847804==    by 0x8078A55: TcpClient::onReceive(pbuf*) (TcpClient.cpp:150)
==1847804==    by 0x8078250: TcpConnection::internalOnReceive(pbuf*, signed char) (TcpConnection.cpp:484)
==1847804==    by 0x8057B90: tcp_input (in /tank/devcode/sming/samples/Basic_Ota/out/Host/debug/firmware/app)
==1847804==    by 0x8061E23: ip4_input (in /tank/devcode/sming/samples/Basic_Ota/out/Host/debug/firmware/app)
==1847804==    by 0x80632B9: ethernet_input (in /tank/devcode/sming/samples/Basic_Ota/out/Host/debug/firmware/app)
==1847804==    by 0x8063875: tapif_select (in /tank/devcode/sming/samples/Basic_Ota/out/Host/debug/firmware/app)
==1847804==    by 0x804DDD7: lwip_arch_service (lwip_arch.cpp:131)
==1847804==    by 0x804D7BE: host_lwip_init(lwip_param const&)::{lambda()#1}::_FUN() (host_lwip.cpp:84)
==1847804==    by 0x807A846: host_service_timers (os_timer.cpp:135)
==1847804==  Address 0x45d7500 is 0 bytes inside a block of size 44 free'd
==1847804==    at 0x4044ED4: operator delete(void*, unsigned int) (vg_replace_malloc.c:1095)
==1847804==    by 0x8067E27: Storage::PartitionStream::~PartitionStream() (PartitionStream.h:25)
==1847804==    by 0x80676AB: ~Item (HttpUpgrader.h:36)
==1847804==    by 0x80676AB: wiring_private::ObjectList<Ota::Network::HttpUpgrader::Item>::clear() (WiringList.h:112)
==1847804==    by 0x806782D: removeAllElements (WVector.h:177)
==1847804==    by 0x806782D: clear (WVector.h:170)
==1847804==    by 0x806782D: Ota::Network::HttpUpgrader::updateFailed() (HttpUpgrader.cpp:97)
==1847804==    by 0x806786E: Ota::Network::HttpUpgrader::itemComplete(HttpConnection&, bool) (HttpUpgrader.cpp:56)
==1847804==    by 0x80775BF: operator() (std_function.h:591)
==1847804==    by 0x80775BF: HttpClientConnection::onMessageComplete(http_parser*) (HttpClientConnection.cpp:123)
==1847804==    by 0x8075AA8: HttpConnection::staticOnMessageComplete(http_parser*) (HttpConnection.cpp:160)
==1847804==    by 0x807DA48: http_parser_execute (http_parser.c:1971)
==1847804==    by 0x8075B70: HttpConnection::onTcpReceive(TcpClient&, char*, int) (HttpConnection.cpp:179)
==1847804==    by 0x8078A55: operator() (std_function.h:591)
==1847804==    by 0x8078A55: TcpClient::onReceive(pbuf*) (TcpClient.cpp:150)
==1847804==    by 0x8078250: TcpConnection::internalOnReceive(pbuf*, signed char) (TcpConnection.cpp:484)
==1847804==    by 0x8057B90: tcp_input (in /tank/devcode/sming/samples/Basic_Ota/out/Host/debug/firmware/app)
==1847804==  Block was alloc'd at
==1847804==    at 0x4041D7D: operator new(unsigned int) (vg_replace_malloc.c:476)
==1847804==    by 0x804F11F: doUpgrade() (application.cpp:70)
==1847804==    by 0x804F450: serialCallBack(Stream&, char, unsigned short) (application.cpp:134)
==1847804==    by 0x806671E: operator() (std_function.h:591)
==1847804==    by 0x806671E: invokeCallbacks (HardwareSerial.cpp:113)
==1847804==    by 0x806671E: HardwareSerial::invokeCallbacks() (HardwareSerial.cpp:96)
==1847804==    by 0x807AD46: process (tasks.cpp:40)
==1847804==    by 0x807AD46: host_service_tasks (tasks.cpp:115)
==1847804==    by 0x804B50E: host_main_loop (startup.cpp:124)
==1847804==    by 0x804ACCF: main (startup.cpp:292)
==1847804== 

So the same problem exists. We need a cleaner way to hand full ownership of item streams to HttpRequest as the current scheme is too fragile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant