Ansification patch should not cause any problems, as there is no promotable types, so no risk of binary incompatibility. The second patch just removes some compile warnings due to "impossible" condition checks.
Created attachment 14822 [details] [review] 0001-libFS-ansification.patch
Created attachment 14823 [details] [review] 0002-Remove-some-always-true-comparisons-due-to-structure.patch
Applied the first patch, but I'm not comfortable removing checks for overflows, so I didn't apply the second one.
(In reply to comment #3) > Applied the first patch, but I'm not comfortable removing checks for > overflows, so I didn't apply the second one. Thanks. Actually patch2 should be reviewed to add a better/proper check. Only the test in src/FSGetCats.c:FSGetCatalogues(), testing field num_catalogs of a fsListCataloguesReply structure is actually testing a 32 bit field, that I think is 32 bits just for padding... The other tests are impossible conditions as the tested field is a CARD8, but probably the length field value should be calculated, and checked if match. It should be a function like rep.length == (rep.num_fields * some_size) + sizeof(some_struct) + (possible_padding) But looking at the code, it seens that an arbitrary amount of data can come after the "useful" data (probably to allow "extensions" transparently working), so it probably should just check if the "num_fields" has a sane value, and the request lenght isn't too small, or it would end up reading past the end of the memory holding the request. Someone with more knowledge could review it :-) As I believe the tests are to prevent attacks, and the test on a 32 bit field probably should check for a sane value, like USHORT_MAX on rep.length, or probably less, instead of SIZE_MAX / sizeof(char*), where size max is either UINT_MAX or ULONG_MAX.
Patch 1 already in git master. Newer gcc is not generating warnings about the "always true" check (probably because gcc is internally, silently "automatically" removing the impossible condition): if (<test1> && <unsigned char variable> >= UINTMAX / sizeof(unsigned int) && <test3>) so, should probably not apply patch 2, to avoid confusion about the reason of removing that check....
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.