nasa / FM

The Core Flight System (cFS) File Manager (FM) application.
Apache License 2.0
42 stars 24 forks source link

New GCC warnings causing build failure [-Werror=stringop-overflow=] #63

Closed thnkslprpt closed 1 year ago

thnkslprpt commented 1 year ago

Checklist

Describe the bug I believe these are newly triggered GCC warnings (treated as errors) that are now causing the standard FM Build + Run workflow to fail.

In function ‘strncpy’,
    inlined from ‘FM_ChildDirListPktCmd’ at /home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c:1227:25:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin___strncpy_chk’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c: In function ‘FM_ChildDirListPktCmd’:
/home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c:1221:35: note: length computed here
 1221 |                     EntryLength = strlen(OS_DIRENTRY_NAME(DirEntry));
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
In function ‘strncat’,
    inlined from ‘FM_ChildDirListPktCmd’ at /home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c:1234:25:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:136:10: error: ‘__builtin___strncat_chk’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=]
  136 |   return __builtin___strncat_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c: In function ‘FM_ChildDirListPktCmd’:
/home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c:1221:35: note: length computed here
 1221 |                     EntryLength = strlen(OS_DIRENTRY_NAME(DirEntry));
      |                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
In function ‘strncpy’,
    inlined from ‘FM_ChildDirListFileLoop’ at /home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c:1450:21:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error: ‘__builtin___strncpy_chk’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c: In function ‘FM_ChildDirListFileLoop’:
/home/runner/work/FM/FM/apps/fm/fsw/src/fm_child.c:1434:31: note: length computed here
 1434 |                 EntryLength = strlen(OS_DIRENTRY_NAME(DirEntry));
      |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

To Reproduce Run the Build + Run GitHub Action on the current main branch FM source code.

Expected behavior Build + Run workflow should run without errors.

Reporter Info Avi @thnkslprpt

jphickey commented 1 year ago

Looking at the code, it does indeed seem suspicious - but there is a length check against OS_MAX_PATH_LEN that prevents anything from overflowing a buffer:

https://github.com/nasa/FM/blob/23b792da6f8124c459492e9923cac1e3263d420a/fsw/src/fm_child.c#L1221-L1228

Recommendation: