if (nbytes The fix that was done was: if (nbytes > sizeof(*hwrpb)) But I think the correct fix is: if (copy_to_user(buffer, hwrpb, sizeof(*hwrpb)) != 0) It never makes sense to copy out of the hwrpb pointer any size other than sizeof(*hwrpb).
Improvements to static analysis in GCC 14
21–30 of 147 posts
Re: Improvements to static analysis in GCC 14
#22Very cool stuff! I haven't done much C development lately, so I'm curious how often `strcpy` and `strcat` are used. Last I checked they're almost as big no-nos as using goto. (Yes, I know goto is often preferred in kernel dev...) Can anyone share on how helpful the c-string analyses are to them?
> Last I checked they're almost as big no-nos as using goto. Huh? Why is goto a no-no? It is there for good reason. I think we all agree with Dijkstra that, in his words, unbridled gotos are harmful, but C's goto is most definitely bridled. I doubt any language created in the last 50+ years has unbridled gotos. That's an ancient programming technique that went out of fashion long ago (in large part because of Dijkstr…
Re: Improvements to static analysis in GCC 14
#23if (nbytes The fix that was done was: if (nbytes > sizeof(*hwrpb)) But I think the correct fix is: if (copy_to_user(buffer, hwrpb, sizeof(*hwrpb)) != 0) It never makes sense to copy out of the hwrpb pointer any size other than sizeof(*hwrpb).
Right, but the size of the buffer is given, it doesn't make sense to stomp over end of the callers buffer either, so you can't use pass in something longer than `nbytes` either.
if (nbytes
If the buffer isn't large enough to hold *hwrpb, then it already fails. The original check was good, only needed to change the amount of bytes copied to sizeof(*hwrpb).Re: Improvements to static analysis in GCC 14
#24Re: Improvements to static analysis in GCC 14
#25Very cool stuff! I haven't done much C development lately, so I'm curious how often `strcpy` and `strcat` are used. Last I checked they're almost as big no-nos as using goto. (Yes, I know goto is often preferred in kernel dev...) Can anyone share on how helpful the c-string analyses are to them?
The use of goto is unambiguously correct and elegant in some contexts. Unwavering avoidance of goto can lead to unnecessarily ugly, convoluted code that is difficult to maintain. It usually isn't common but it has valid uses. While use of functions like `strcpy` are less advisable, there are contexts in which they are guaranteed to be correct unless other strong (e.g. language-level) invariants are broken, in which c…
For C, absolutely. For C++, it's likely a footgun.
Re: Improvements to static analysis in GCC 14
#26I wish there was a better output format for the analysis, because this is hell for screen readers.
You can see an example of the output here: https://godbolt.org/z/aan6Kfxds (that's the first example from the article, with -fdiagnostics-format=sarif-stderr added to the command-line options)
I experimented with SVG output for the diagrams, but didn't get this in good enough shape for GCC 14.
Re: Improvements to static analysis in GCC 14
#27Very cool stuff! I haven't done much C development lately, so I'm curious how often `strcpy` and `strcat` are used. Last I checked they're almost as big no-nos as using goto. (Yes, I know goto is often preferred in kernel dev...) Can anyone share on how helpful the c-string analyses are to them?
> Last I checked they're almost as big no-nos as using goto. Huh? Why is goto a no-no? It is there for good reason. I think we all agree with Dijkstra that, in his words, unbridled gotos are harmful, but C's goto is most definitely bridled. I doubt any language created in the last 50+ years has unbridled gotos. That's an ancient programming technique that went out of fashion long ago (in large part because of Dijkstr…
Re: Improvements to static analysis in GCC 14
#28Earlier quoted context omitted.
There's nothing wrong with simple usages of goto. The strxcpy family on the other hand is complete garbage and should never be used for any reason. I'm horrified that they're used in the kernel at all. All of those functions (and every failed attempt at "fixing" them) should have been nuked from orbit.
What's wrong with `strncpy`?
I've seen a lot of code where people changed from `strcpy` to `strncpy` because they thought that was safety and security best practice. Even sometimes creating a new security vulnerability which wasn't there with `strcpy`.
`strncpy` does two unexpected things which lead to safety, security and performance issues, especially in large codebases where the destination buffers are passed to other code:
• `strncpy` does NOT zero-terminate the copied string if it limits the length.
Whatever is given the copied string in future is vulnerable to a buffer-read-overrun and junk characters appended to the string, unless the reader has specific knowledge of the buffer length and is strict about NOT treating it as a null-terminated string. That's unusual C, so it's rarely done correctly. It also doesn't show up in testing or normal use, if `strnlen` is "for safety" and nobody enters data that large.
• `strncpy` writes the entire destination buffer with zeros after the copied string.
Usually this isn't a safety and security problem, but it can be terrible for performace if large buffers are being used to ensure there's room for all likely input data.
I've seen these issues in large, commercial C code, with unfortunate effects:
The code had a security fault because under some circumstances, a password check would read characters after the end of a buffer due to lack of a zero-terminator, that authors over the years assumed would always be there.
A password change function could set the new password to something different than the user entered, so they couldn't login after.
The code was assumed to be "fast" because it was C, and avoided "slow" memory allocation and a string API when processing strings. It used preallocated char arrays all over the place to hold temporary strings and `strncpy` to "safely" copy. They were wrong: It would have run faster with a clean string API that did allocations (for multiple reasons, not just `strncpy`).
Those char arrays had the slight inconvenience of causing oddly mismatched string length limits in text fields all over the place. But it was worth it for performance, they thought. To avoid that being a real problem, buffers tended to be sized to be "larger" than any likely value, so buffer sizes like 256 or 1000, 10000 or other arbitrary lengths plucked at random depending on developer mood at the time, and mismatched between countless different places in the large codebase. `strncpy` was used to write to them.
Using `malloc`, or better a proper string object API, would have run much faster in real use, at the same time as being safer and cleaner code.
Even worse, sometimes strings would be appended in pieces, each time using `strncpy` with the remaining length of the destination buffer. That filled the destination with zeros repeatedly, for every few characters appended. Sometimes causing user-interactions that would take milliseconds if coded properly, to take minutes.
Ironically, even a slow scripting language like Python using ordinary string type would have probably run faster than the C application. (Also Python dictionaries would have been faster than the buggy C hash tables in that application which took O(n) lookup time, and SQLite database tables would have been faster, smaller and simpler than the slow and large C "optimised" data structures they used to store data).
Re: Improvements to static analysis in GCC 14
#29Very cool stuff! I haven't done much C development lately, so I'm curious how often `strcpy` and `strcat` are used. Last I checked they're almost as big no-nos as using goto. (Yes, I know goto is often preferred in kernel dev...) Can anyone share on how helpful the c-string analyses are to them?
I don't think so. Gotos are fine, strcat and strcpy without a malloc with the correct size in the same scope is a code smell.
Re: Improvements to static analysis in GCC 14
#30Earlier quoted context omitted.
> Last I checked they're almost as big no-nos as using goto. Huh? Why is goto a no-no? It is there for good reason. I think we all agree with Dijkstra that, in his words, unbridled gotos are harmful, but C's goto is most definitely bridled. I doubt any language created in the last 50+ years has unbridled gotos. That's an ancient programming technique that went out of fashion long ago (in large part because of Dijkstr…
Languages other than C give you options for flow control so that you don't need goto for that. It is a spectrum, if you only use goto to jump to the end of a small function on error it is okay, though I prefer something better in my language. I've seen 30,000 line functions with gotos used for flow control (loops and if branches) - something you can do in C if you are really that stupid and I think we will all agree…
The idiom `if (error) goto cleanup` is about the only thing I see goto used for. What flow control replaces that other than exceptions?