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

Fix nullptr dereference error in CallableFlow and MemoryConfigClient. #678

Merged
merged 1 commit into from
Nov 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/dcc/ProgrammingTrackBackend.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ public:
/// the short detector.
void notify_service_mode_ack()
{
if (!request())
if (!has_request())
{
return;
}
Expand All @@ -242,7 +242,7 @@ public:
/// Call this function when the service mode current limit is exceeded.
void notify_service_mode_short()
{
if (!request())
if (!has_request())
{
return;
}
Expand Down Expand Up @@ -362,7 +362,7 @@ private:
/// @param packet buffer to fill in with next packet to send.
void get_next_packet(unsigned code, dcc::Packet *packet) override
{
if (request() == nullptr)
if (!has_request())
{
packet->set_dcc_reset_all_decoders();
#ifdef DEBUG_PROGRAMTRACK_BACKEND
Expand Down
24 changes: 21 additions & 3 deletions src/executor/CallableFlow.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,27 @@ public:
protected:
using Action = StateFlowBase::Action;

/// @return the current request we are working on.
RequestType* request() {
return this->message() ? this->message()->data() : nullptr;
/// @return the current request we are working on. This function may be
/// called only if there is an active request. If unsure, use
/// {\link has_request() } to verify it first.
RequestType *request()
{
if (!this->message())
{
// This is not an assert macro, because this function gets inlined
// into a lot of places, and we want the shortest possible code
// size. However, letting this pass into a hard fault due to the
// nullptr dereference is extremely hard to debug.
abort();
}
return this->message()->data();
}

/// @return true if there is an active request, i.e., when request() is
/// allowed to be called.
bool has_request()
{
return this->message() != nullptr;
}

/// Terminates the flow and returns the request buffer to the caller with
Expand Down
4 changes: 4 additions & 0 deletions src/openlcb/MemoryConfigClient.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,10 @@ private:
private:
Action entry() override
{
if (!parent_->has_request())
{
return respond_reject(Defs::ERROR_OUT_OF_ORDER);
}
if (!parent_->node_->iface()->matching_node(
parent_->request()->dst, message()->data()->src))
{
Expand Down