Having problems when trying to instrument all code of a specific UEFI driver (including the library code)


mick21@...
 

Hello everyone,

I hope this is the right place to ask this, I haven't been able to find answers online or in the edk2 wiki.

I'm trying to apply sanitizers to system management mode (SMM) drivers (VariableSmm.efi, for instance) in edk2, but I'm running into the problem that at the moment I am unable to instrument the library code that later gets linked with (the library code referenced in the .inf file). At the moment, when instrumenting a driver, for example VariableSmm.inf, I'm providing compilation flags via the [BuildOptions] section and then append the flags to the *_*_*_CC_FLAGS variable. This allows me to instrument the source files present under the [Sources] section. However, the library code under the [LibraryClasses] section that later gets linked to create the eventual .efi file is not instrumented, as it is already compiled separately before.

So the problem I have is that for certain UEFI drivers I want to instrument the library code, but for others not. So to clarify, for UEFI driver A, I want the library code compiled with "-fsanitize=memory", while for other UEFI drivers, I want to use the default compiled library code. I haven't found a clean way to do this in the building process. Is there anyone who has tips regarding this problem? I hope the problem is clear, if not, please indicate so.

Kind regards,
Mick


Andrew Fish
 

On Apr 9, 2021, at 7:55 AM, mick21@live.nl wrote:

Hello everyone,

I hope this is the right place to ask this, I haven't been able to find answers online or in the edk2 wiki.

I'm trying to apply sanitizers to system management mode (SMM) drivers (VariableSmm.efi, for instance) in edk2, but I'm running into the problem that at the moment I am unable to instrument the library code that later gets linked with (the library code referenced in the .inf file). At the moment, when instrumenting a driver, for example VariableSmm.inf, I'm providing compilation flags via the [BuildOptions] section and then append the flags to the *_*_*_CC_FLAGS variable. This allows me to instrument the source files present under the [Sources] section. However, the library code under the [LibraryClasses] section that later gets linked to create the eventual .efi file is not instrumented, as it is already compiled separately before.

So the problem I have is that for certain UEFI drivers I want to instrument the library code, but for others not. So to clarify, for UEFI driver A, I want the library code compiled with "-fsanitize=memory", while for other UEFI drivers, I want to use the default compiled library code. I haven't found a clean way to do this in the building process. Is there anyone who has tips regarding this problem? I hope the problem is clear, if not, please indicate so.
Mick,

If you add the --report-file=REPORTFILE to the build command when you compile it will generate a report about your build. I think the info in this report can help you out.
1) It ill show all the library instances that map to the [LibraryClassess] list in the driver INF file for every driver.
a) This shows you the mapping of the [LibraryClassess] to specific library instances.
b) This includes the list of [LibraryClasses] from the dependent library instances [LibraryClasses] section that got indirectly linked to resolve dependencies for the libraries, so the full set.
c) This list also shows you the library constructors and the order they are called.
2) The report file also will show you the per compiler CC_FLAGS and given what you are doing this may be helpful.

The makefiles are generated and live with the drivers build output under the Build dir so somethings looking at the generated makefiles is helpful. I generally find the log file is a lot easier than looking into the makefiles.

[1] $ . edksetup.sh
Loading previous configuration from /Volumes/Case/edk2-github/Conf/BuildEnv.sh
WORKSPACE: /Volumes/Case/edk2-github
EDK_TOOLS_PATH: /Volumes/Case/edk2-github/BaseTools
CONF_PATH: /Volumes/Case/edk2-github/Conf
$ build --help
Usage: build.exe [options] [all|fds|genc|genmake|clean|cleanall|cleanlib|modules|libraries|run]

Copyright (c) 2007 - 2018, Intel Corporation All rights reserved.

Options:
--version show program's version number and exit
-h, --help show this help message and exit
-a TARGETARCH, --arch=TARGETARCH
ARCHS is one of list: IA32, X64, ARM, AARCH64, RISCV64
or EBC, which overrides target.txt's TARGET_ARCH
definition. To specify more archs, please repeat this
option.
-p PLATFORMFILE, --platform=PLATFORMFILE
Build the platform specified by the DSC file name
argument, overriding target.txt's ACTIVE_PLATFORM
definition.
-m MODULEFILE, --module=MODULEFILE
Build the module specified by the INF file name
argument.
-b BUILDTARGET, --buildtarget=BUILDTARGET
Using the TARGET to build the platform, overriding
target.txt's TARGET definition.
-t TOOLCHAIN, --tagname=TOOLCHAIN
Using the Tool Chain Tagname to build the platform,
overriding target.txt's TOOL_CHAIN_TAG definition.
-x SKUID, --sku-id=SKUID
Using this name of SKU ID to build the platform,
overriding SKUID_IDENTIFIER in DSC file.
-n THREADNUMBER Build the platform using multi-threaded compiler. The
value overrides target.txt's
MAX_CONCURRENT_THREAD_NUMBER. When value is set to 0,
tool automatically detect number of processor threads,
set value to 1 means disable multi-thread build, and
set value to more than 1 means user specify the
threads number to build.
-f FDFFILE, --fdf=FDFFILE
The name of the FDF file to use, which overrides the
setting in the DSC file.
-r ROMIMAGE, --rom-image=ROMIMAGE
The name of FD to be generated. The name must be from
[FD] section in FDF file.
-i FVIMAGE, --fv-image=FVIMAGE
The name of FV to be generated. The name must be from
[FV] section in FDF file.
-C CAPNAME, --capsule-image=CAPNAME
The name of Capsule to be generated. The name must be
from [Capsule] section in FDF file.
-u, --skip-autogen Skip AutoGen step.
-e, --re-parse Re-parse all meta-data files.
-c, --case-insensitive
Don't check case of file name.
-w, --warning-as-error
Treat warning in tools as error.
-j LOGFILE, --log=LOGFILE
Put log in specified file as well as on console.
-s, --silent Make use of silent mode of (n)make.
-q, --quiet Disable all messages except FATAL ERRORS.
-v, --verbose Turn on verbose output with informational messages
printed, including library instances selected, final
dependency expression, and warning messages, etc.
-d DEBUG, --debug=DEBUG
Enable debug messages at specified level.
-D MACROS, --define=MACROS
Macro: "Name [= Value]".
-y REPORTFILE, --report-file=REPORTFILE
Create/overwrite the report to the specified filename.
-Y REPORTTYPE, --report-type=REPORTTYPE
Flags that control the type of build report to
generate. Must be one of: [PCD, LIBRARY, FLASH,
DEPEX, BUILD_FLAGS, FIXED_ADDRESS, HASH,
EXECUTION_ORDER]. To specify more than one flag,
repeat this option on the command line and the default
flag set is [PCD, LIBRARY, FLASH, DEPEX, HASH,
BUILD_FLAGS, FIXED_ADDRESS]
-F FLAG, --flag=FLAG Specify the specific option to parse EDK UNI file.
Must be one of: [-c, -s]. -c is for EDK framework UNI
file, and -s is for EDK UEFI UNI file. This option can
also be specified by setting *_*_*_BUILD_FLAGS in
[BuildOptions] section of platform DSC. If they are
both specified, this value will override the setting
in [BuildOptions] section of platform DSC.
-N, --no-cache Disable build cache mechanism
--conf=CONFDIRECTORY Specify the customized Conf directory.
--check-usage Check usage content of entries listed in INF file.
--ignore-sources Focus to a binary build and ignore all source files
--pcd=OPTIONPCD Set PCD value by command line. Format: "PcdName=Value"
-l COMMANDLENGTH, --cmd-len=COMMANDLENGTH
Specify the maximum line length of build command.
Default is 4096.
--hash Enable hash-based caching during build process.
--binary-destination=BINCACHEDEST
Generate a cache of binary files in the specified
directory.
--binary-source=BINCACHESOURCE
Consume a cache of binary files from the specified
directory.
--genfds-multi-thread
Enable GenFds multi thread to generate ffs file.
--no-genfds-multi-thread
Disable GenFds multi thread to generate ffs file.
--disable-include-path-check
Disable the include path check for outside of package.


Thanks,

Andrew Fish

Kind regards,
Mick





mick21@...
 

Hi Andrew,

Thank you for your reply, I didn't know this report option existed!

If you add the --report-file=REPORTFILE to the build command when you compile it will generate a report about your build. I think the info in this report can help you out.
I have looked at the output of the report file, though, it did provide some clarity on what libraries are included, it seemed to match with the "OUTPUT/static_library_files.lst" library listing that is created when a driver is built.

I found that I can change the file paths in the GNUmakefile of a UEFI driver (for example Build/OvmfX64/DEBUG_CLANGPDB/X64/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm/GNUmakefile). By changing the STATIC_LIBRARY_FILES or CC_FLAGS variables for a specific UEFI driver or library, I am maybe able to compile certain drivers with other library code than others, as the build script doesn't seem to overwrite the GNUmakefile file when I change it. I hope this will work, despite it being not so elegant.

Thank you,

Mick


Andrew Fish
 

Mick,

Are you trying to do runtime or static analysis? That would help guide my answers.

What your are are doing in the INF file you can generally do in the DSC file too per driver. There is the concept of override of things per single INF file entry in the DSC [1]. This syntax include <BuildOptions> and pointing at alternate libraries for just those drivers. If you have common libs that you need 2 flavors of you could fork a copy and point to those from the per driver entries in the DSC, for the drivers that you care about. Conceptually you could fork a DSC file, or !if def your DSC file to support multiple modes (normal and analysis) of build.

We added the overrides to the DSC file so your platform could override things without having to override the driver source (or INF) that you might have gotten from a vendor. Thinking act would be easier to maintain over the long run, and easier to merge changes from updates to the vendor code.


[1] https://github.com/tianocore/edk2/blob/master/OvmfPkg/OvmfPkgX64.dsc#L937

ShellPkg/Application/Shell/Shell.inf {
<LibraryClasses>
ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
!if $(NETWORK_IP6_ENABLE) == TRUE
NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
!endif
HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf

<PcdsFixedAtBuild>
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF
gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
}

Anyway always happy to help folks brainstorm on the easiest way to do things. Sometimes with the edk2 it is not always obvious….

Thanks,

Andrew Fish

On Apr 10, 2021, at 5:19 AM, mick21@live.nl wrote:

Hi Andrew,

Thank you for your reply, I didn't know this report option existed!

If you add the --report-file=REPORTFILE to the build command when you compile it will generate a report about your build. I think the info in this report can help you out.
I have looked at the output of the report file, though, it did provide some clarity on what libraries are included, it seemed to match with the "OUTPUT/static_library_files.lst" library listing that is created when a driver is built.

I found that I can change the file paths in the GNUmakefile of a UEFI driver (for example Build/OvmfX64/DEBUG_CLANGPDB/X64/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm/GNUmakefile). By changing the STATIC_LIBRARY_FILES or CC_FLAGS variables for a specific UEFI driver or library, I am maybe able to compile certain drivers with other library code than others, as the build script doesn't seem to overwrite the GNUmakefile file when I change it. I hope this will work, despite it being not so elegant.

Thank you,

Mick


mick21@...
 

Hi Andrew,

I'm trying to enable MemorySanitizer/AddressSanitizer for SMM drivers in TianoCore, so I will use it for dynamic analysis. To do this, I want to use LLVM to instrument the SMM drivers when they are built. but since the libraries are built separately and later linked with the compiled source files of the corresponding .inf file, these library files will not be instrumented.

I can now instrument the source files, for instance with "-fsanitize=memory", but the library code (LibraryClasses) for VariableSmm.efi will not be instrumented, as the library code is not recompiled for VariableSmm.efi, but only compiled once at the start and then linked repeatedly with the various drivers (this is what I assume). So, ideally, I would like to recompile the library code for VariableSmm.efi with "-fsanitize=memory", but only do this for VariableSmm.efi, not for other UEFI drivers.

There is the concept of override of things per single INF file entry in the DSC [1]. This syntax include <BuildOptions> and pointing at alternate libraries for just those drivers. If you have common libs that you need 2
flavors of you could fork a copy and point to those from the per driver entries in the DSC, for the drivers that you care about
I think this is what I need, but from your example, it seems that a library is not replaced, it is only appended (UefiShellNetwork2CommandsLib.inf is either included or not) or does the code you provide overwrite the whole LibraryClasses section for a specific driver? I think I'm misunderstanding something, but what you say seems to be what I'm looking for.

NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
!if $(NETWORK_IP6_ENABLE) == TRUE
NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
!endif
To be more specific, here either UefiShellNetwork2CommandsLib.inf is included or not, UefiShellNetwork2CommandsLib.inf does not overwrite a certain library, or does the overwriting of libraries (to use either library A or library B for the same functionality) happen somewhere else?

I hope I explained my problem clearly, if not, please say so! I also appreciate the brainstorming, it allows me to learn more about the project :).

Kind regards,

Mick


Andrew Fish
 

On Apr 10, 2021, at 1:31 PM, mick21@live.nl wrote:

Hi Andrew,

I'm trying to enable MemorySanitizer/AddressSanitizer for SMM drivers in TianoCore, so I will use it for dynamic analysis. To do this, I want to use LLVM to instrument the SMM drivers when they are built. but since the libraries are built separately and later linked with the compiled source files of the corresponding .inf file, these library files will not be instrumented.

I can now instrument the source files, for instance with "-fsanitize=memory", but the library code (LibraryClasses) for VariableSmm.efi will not be instrumented, as the library code is not recompiled for VariableSmm.efi, but only compiled once at the start and then linked repeatedly with the various drivers (this is what I assume). So, ideally, I would like to recompile the library code for VariableSmm.efi with "-fsanitize=memory", but only do this for VariableSmm.efi, not for other UEFI drivers.

There is the concept of override of things per single INF file entry in the DSC [1]. This syntax include <BuildOptions> and pointing at alternate libraries for just those drivers. If you have common libs that you need 2
flavors of you could fork a copy and point to those from the per driver entries in the DSC, for the drivers that you care about
I think this is what I need, but from your example, it seems that a library is not replaced, it is only appended (UefiShellNetwork2CommandsLib.inf is either included or not) or does the code you provide overwrite the whole LibraryClasses section for a specific driver? I think I'm misunderstanding something, but what you say seems to be what I'm looking for.

NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
!if $(NETWORK_IP6_ENABLE) == TRUE
NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
!endif
To be more specific, here either UefiShellNetwork2CommandsLib.inf is included or not, UefiShellNetwork2CommandsLib.inf does not overwrite a certain library, or does the overwriting of libraries (to use either library A or library B for the same functionality) happen somewhere else?

I hope I explained my problem clearly, if not, please say so! I also appreciate the brainstorming, it allows me to learn more about the project :).
Mick,

Thanks that is a clear explanation.

Sorry I picked a bad example as a NULL library class means force link the library even if it is not listed in the drivers INF file. Examples of NULL would be a compiler intrinsic lib, or a library (that calls another EFI or lib API) to register services.

You should be able to do something like

ShellPkg/Application/Shell/Shell.inf {
<LibraryClasses>
HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.sanitize.inf
}

So basically in every library instance you car about you clone the INF file to the *.sanitize.inf version and flip the compiler flags. Then you can point sanitized drivers at the sanitized lib. You can also use the NULL library if you need to inject some intrinsics for the sanitizer runtime (unless you just had it compile traps).

If you look in the Build output you will notice there are 2 levels that often have the same, or similar, name [1], I think you will find the 2nd name is the INF file name so it should be possible

[1] Build//OvmfX64/DEBUG_XCODE5/X64/OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib/


Thanks,

Andrew Fish

Kind regards,

Mick





mick21@...
 

Hi Andrew,

Apologies for the late reply!

ShellPkg/Application/Shell/Shell.inf {
<LibraryClasses>
HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.sanitize.inf
}
I think I have it working now, I'm overwriting all of the used libraries with the *.sanitizer.inf alternative, like you said, and add the BuildOptions there. I'm not sure whether this is the best option, but it is cleaner than what I had in mind and most importantly: it works :).

Thank you for your help, I really appreciate it.

Kind regards,

Mick


Andrew Fish
 

Feel free to post examples if it would help others.

On Apr 13, 2021, at 12:34 PM, mick21@live.nl wrote:

Hi Andrew,

Apologies for the late reply!

ShellPkg/Application/Shell/Shell.inf {
<LibraryClasses>
HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.sanitize.inf
}
I think I have it working now, I'm overwriting all of the used libraries with the *.sanitizer.inf alternative, like you said, and add the BuildOptions there. I'm not sure whether this is the best option, but it is cleaner than what I had in mind and most importantly: it works :).

Thank you for your help, I really appreciate it.

Kind regards,

Mick





mick21@...
 

Sure, currently I add the adjusted libraries like this:

MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf {
<LibraryClasses>
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.sanitizer.inf
TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.sanitizer.inf
ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.sanitizer.inf
MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.sanitizer.inf
...

Then I copy all the original library.inf to library.sanitizer.inf and then append my [BuildOptions] section, depending on the instrumentation. You can then check the static_library_files.lst in the build directory for your driver to see if all libraries are replaced with the sanitized ones. This is only "one layer" though, I'm not sure whether this matters for my implementation, but libraries can use other libraries, which should also be instrumented, that is not the case with this.


Andrew Fish
 

On Apr 13, 2021, at 2:38 PM, mick21@live.nl wrote:

Sure, currently I add the adjusted libraries like this:

MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf {
<LibraryClasses>
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.sanitizer.inf
TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.sanitizer.inf
ResetSystemLib|OvmfPkg/Library/ResetSystemLib/DxeResetSystemLib.sanitizer.inf
MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.sanitizer.inf
What compiler flags did you change to get the clang sanitizer working?

Thanks,

Andrew Fish

...

Then I copy all the original library.inf to library.sanitizer.inf and then append my [BuildOptions] section, depending on the instrumentation. You can then check the static_library_files.lst in the build directory for your driver to see if all libraries are replaced with the sanitized ones. This is only "one layer" though, I'm not sure whether this matters for my implementation, but libraries can use other libraries, which should also be instrumented, that is not the case with this.


mick21@...
 

For ASan and MSan I just add the relevant flags to CC_FLAGS and sometimes I have to add custom flags. I must mention that I enabled Windows as a target for MSan, this wasn't the case for my LLVM version.

[BuildOptions]
*_*_*_CC_FLAGS = -fsanitize=memory -mllvm -msan-smm-tianocore=1 -fsanitize-blacklist=/mnt/part5/edk2-msan/msan_blacklist.txt
For ASan I had to remove link-time optimizations, due to errors related to comdat sections which I could not fix. The more difficult part for me is setting up the shadow memory and implementing enough of the sanitizer runtime in order for it to work.

Also, some functions have other dependencies which are out of the instrumentation still, for instance [1]:

InternalSmmBase2->GetSmstLocation (InternalSmmBase2, &gSmst);
ASSERT (gSmst != NULL);
This initializes the gSmst variable, with the function SmmBase2GetSmstLocation() [2] in PiSmmIpl.c, and will now cause MSan to error on the ASSERT() call. I still have to work out these cases. That said, it is still very much a work in progress and it is all a bit hacky.

Kind regards,

Mick

[1] https://github.com/tianocore/edk2/blob/83876950ab3cf5278d0ae7542086bd4be75059d3/MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.c#L52
[2] https://github.com/tianocore/edk2/blob/83876950ab3cf5278d0ae7542086bd4be75059d3/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c#L77