On Mon, 2025-11-03 at 12:16 -0500, Stephen Gallagher wrote: > On Mon, Nov 3, 2025 at 11:03 AM Kamil Dudka <[email protected]> > wrote: > > > On Monday, 3 November 2025 16:30:49 CET David Malcolm wrote: > > > On Mon, 2025-11-03 at 10:15 -0500, David Malcolm wrote: > > > > On Mon, 2025-11-03 at 10:46 +0100, Kamil Dudka wrote: > > > > > On Wednesday, 29 October 2025 21:20:12 CET Stephen Gallagher > > > > > wrote: > > > > > > On Wed, Oct 29, 2025 at 6:00 AM Siteshwar Vashisht > > > > > > <[email protected]> > > > > > > wrote: > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > I am writing this message to get feedback from the > > > > > > > community on > > > > > > > new > > > > > > > findings by static analyzers in Critical Path Packages > > > > > > > that > > > > > > > have > > > > > > > changed in Fedora 44. > > > > > > > > > > > > > > TLDR: This report[1] contains a total of 47352 findings > > > > > > > and 843 > > > > > > > new > > > > > > > findings identified since Fedora 43. Please review the > > > > > > > report > > > > > > > and > > > > > > > provide feedback. False positives can now be recorded in > > > > > > > the > > > > > > > known-false-positives[5] repository. > > > > > > > > > > > > > > A mass scan was performed on the packages that have > > > > > > > changed in > > > > > > > Fedora > > > > > > > 44. This report[1] contains all the findings that have > > > > > > > been > > > > > > > identified > > > > > > > in the Critical Path Packages. Newly added findings since > > > > > > > Fedora > > > > > > > 43 > > > > > > > are listed under ‘+’ column and these should be > > > > > > > prioritized > > > > > > > while > > > > > > > reviewing the findings (and fixing them upstream). Not > > > > > > > all > > > > > > > findings > > > > > > > reported by OpenScanHub may be actual bugs, so please > > > > > > > verify > > > > > > > reported > > > > > > > findings before investing time into fixing or reporting > > > > > > > them. > > > > > > > We > > > > > > > have > > > > > > > used the current development version of GCC to perform > > > > > > > the > > > > > > > scans, > > > > > > > which may increase the likelihood of having false > > > > > > > positives in > > > > > > > the GCC > > > > > > > reports. > > > > > > > > > > > > > > False positives can now be recorded in the known-false- > > > > > > > positives[5] > > > > > > > repository. These findings are automatically suppressed > > > > > > > by > > > > > > > OpenScanHub > > > > > > > in scans that are triggered later. Also, you can filter > > > > > > > findings > > > > > > > with > > > > > > > the csgrep utility to make it easier to review reports > > > > > > > that may > > > > > > > contain a large amount of false positives. Examples of > > > > > > > csgrep > > > > > > > invocation are available on the Fedora wiki[4]. > > > > > > > > > > > > > > We hope this is helpful for the packages you maintain and > > > > > > > for > > > > > > > the > > > > > > > upstream projects. Questions can be asked on the > > > > > > > OpenScanHub > > > > > > > mailing > > > > > > > list[2]. If you want to see the full logs of the scans, > > > > > > > they > > > > > > > are > > > > > > > available on the tasks[3] page. User documentation for > > > > > > > performing > > > > > > > a > > > > > > > scan is available on the Fedora wiki[4]. > > > > > > > > > > > > > > Please keep the feedback on this thread constructive. > > > > > > > Thank > > > > > > > you! > > > > > > > > > > > > > > [1] > > > > > > > > > https://svashisht.fedorapeople.org/openscanhub/mass-scans/f44-28-Oct-2025/ > > > > > > > > > > > > > > [2] > > > > > > > > > https://lists.fedoraproject.org/archives/list/[email protected]/ > > > > > > > > > > > > > > [3] https://openscanhub.fedoraproject.org/task/ > > > > > > > > > > > > > > [4] https://fedoraproject.org/wiki/OpenScanHub > > > > > > > > > > > > > > [5] https://github.com/openscanhub/known-false-positives > > > > > > > > > > > > > > -- > > > > > > > > > > > > > > > > > > > > > > > > > I'm pretty sure > > > > > > > > https://svashisht.fedorapeople.org/openscanhub/mass-scans/f44-28-Oct-2025/sscg-4.0.0-1.fc44/added.html > > > > > > is a false-positive. The line that it claims "leaks" isn't > > > > > > an > > > > > > exit > > > > > > to the > > > > > > function and the memory is freed just a few lines later. > > > > > > I'm not > > > > > > sure why > > > > > > OSH thinks that there's a problem. The BIO_read() function > > > > > > from > > > > > > OpenSSL is > > > > > > essentially just a memcpy() into the buffer that was passed > > > > > > in. > > > > > > > > > > > > (FWIW, this is also only in a unit test; there's no impact > > > > > > to the > > > > > > actual > > > > > > delivered package.) > > > > > > > > > > I agree this is a false positive. It does not happen with > > > > > gcc- > > > > > 15.2.1-3.fc44. > > > > > It happens with gcc-latest-16.0.0- > > > > > 5.20250914git38666cbccff5.fc44 > > > > > from > > > > > COPR. > > > > > > > > > > Dave, any idea what went wrong? > > > > > > > > > > The original SARIF file produced by GCC can be downloaded > > > > > with the > > > > > following > > > > > command: > > > > > > > > > > curl -s > > > > > ' > > > > > https://openscanhub.fedoraproject.org/task/78401/log/sscg-4.0.0-1.fc > > > > > 44.tar.xz?format=raw' \ > > > > > | xz -cd \ > > > > > | tar -xv sscg-4.0.0-1.fc44/debug/raw- > > > > > results/builddir/gcc- > > > > > results/452-M3DZ.sarif > > > > > > > > $ sarif-replay sscg-4.0.0-1.fc44/debug/raw- > > > > results/builddir/gcc- > > > > results/452-M3DZ.sarif > > > > In function 'verify_name_constraints': > > > > ../test/create_cert_test.c:118:3: warning: leak of ‘ext_str’ [- > > > > Wanalyzer-malloc-leak] > > > > 38 | #include <openssl/x509.h> > > > > ...... > > > > 118 | BIO_read (bio, ext_str, ext_len); > > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > 'verify_name_constraints': events 1-18 > > > > 61 | if (ext_idx < 0) > > > > | ^ > > > > | | > > > > | (1) following ‘false’ branch... > > > > ...... > > > > 69 | name_constraints_ext = X509_get_ext (x509, ext_idx); > > > > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > | | > > > > | (2) ...to here > > > > 70 | if (!name_constraints_ext) > > > > | ~ > > > > | | > > > > | (3) following ‘false’ branch... > > > > ...... > > > > 77 | ext_data = X509_EXTENSION_get_data > > > > (name_constraints_ext); > > > > | > > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > | | > > > > | (4) ...to here > > > > 78 | if (!ext_data) > > > > | ~ > > > > | | > > > > | (5) following ‘false’ branch... > > > > ...... > > > > 85 | bio = BIO_new (BIO_s_mem ()); > > > > | ~~~~~~~~~~~~~~~~~~~~~~ > > > > | | > > > > | (6) ...to here > > > > 86 | if (!bio) > > > > | ~Is there a way > > > > | | > > > > | (7) following ‘false’ branch... > > > > ...... > > > > 93 | if (!X509V3_EXT_print (bio, name_constraints_ext, 0, > > > > 0)) > > > > | ~ > > > > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > | | | > > > > | | (8) ...to here > > > > | (9) following ‘false’ branch... > > > > ...... > > > > 101 | ext_len = BIO_pending (bio); > > > > | ~ > > > > | | > > > > | (10) ...to here > > > > 102 | if (ext_len <= 0) > > > > | ~ > > > > | | > > > > | (11) following ‘false’ branch (when ‘ext_len > > > > > 0’)... > > > > ...... > > > > 109 | ext_str = malloc (ext_len + 1); > > > > | ~~~~~~~~~~~~~~~~~~~~ > > > > | | | > > > > | | (12) ...to here > > > > | (13) allocated here > > > > 110 | if (!ext_str) > > > > | ~ > > > > | | > > > > | (14) assuming ‘ext_str’ is non-NULL > > > > | (15) following ‘false’ branch (when ‘ext_str’ is > > > > non- > > > > NULL)... > > > > ...... > > > > 118 | BIO_read (bio, ext_str, ext_len); > > > > | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > | | > > > > | (16) ...to here > > > > | (17) if ‘BIO_read’ throws an exception... > > > > | (18) ‘ext_str’ leaks here; was allocated at (13) > > > > > > > > One of the things I've implemented for GCC 16 is exception- > > > > handling > > > > support in the analyzer. Looking at the above, GCC's - > > > > fanalyzer > > > > "thinks" that BIO_read could throw an exception, and sure, if > > > > it did, > > > > then that malloc at line 109 would leak. > > > > > > Interestingly, looking at > > > > > https://svashisht.fedorapeople.org/openscanhub/mass-scans/f44-28-Oct-2025/sscg-4.0.0-1.fc44/added.html > > > the test printing of the execution path there seems to have > > > events (1) > > > to (16) and (18), but not event (17) which is the important one > > > for > > > understanding what's going on. > > > > I guess this is a deficiency of the SARIF reader in `csgrep`. I > > get the > > same > > output with csgrep locally: > > > > % csgrep > > sscg-4.0.0-1.fc44/debug/raw-results/builddir/gcc-results/452- > > M3DZ.sarif > > > > > In "Scan Properties" I see: > > > analyzer-version-gcc: 15.2.1 > > > analyzer-version-gcc-analyzer: 16.0.0 > > > > > > I don't understand the distinction here; could it be showing gcc > > > 15 > > > results? > > > > This means that csmock used `gcc-15.x` as the system compiler to > > drive > > the native build and, for each compiler invocation, it ran `gcc- > > 16.x` > > with `-fanalyzer` in a separate process to capture the > > corresponding > > SARIF file. > > > > > IMHO sarif-replay's way of printing this is much more readable > > > than > > > what added.html is currently showing. > > > > Yes, it is. > > > > > Is it possible to use sarif-replay there? (I can help if you > > > like). > > > > Unfortunately, this is not going to be easy because csmock does not > > use > > SARIF internally. It merges diagnostic output from all the enabled > > tools > > into a simple JSON format that throws away a lot of details. > > > > In theory, csmock could be reworked to use SARIF internally but it > > would > > not be a smooth transition and it would introduce a performance > > penalty > > on scans with a lot of findings in the results. > > > > > But note that GCC 16 and its > > > sarif-replay also has HTML output now, as per: > > > https://dmalcolm.fedorapeople.org/gcc/2025-11-03/452-M3DZ.html > > > below (and potentially can show extra debugging info when > > > stepping > > > though the code with "j" and "k" if you supply "state-graphs=yes" > > > when > > > adding the SARIF sink): > > > -fdiagnostics-add-output=sarif:state-graphs=yes > > > see the example at: > > > https://dmalcolm.fedorapeople.org/gcc/2025-06-23/state-diagram-1.c.html > > > > This is cool. I like the visualization. > > > > > Dave > > > > > > > > > > > Looking in the SARIF file, I see "-fexceptions" in the command- > > > > line > > > > flags. Is that deliberate? Is this code meant to support > > > > exceptions? > > > > Good point. Stephen, could you please comment on that? > > > > > Those are standard build flags put there by the %meson RPM macro > (they also > appear in the %configure macro). Which means that the `-fexceptions` > flag > is present on virtually every C and C++ package on Fedora. > > > $ rpmbuild --eval "%meson" > > > CFLAGS="${CFLAGS:--O2 -flto=auto -ffat-lto-objects -fexceptions -g > -grecord-gcc-switches -pipe -Wall -Werror=format-security > -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS > -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-pro > tector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 > -march=x86-64 -mtune=generic -fasynchronous-unwind-tables > -fstack-clash-protection -fcf-protection -mtls-dialect=gnu2 > -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer }" ; export CFL > AGS ;
[...snip...] Aha - thanks Stephen. Some notes here: * gcc <= 15 -fanalyzer will get confused on exception handling within a function and emit lots of false positives, but it's not clear to me that that's possible for functions generated from C code (specifically, it's confused by CFG edges with the "EH" flag) * gcc 16 -fanalyzer considers that any function that isn't explicitly marked as "nothrow" could throw, and considers that as a possible execution path through the code (here it would unwind the frame, and thus leak the malloc). Maybe there could be better heuristics here; could BIO_read or something it calls throw an exception? * it looks like csgrep is omitting the important event about the exception thus making the warning confusing to read (see elsewhere in the thread, and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122544 with which I'll try to patch things up from the gcc side) Hope this is constructive Dave -- _______________________________________________ devel mailing list -- [email protected] To unsubscribe send an email to [email protected] Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/[email protected] Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
