Re: [PATCH v4 00/16] Start compiling with `-Wsign-compare`
- From
karthik nayak <karthik.188@gmail.com>
- Date
- Dec 6, 2024, 13:11 UTC
- Message-ID
- <CAOLa=ZSLEsfDLaH57enc81FCqLMPvgDj1+-fdsMHOVip5W2iLQ@mail.gmail.com>
- In-Reply-To
- <20241206-pks-sign-compare-v4-0-0344c6dfb219@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 22 quoted lines
> Hi, > > when compiling with DEVELOPER=YesPlease, we explicitly disable the > "-Wsign-compare" warning. This is mostly because our code base is full > of cases where we don't bother at all whether something should be signed > or unsigned, and enabling the warning would thus cause tons of warnings > to pop up. > > Unfortunately, disabling this warning also masks real issues. There have > been multiple CVEs in the Git project that would have been flagged by > this warning (e.g. CVE-2022-39260, CVE-2022-41903 and several fixes in > the vicinity of these CVEs). Furthermore, the final audit report by > X41 D-Sec, who are the ones who have discovered some of the CVEs, hinted > that it might be a good idea to become more strict in this context. > > Now simply enabling the warning globally does not fly due to the stated > reason above that we simply have too many sites where we use the wrong > integer types. Instead, this patch series introduces a new macro that > allows us to explicitly mark files that generate such warnings. Like > this, we can adapt the codebase over time and hopefully make this class > of vulnerabilities harder to land. >
I tested the changes with clang and GCC and everything seems good. I went through the changes themselves and have nothing to add.
Thanks
[snip]