Qualcomm Sec Protocol Addresses Lookup lib: Issues with uninitialized variables #479
Description
An issue was discovered while working on #476 related to the QcSecProtocolsLocatorSecLib code.
The code was directly ported from the standalone tool you previously used to run on your computer to set a per device value (the code of such tool can be found here: https://gist.github.com/sunflower2333/703d7af2523a351357ee0ff91fad4cbf
This tool initializes a basic data structure before starting its work, named FileContent, and for the sake of simplicity, here is the file loading and init code snipped out of above:
/**
* Get file size based on given fileContent.
*
* @param fileContent provide filePath, will also set fileSize in it.
* @retval 0 File does not exist.
* @retval size_t File size
*
*/
size_t get_file_size(FileContent* fileContent) {
FILE* pFile = fopen(fileContent->filePath, "r");
if (pFile == NULL) {
printf("Error: %s not found\n", fileContent->filePath);
return 0;
}
fseek(pFile, 0, SEEK_END);
size_t len = ftell(pFile);
fclose(pFile);
fileContent->fileSize = len;
fileContent->teSize = len - sizeof(EFI_TE_IMAGE_HEADER);
return len;
}The reason Im talking about this old tool code is because the code we have right now is pretty much this tool built right into our UEFI Sec Phase, theres however an issue that slipped through the net, while the code remains identical obviously the code to load the TE file isnt and had to be built to locate its address, thats done right, what however was forgotten, was to initiailize fileSize and teSize, and thats an issue, because of codes like these which depend on these values, and if they arent set, will result in an infinite loop and your phone hanging, which, can happen (there are valid cases where we cant find the addresses besides running on a Firmware/Chip not having these to begin with, we do not in fact control what existed before us afterall and memory can be rewritten):
This in turns means the code really isnt robust and can break at any point. We are lucky at the moment that some devices havent broke but this isnt the case for upcoming new devices to be added here.
Currently with QRD8250, I set a generous placeholder value and intend to merge this as a middle ground fix in general for this lib, however, we do want a proper way to get the file size.