ArduPilot / ArduPilot/ardupilot

ArduPilot: Firmware requests statistics for 65535 logs even if they don't exist

Open
#24,940 3 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.