Strings: remove unbounded memory access #1995
Merged
Conversation
This file contains 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
This change amends format strings in various places in order to remove unneeded "%s" format specifiers, e.g. when they can be replaced by "%c", or when err_msg() can be called instead of rprintf(). In addition, "%s" specifiers are being removed in a few places where there is no corresponding string in the function argument list. This is being done in preparation for the removal of unbounded string functions.
This macro was needed for using symbol functionalities from within klib code, before dynamic relocation of kernel symbols was implemented for klibs; now it's not needed anymore.
The "rprintf" and "intern" members of the syslog_cfg closure were needed in order for the syslog klib to be able to call those kernel functions, before dynamic relocation of kernel symbols was implemented for klibs; now they are not needed anymore.
This change amends the argument list in all calls to timm() that specify multiple tuple attributes, so that only the first attibute is specified in the timm() call, while the additional attributes are specified via calls to timm_append(). This is done in preparation for the commit that will remove unbounded string functions, which will change the timm() implementation to support only one tuple attribute.
This change replaces a few timm() calls occurring on out of memory conditions with references to the `timm_oom` global tuple. This simplifies the logic in the Unix fault handler to check for out of memory errors, and removes one use of the (soon to be removed) buffer_compare_with_cstring_ci() function.
The name filed in the lwIP timer struct is only used in lwip_debug() calls, thus it is unused in non-debug builds. This change removes the name field from non-debug builds (so that unused strings are removed from the kernel binary) and refactors the timer code accordingly.
By using assert(), a stack trace (from which the problematic refcount can be identified) will be printed when the condition being checked is not verified. This change is being done in preparation for removing unbounded kernel string functions, which would require changing the argument list in the halt() call.
The existing check was not actually verifying that the offset and size of the section header is consistent with the size of the ELF file.
This changeset removes the use of string functions that access string memory without a limit (i.e. that rely on the presence of a string terminator to determine the end of a string). Instead, a new string type (sstring) is being defined; this type is a struct that includes a pointer to string memory and the length of the string; use of this type makes it unnecessary to look for the string terminator in order to determine the length of a string, and thus avoids unbounded access to string memory. C string literals (which by definition are NULL-terminated strings) are still allowed in the kernel code, but they are converted at compile time into sstring types, e.g. by using the ss() macro. Since it is still necessary for the kernel to be able to process NULL-terminated strings (for example, strings read by drivers from peripheral memory), the sstring_from_cstring() function is being defined: this function takes a memory pointer and a maximum length value, and returns an sstring built by parsing the memory (up to the maximum length) looking for the string terminator (if no terminator is found, the returned string length is the maximum length). The "%s" format specifier in printf-style functions now takes an sstring argument instead of a `char *`; the name field has been removed from closure structs, and the function handling the "%F" format specifier has been changed to retrieve a closure name from the kernel symbol table.
francescolavra
force-pushed
the
feature/sstring
branch
from
68b6f0e
to
364b517
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This changeset removes the use of string functions that access string memory without a limit (i.e. that rely on the presence of a string terminator to determine the end of a string). Instead, a new string type (sstring) is being defined; this type is a struct that includes a pointer to string memory and the length of the string; use of this type makes it unnecessary to look for the string terminator in order to determine the length of a string, and thus avoids unbounded access to string memory.
C string literals (which by definition are NULL-terminated strings) are still allowed in the kernel code, but they are converted at compile time into sstring types, e.g. by using the ss() macro.
Since it is still necessary for the kernel to be able to process NULL-terminated strings (for example, strings read by drivers from peripheral memory), the sstring_from_cstring() function is being defined: this function takes a memory pointer and a maximum length value, and returns an sstring built by parsing the memory (up to the maximum length) looking for the string terminator (if no terminator is found, the returned string length is the maximum length).
The "%s" format specifier in printf-style functions now takes an sstring argument instead of a
char *; the name field has been removed from closure structs, and the function handling the "%F" format specifier has been changed to retrieve a closure name from the kernel symbol table.Requires nanovms/lwip#11 and nanovms/mbedtls#3