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

add integer bound check #28790

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RamReso
Copy link

@RamReso RamReso commented Dec 1, 2024

When sending mavlink command_long messages with some arbitrary parameters, I noticed random crashes in the simulation. I looked into it and found, that the simulation always crashes, when I send a parameter >215 or <-215 for a coordinate. This was due to a integer over/underflow in the conversion from long to int.

I added the necessary checks to the converting function and now I do not longer notice the problem.

@tpwrules
Copy link
Contributor

tpwrules commented Dec 1, 2024

It is strange, but not impossible, that this should crash. What platform are you running on and what message are you trying to send?

The check needs to be moved to only be done if stores_location and after the multiplication (but before the cast).

@RamReso
Copy link
Author

RamReso commented Dec 2, 2024

I'm not 100% sure what you mean by platform. I'm only using a simulation which I start with:
python Tools/autotest/sim_vehicle.py -v ArduCopter -D

I encounter the problem if I send the command_long over mavlink using a self written python program with the following parameters:

target_system = 1
target_component = 1
command = 201 (MAV_CMD_DO_SET_ROI)
confirmation = 0
param_1= 841.8709017045903
param_2: -355.8708939554573
param_3: -487.38928814332417
param_4: -605.9047446662618
param_5: 409.52030363804556
param_6: -237.83399241328152
param_7: -691.756644619793 

These parameter values are random, but they should not lead to a crash.

I also noticed the same problems using different command, e.g. 195-MAV_CMD_DO_SET_ROI_LOCATION

The stack trace is the following:

convert_COMMAND_LONG_loc_param GCS_Common.cpp:5154
GCS_MAVLINK::convert_COMMAND_LONG_to_COMMAND_INT GCS_Common.cpp:5174
GCS_MAVLINK::try_command_long_as_command_int GCS_Common.cpp:5140
GCS_MAVLINK::handle_command_long GCS_Common.cpp:5195
GCS_MAVLINK::handle_message GCS_Common.cpp:4271
GCS_MAVLINK_Copter::handle_message GCS_Mavlink.cpp:1523
GCS_MAVLINK::packetReceived GCS_Common.cpp:1835
GCS_MAVLINK_Copter::packetReceived GCS_Mavlink.cpp:646
GCS_MAVLINK::update_receive GCS_Common.cpp:1887
GCS::update_receive GCS_Common.cpp:2707
Functor<void>::method_wrapper<GCS, &GCS::update_receive> functor.h:88
Functor::operator() functor.h:54
AP_Scheduler::run AP_Scheduler.cpp:264
AP_Scheduler::loop AP_Scheduler.cpp:393
AP_Vehicle::loop AP_Vehicle.cpp:544
HAL_SITL::run HAL_SITL_Class.cpp:289
main Copter.cpp:957

Thank you for your recommendation regarding the position of the check. I will adjust the code accordingly.

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

Successfully merging this pull request may close these issues.

2 participants