SA Ptr, Memory Leak PR

[rjbrown2]

#184

The above pull request was submitted by kibnakamoto below. Unfortunately, we do not currently have a mechanism in place for outside contributor license. I'm moving this to an issue for tracking and completion until it is fixed, or external contributors licensing is available.

kibnakamoto commented on Jul 12
When I compiled using make, I got the following error:

[  1%] Building C object src/CMakeFiles/Crypto.dir/src_main/sadb_routine_inmemory.template.c.o
/home/kibnakamoto/workspace/NASA/CryptoLib/src/src_main/sadb_routine_inmemory.template.c: In function ‘sadb_get_sa_from_spi’:
/home/kibnakamoto/workspace/NASA/CryptoLib/src/src_main/sadb_routine_inmemory.template.c:384:12: error: the comparison will always evaluate as ‘false’ for the address of ‘sa’ will never be NULL [-Werror=address]
  384 |     if (sa == NULL)
      |            ^~
/home/kibnakamoto/workspace/NASA/CryptoLib/src/src_main/sadb_routine_inmemory.template.c:41:30: note: ‘sa’ declared here
   41 | static SecurityAssociation_t sa[NUM_SA];
      |                              ^~
/home/kibnakamoto/workspace/NASA/CryptoLib/src/src_main/sadb_routine_inmemory.template.c: In function ‘sadb_get_operational_sa_from_gvcid’:
/home/kibnakamoto/workspace/NASA/CryptoLib/src/src_main/sadb_routine_inmemory.template.c:419:12: error: the comparison will always evaluate as ‘false’ for the address of ‘sa’ will never be NULL [-Werror=address]
  419 |     if (sa == NULL)
      |            ^~
/home/kibnakamoto/workspace/NASA/CryptoLib/src/src_main/sadb_routine_inmemory.template.c:41:30: note: ‘sa’ declared here
   41 | static SecurityAssociation_t sa[NUM_SA];
      |  
                          ^~

To fix it, I changed the sa array into a pointer initialized with calloc, I freed it in the sadb_close() function.

This solves it because if sa is an array, comparing to NULL always returns false because array != NULL. So I converted the sa to pointer.

The second fix is:
I ran valgrind and saw that there was 14 bytes of memory definitely lost. This was caused by sa[12].arsn definition which was completely wrong to begin with.

Line 311:

sa[12].arsn = (uint8_t*) calloc(1, sa[1].arsn_len * sizeof(uint8_t));
To my understanding there are a few problems with this line, sa[1].arsn_len = 2, while sa[12].arsn_len = 0
I think this line was meant to be

sa[12].arsn = (uint8_t*) calloc(1, sa[12].arsn_len * sizeof(uint8_t));
but either way, this line causes a memory leakage that can be solved by not setting it to anything (so that sa[12].arsn = NULL).

Therefore, considering that sa[12].arsn_len = 0, then sa[12].arsn = NULL. This was correctly applied to sa[8] but not here.

Running tests...
Test project /home/kibnakamoto/workspace/NASA/CryptoLib
    Start 1: UT_TC_APPLY
1/8 Test #1: UT_TC_APPLY ......................   Passed    0.59 sec
    Start 2: UT_TC_PROCESS
2/8 Test #2: UT_TC_PROCESS ....................   Passed    0.32 sec
    Start 3: UT_CRYPTO_CONFIG
3/8 Test #3: UT_CRYPTO_CONFIG .................   Passed    0.00 sec
    Start 4: UT_CRYPTO
4/8 Test #4: UT_CRYPTO ........................   Passed    0.13 sec
    Start 5: UT_CRYPTO_AOS
5/8 Test #5: UT_CRYPTO_AOS ....................   Passed    0.00 sec
    Start 6: UT_CRYPTO_MC
6/8 Test #6: UT_CRYPTO_MC .....................   Passed    0.04 sec
    Start 7: UT_TM_APPLY
7/8 Test #7: UT_TM_APPLY ......................   Passed    0.28 sec
    Start 8: UT_TM_PROCESS
8/8 Test #8: UT_TM_PROCESS ....................   Passed    0.27 sec

100% tests passed, 0 tests failed out of 8

Total Test time (real) =   1.64 sec
All tests passed.