MaskRay wrote: > > I really appreciate the suggestions. `alias-unused.cpp` and > > `alias-unused-win.cpp` contain test improvement that should be pre-commited > > once they look good enough. Then this PR can be changed to show the > > difference. > > On a separate note, I wanted to clarify that `-Wunused-function` false > > positives/negatives shouldn't be automatically considered security bugs. > > Categorizing all these warning improvements as "security bugs" would dilute > > the meaning of "security bugs" and make it harder to prioritize real > > vulnerabilities. > > ( > > ``` > > What is a GCC security bug? > > =========================== > > > > A security bug is one that threatens the security of a system or > > network, or might compromise the security of data stored on it. > > In the context of GCC, there are multiple ways in which this might > > happen and some common scenarios are detailed below. > > ``` > > > > > > > > > > > > > > > > > > > > > > > > ) > > (An attacker can easily bypass warnings: remove `-Werror` (uncommon in > > distros anyway), remove `-Wall` (which covers `-Wunused-function`, or use a > > pragma to disable `-Wunused-function` locally. ) > > The description contains an example about name mangling differences > > ([#87130 > > (files)](https://github.com/llvm/llvm-project/pull/87130/files#r1554029811)) > > and I mentioned that "This inconsistency makes alias/ifunc difficult to > > use in C++ with portability." > > ``` > > extern "C" { > > static void f0() {} > > // GCC: void g0() __attribute__((alias("_ZL2f0v"))); > > // Clang: void g0() __attribute__((alias("f0"))); > > } > > ``` > > > > > > > > > > > > > > > > > > > > > > > > I added microsoftDemangle tests to show the current behavior. Since the > > feature that demangles to the function name without parameters (f3 instead > > of f3(int)) appears to be missing, I cannot address -Wunused-function false > > positives for microsoftDemangle with reasonable time complexity. > > I don't believe we ARE trying to make that change to security definitions. > However, we are all being particularly security conscious of this patch > because the reporter is the person who JUST did the xz exploit over years. > IMO, this bug report (and the original C only fix) were used in part to help > cover his tracks, so being particularly careful here is paramount.
The description might derail from the primary purpose of the patch. The -Wunused-function false positive was not used in part to cover the issue. "Hans Jansen" claimed 5% improvement for his app that called the crc func and contributed that ifunc code. Lasse Collin's commit (https://git.tukaani.org/?p=xz.git;a=commit;h=a37a2763383e6c204fe878e1416dd35e7711d3a9), confirmed with the author on IRC, aimed to fix a configure issue with Clang -Wall. Lasse did not report the issue. "Jia Tan" did. The attacker did not need to care about the -Wunused-function false positive. They could remove `static` from the resolver function to avoid the -Wunused-function false positive. Anyhow, the backdoor doesn't target Clang (`if test "x$CC" != 'xgcc' > /dev/null`), which might be related to other codegen discrepancy the attacker did not intend to control. https://github.com/llvm/llvm-project/pull/87130 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
