Xose has kindly informed me about the warnings from Fedora's static code analysis tool.
I'm sending a couple of related fixes here. Most of the warnings appear to be false positives though, see below (a second look by another eye pair would be appreciated). Regards Martin On Sat, 2025-05-03 at 23:29 +0200, Xose Vazquez Perez wrote: > Findings by static analyzers in Fedora 43 Critical Path Packages: > https://marc.info/?l=fedora-devel-list&m=174582743823723 > > device-mapper-multipath-0.11.1-1.fc43: > https://openscanhub.fedoraproject.org/task/51915/ > > device-mapper-multipath-0.11.1-1.fc43 List of Findings: > https://openscanhub.fedoraproject.org/task/51915/log/device-mapper-multipath-0.11.1-1.fc43/scan-results.html I'll leave the shellcheck errors in mpathconf to Ben. > Error: GCC_ANALYZER_WARNING (CWE-775): [#def42] > multipath-tools-0.11.1/kpartx/dasd.c:89:24: warning[-Wanalyzer-fd-leak]: leak > of file descriptor ‘fd_dasd’ > multipath-tools-0.11.1/kpartx/dasd.c:69:1: enter_function: entry to > ‘read_dasd_pt’ > ... > multipath-tools-0.11.1/kpartx/dasd.c:134:20: branch_true: following ‘true’ > branch (when ‘fd_dasd < 0’)... > branch_true: ...to here This one looks like a false positive to me. If fd_dasd < 0, it can't be leaked. > Error: GCC_ANALYZER_WARNING (CWE-476): [#def45] > multipath-tools-0.11.1/libmpathutil/parser.c:139:29: > warning[-Wanalyzer-null-dereference]: dereference of NULL ‘keyword’ False positive, too. VECTOR_SLOT won't return NULL here. We could add a test to make the compiler happy, but I don't quite see the point. > Error: GCC_ANALYZER_WARNING (CWE-457): [#def49] > multipath-tools-0.11.1/libmultipath/alias.c:201:31: > warning[-Wanalyzer-use-of-uninitialized-value]: use of uninitialized value > ‘bdg’ False positive. bdg won't be NULL here. > Error: GCC_ANALYZER_WARNING (CWE-465): [#def50] > multipath-tools-0.11.1/libmultipath/dmparser.c:31:12: > warning[-Wanalyzer-deref-before-check]: check of ‘p’ for NULL after already > dereferencing it False positive. *dst is non-NULL when we call merge_words(). We must double-check after calling realloc(). (That said, we should reimplement this using strbuf functions). > Error: GCC_ANALYZER_WARNING (CWE-476): [#def51] > multipath-tools-0.11.1/libmultipath/dmparser.c:436:25: > warning[-Wanalyzer-null-dereference]: dereference of NULL ‘pgp’ False positive. VECTOR_SLOT won't return NULL here. Same for the next 3 errors. > Error: GCC_ANALYZER_WARNING (CWE-476): [#def55] > multipath-tools-0.11.1/libmultipath/dmparser.c:501:33: > warning[-Wanalyzer-null-dereference]: dereference of NULL ‘pp’ Likewise. Same for the next 2 errors. > Error: GCC_ANALYZER_WARNING (CWE-476): [#def58] > multipath-tools-0.11.1/libmultipath/foreign/nvme.c:494:28: > warning[-Wanalyzer-null-dereference]: dereference of NULL ‘path’ The nvme_pathgroup is a member of struct nvme_path. nvme_pg_to_path() doesn't return NULL. Same for the next 2 errors in nvme.c > Error: GCC_ANALYZER_WARNING (CWE-476): [#def62] > multipath-tools-0.11.1/libmultipath/pgpolicies.c:191:17: > warning[-Wanalyzer-null-dereference]: dereference of NULL ‘pp1’ Again, false positives; VECTOR_SLOT() doesn't return NULL here. Same for the next 3 errors. > Error: GCC_ANALYZER_WARNING (CWE-401): [#def66] > multipath-tools-0.11.1/libmultipath/print.c:920:16: > warning[-Wanalyzer-malloc-leak]: leak of ‘alloc_path_layout()’ False positive. Strange one - gcc doesn't understand its own __attribute__((cleanup))? Or am I blind? Same for the next one. > Error: GCC_ANALYZER_WARNING (CWE-126): [#def70] multipath-tools-0.11.1/libmultipath/uevent.c:122:9: warning[-Wanalyzer-out-of-bounds]: stack-based buffer over-read This one is somewhat harder because it involves list handling macros, but I believe that our code is correct and that this is a false positive, too. > Error: GCC_ANALYZER_WARNING (CWE-775): [#def69] > multipath-tools-0.11.1/libmultipath/sysfs.c:309:22: > warning[-Wanalyzer-fd-leak]: leak of file descriptor ‘open(&pathbuf, 0)’ Again, false positive; there's a cleanup function for fd. > Error: GCC_ANALYZER_WARNING (CWE-775): [#def71] > multipath-tools-0.11.1/libmultipath/valid.c:208:34: > warning[-Wanalyzer-file-leak]: leak of FILE ‘fopen(&mountinfo, "r")’ False positive, gcc doesn't understand the cleanup_fclose(). Same for the next error. > Error: GCC_ANALYZER_WARNING (CWE-415): [#def73] > multipath-tools-0.11.1/multipathd/fpin_handlers.c:508:17: > warning[-Wanalyzer-double-free]: double-‘free’ of > ‘els_marginal_list_head.next + -2056’ I don't see an error in this code. Looks like another false positive to me. > Error: GCC_ANALYZER_WARNING: [#def74] > multipath-tools-0.11.1/multipathd/fpin_handlers.c:624:23: > warning[-Wanalyzer-fd-use-without-check]: ‘read’ on possibly invalid file > descriptor ‘fd’ False positive, too. fd == -1 is checked beforehand. > Error: GCC_ANALYZER_WARNING (CWE-688): [#def75] > multipath-tools-0.11.1/multipathd/main.c:3286:21: > warning[-Wanalyzer-null-argument]: use of NULL ‘new’ where non-null expected Similar to other false positives above, VECTOR_SLOT() won't return NULL in a loop like this. > Error: GCC_ANALYZER_WARNING (CWE-775): [#def77] > multipath-tools-0.11.1/multipathd/main.c:3973:12: > warning[-Wanalyzer-fd-leak]: leak of file descriptor ‘dup2(open("/dev/null", > 2), 0)’ Yet another false positive, the fd is closed in the cleanup handler. Same for the next 3 errors. > Error: GCC_ANALYZER_WARNING (CWE-476): [#def81] multipath-tools-0.11.1/multipathd/multipathc.c:97:30: warning[-Wanalyzer-null-dereference]: dereference of NULL ‘kw’ Another case of VECTOR_SLOT() false positive. Same for the next error. Regards Martin Martin Wilck (6): kpartx_id: fix shellcheck reported errors kpartx: fix file descriptor leak libmpathpersist: fix memory leak in mpath_prout_rel() libmpathutil: silence compiler warning in vector_del_slot() libmultipath: fix undefined behavior in 31-bit shift libmultipath: prioritizers/iet: fix possible NULL dereference kpartx/kpartx.c | 14 +++++++------- kpartx/kpartx_id | 8 ++++---- libmpathpersist/mpath_persist_int.c | 12 ++++++------ libmpathutil/vector.c | 16 ++++++++-------- libmultipath/nvme/nvme-ioctl.c | 2 +- libmultipath/prioritizers/iet.c | 3 +++ 6 files changed, 29 insertions(+), 26 deletions(-) -- 2.49.0
