nasa / CF

The Core Flight System (cFS) CFDP application.
Apache License 2.0
80 stars 45 forks source link

Segfault trying to dereference NULL pointer. #435

Open stephen-hunt opened 7 months ago

stephen-hunt commented 7 months ago

My team and I are using CF for our project. During development and testing, I hit a segfault. I already debugged it and am writing this to share with the open source community.

Describe the bug CF app experiences segfault.

Issue File: cf_cfdp.c Function: CF_CFDP_RecvIdle()

Within that function, there is a logical path where you can enter the else statement that states "R2 can handle missing metadata, so go ahead and create a temp file". The following then happens:

CF_CFDP_RecvIdle()--->else /* R2 can handle missing metadata...--->CF_CFDP_DispatchRecv(t, ph)--->CF_CFDP_R2_Recv(t)--->CF_CFDP_R2_SubstateRecvFileData(t)--->CF_CFDP_R2_Reset(t)--->CF_CFDP_R1_Reset(t)--->CF_CFDP_ResetTransaction(t)--->CF_FreeTransaction(t)--->Set Everything in t to 0 including pointers--->Come back out into the CF_CFDP_RecvIdle() function and continue--->if (t->state == CF_TxnState_IDLE){}(Of course it is, everything was just set to 0)--->CF_CFDP_ResetTransaction(t)--->if (t->history->dir == CF_Direction_TX)--->SEGFAULT because history (a pointer) was set to NULL by CF_FreeTransaction().

My Quick Fix For my project, I just created a quick fix in  CF_CFDP_RecvIdle().

if (t->history != NULL && t->state == CF_TxnState_IDLE) { /* state was not changed, so free the transaction */ CF_CFDP_ResetTransaction(t, 0); }

There may be a more elegant fix.

To Reproduce Not sure how to reproduce, it was a pretty specific path that we hit. Hopefully my description helps you artificially hit that path.

Expected behavior Code does not segfault. It just continues forward.

Code snips Mentioned above.

System observed on:

Additional context I will say I ported the app to run as a standalone application on my Linux machine, so it occurred outside the expected environment, but the logical path is still there. And the changes I made were minimal. The issue occurred in an area of code that I did not touch.

Reporter Info Stephen Hunt Turion Space