On Mon, 2025-11-03 at 17:03 +0100, Kamil Dudka 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 >
Interesting. Looking at 452-M3DZ.sarif I notice that almost all of threadFlowLocation objects in the path have a "kinds" property as per 3.38.8 kinds property: https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/sarif-v2.1.0-errata01-os-complete.html#_Toc141791009 but the one for "(17) if ‘BIO_read’ throws an exception..." *doesn't* have a "kinds" property. That's just a "MAY contain" property and gcc won't supply it if no properties are appropriate. Is csgrep perhaps ignoring threadFlowLocation objects that don't have a "kinds" property? Arguably it could be good to have property values in the SARIF spec for exception-handling; I've filed this here: https://github.com/oasis-tcs/sarif-spec/issues/735 > > 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. Right, but in the generated HTML it looks like you're invoking csgrep to read the SARIF and print it: presumably you could slot in sarif- replay and have that print it instead? Or am I misunderstanding something? > > 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? I've been advising people not to use -fanalyzer with GCC 15 and earlier on C++ code because of exception-handling problems. I suspect similar problems could arise on C code compiled with -fexceptions. Dave > > Kamil > > > > > > > FWIW I also made an HTML version of the above with: > > > $ sarif-replay \ > > > -fdiagnostics-add-output=experimental-html:file=452-M3DZ.html \ > > > sscg-4.0.0-1.fc44/debug/raw-results/builddir/gcc-results/452- > > > M3DZ.sarif > > > > > > and uploaded it here: > > > https://dmalcolm.fedorapeople.org/gcc/2025-11-03/452-M3DZ.html > > > Would it be useful if the mass scans provided this to package > > > maintainers? > > > (this HTML generation is new in GCC 16; you can use "j" and "k" > > > to > > > cycle through the events in the log) > > > > > > 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
