ArduPilot / ArduPilot/ardupilot
ArduPilot: Firmware requests statistics for 65535 logs even if they don't exist
- Dominant language
- C++
- Stars
- 15.9k
- Forks
- 21.4k
- Avg merge
- 3d 17h
- Merged PRs (30d)
- 119
Description
## Bug report
**Issue summary**
Malformed MAVLink packet leads to firmware requesting size (`_get_log_size`) and time (`_get_log_time`) statistics for 65535 (max uint16_t) log files. Firmware keeps requesting statistics irrespective of log file existing or not.
**Issue details**
```
MAVLink packet: fd0500008effe675000000000000017166
Information:
Payload: LOG_REQUEST_LIST (117)
target_system (uint8_t): 1
target_component (uint8_t): 0
start (uint16_t): 0
end (uint16_t): 0
```
leads to
```cpp
_log_last_list_entry = 0
_log_next_list_entry = 1
```
in [AP_Logger::handle_log_request_list](https://github.com/ArduPilot/ardupilot/blob/502702df62572519b56971fe9fed5b2883640879/libraries/AP_Logger/AP_Logger_MAVLinkLogTransfer.cpp#L66). Later, [AP_Logger::handle_log_send_listing](https://github.com/ArduPilot/ardupilot/blob/502702df62572519b56971fe9fed5b2883640879/libraries/AP_Logger/AP_Logger_MAVLinkLogTransfer.cpp#L243) keeps calling `get_log_info` until `_log_next_list_entry == _log_last_list_entry`.
```cpp
if (_log_next_list_entry == 0) {
size = 0;
time_utc = 0;
} else {
get_log_info(_log_next_list_entry, size, time_utc);
}
...
...
if (_log_next_list_entry == _log_last_list_entry) {
transfer_activity = TransferActivity::IDLE;
_log_sending_link = nullptr;
} else {
_log_next_list_entry++;
}
```
`_log_next_list_entry` is incremented in steps of 1. So, the firmware requests statistics for log files until `_log_next_list_entry` wraps to 0 after 65535 increments. `_log_last_list_entry` and `_log_next_list_entry` are both `uint16_t`.
**Version**
ArduCopter V4.4.0 (502702df)
**Platform**
- [x] All
[ ] AntennaTracker
[ ] Copter
[ ] Plane
[ ] Rover
[ ] Submarine
**Airframe type**
Not applicable.
**Hardware type**
Board: omnibusF4
**Logs**
For example: 4 log files exist on my card. Mavproxy output with above packet:
```
Log 1 numLogs 4 lastLog 0 size 73728 Tue Jan 1 01:00:00 1980
Log 2 numLogs 4 lastLog 0 size 114688 Tue Jan 1 01:00:00 1980
Log 3 numLogs 4 lastLog 0 size 65536 Tue Jan 1 01:00:00 1980
Log 4 numLogs 4 lastLog 0 size 108004 Tue Jan 1 01:00:00 1980
Log 5 numLogs 4 lastLog 0 size 0
Log 6 numLogs 4 lastLog 0 size 0
Log 7 numLogs 4 lastLog 0 size 0
Log 8 numLogs 4 lastLog 0 size 0
Log 9 numLogs 4 lastLog 0 size 0
Log 10 numLogs 4 lastLog 0 size 0
Log 11 numLogs 4 lastLog 0 size 0
Log 12 numLogs 4 lastLog 0 size 0
Log 13 numLogs 4 lastLog 0 size 0
Log 14 numLogs 4 lastLog 0 size 0
Log 15 numLogs 4 lastLog 0 size 0
Log 16 numLogs 4 lastLog 0 size 0
Log 17 numLogs 4 lastLog 0 size 0
Log 18 numLogs 4 lastLog 0 size 0
Log 19 numLogs 4 lastLog 0 size 0
Log 20 numLogs 4 lastLog 0 size 0
Log 21 numLogs 4 lastLog 0 size 0
Log 22 numLogs 4 lastLog 0 size 0
Log 23 numLogs 4 lastLog 0 size 0
Log 24 numLogs 4 lastLog 0 size 0
Log 25 numLogs 4 lastLog 0 size 0
Log 26 numLogs 4 lastLog 0 size 0
Log 27 numLogs 4 lastLog 0 size 0
Log 28 numLogs 4 lastLog 0 size 0
Log 29 numLogs 4 lastLog 0 size 0
Log 30 numLogs 4 lastLog 0 size 0
Log 31 numLogs 4 lastLog 0 size 0
Log 32 numLogs 4 lastLog 0 size 0
Log 33 numLogs 4 lastLog 0 size 0
Log 34 numLogs 4 lastLog 0 size 0
Log 35 numLogs 4 lastLog 0 size 0
Log 36 numLogs 4 lastLog 0 size 0
...
...
...
...
Log 65501 numLogs 4 lastLog 0 size 0
Log 65502 numLogs 4 lastLog 0 size 0
Log 65503 numLogs 4 lastLog 0 size 0
Log 65504 numLogs 4 lastLog 0 size 0
Log 65505 numLogs 4 lastLog 0 size 0
Log 65506 numLogs 4 lastLog 0 size 0
Log 65507 numLogs 4 lastLog 0 size 0
Log 65508 numLogs 4 lastLog 0 size 0
Log 65509 numLogs 4 lastLog 0 size 0
Log 65510 numLogs 4 lastLog 0 size 0
Log 65511 numLogs 4 lastLog 0 size 0
Log 65512 numLogs 4 lastLog 0 size 0
Log 65513 numLogs 4 lastLog 0 size 0
Log 65514 numLogs 4 lastLog 0 size 0
Log 65515 numLogs 4 lastLog 0 size 0
Log 65516 numLogs 4 lastLog 0 size 0
Log 65517 numLogs 4 lastLog 0 size 0
Log 65518 numLogs 4 lastLog 0 size 0
Log 65519 numLogs 4 lastLog 0 size 0
Log 65520 numLogs 4 lastLog 0 size 0
Log 65521 numLogs 4 lastLog 0 size 0
Log 65522 numLogs 4 lastLog 0 size 0
Log 65523 numLogs 4 lastLog 0 size 0
Log 65524 numLogs 4 lastLog 0 size 0
Log 65525 numLogs 4 lastLog 0 size 0
...
```
Contributor guide
Research direction
Start in libraries/AP_Logger/AP_Logger_MAVLinkLogTransfer.cpp, reading AP_Logger::handle_log_request_list and handle_log_send_listing. Reproduce with the malformed LOG_REQUEST_LIST packet and inspect the uint16_t entry handling. Done means a request with start and end set to zero does not iterate through nonexistent log IDs up to 65535.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- embedded-iot
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100