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?

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

Reply via email to