Fix the warnings gcc reports on 7.1 headers (maybe-uninitialized 74, format-truncation 5, frame-larger-than 1 -> 0) - #41
Open
iav wants to merge 7 commits into
Open
Conversation
On 7.1 headers copy_from_user() is checked through check_object_size() and gcc reports every uninitialised local target as -Wmaybe-uninitialized. Zero-initialising the buffers proves them defined and also guarantees the parsed string is terminated when the copy fills the buffer short of its end.
The write handlers copy count bytes into a fixed buffer and parse it as a string; a write of exactly sizeof(tmp) bytes filled the buffer with no terminator. Reject count >= sizeof(tmp) so the last byte stays zero.
The handler copied count bytes into a 255-byte stack buffer with no upper bound; every other write handler rejects oversized input first. Add the same check.
Every handler already bounds wrqu->length with rtw_do_mp_iwdata_len_chk() before the copy, so this only lets gcc prove the buffer defined for the subsequent parsing (-Wmaybe-uninitialized through check_object_size on 7.1 headers).
ielen is only assigned when the loop finds the element; the check after the loop tests p != NULL first, so the read is guarded in practice, but gcc cannot connect the two (-Wmaybe-uninitialized). Start from 0.
rtw_rf.c builds a printf format with %zu into a 16-byte buffer, which gcc cannot prove fits; size it to 32 and pass sizeof. The SDIO xmit thread name is "RTWHALXT-" plus the interface name (up to IFNAMSIZ), so 20 bytes may truncate; use 32.
struct cfg80211_roam_info is over a kilobyte on recent kernels and pushes the frame past the 1024-byte limit (-Wframe-larger-than). Allocate it for the duration of the cfg80211_roamed() call; if the allocation fails the roam is logged and not reported. The connect-result path is unaffected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Building the driver against 7.1 headers (arm64, Armbian meson64 config,
gcc 13) reports 80 warnings that 6.x kernels do not show:
scratch buffers (110) and reject count >= sizeof(tmp) instead of >
(141) - a write of exactly sizeof bytes left the string unterminated.
proc_set_pathb_phase copied with no upper bound at all into a 255-byte
stack buffer; it now checks like the others.
rtw_do_mp_iwdata_len_chk(); this only lets gcc prove the buffer defined.
the two).
("RTWHALXT-" + IFNAMSIZ).
for the cfg80211_roamed() call.
Result: -Wmaybe-uninitialized 74 -> 0, -Wformat-truncation 5 -> 0,
-Wframe-larger-than 1 -> 0, no errors, .ko links. -Wunused-function stays
at 0 on 7.1 with #30/#32.