Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors while using Zydis.lib in windows driver #16

Closed
pakt opened this issue Jul 28, 2017 · 20 comments
Closed

Errors while using Zydis.lib in windows driver #16

pakt opened this issue Jul 28, 2017 · 20 comments

Comments

@pakt
Copy link

pakt commented Jul 28, 2017

Quick fixes for:

  • stdint.h not found -> just add a copy from visual studio
  • unresolved external symbol __imp_wassert: redefine ZYDIS_ASSERT and ZYDIS_UNREACHABLE to ;
  • unresolved external symbol __imp___stdio_common_vsprintf: use winkernel_mm.* from capstone
  • missing DriverEntry / ALIGN errors -> see this issue
  • unresolved external symbol __imp___stdio_common_vsprintf: add /D _NO_CRT_STDIO_INLINE to cl options in Zydis project
@athre0z
Copy link
Member

athre0z commented Jul 28, 2017

Thanks, I'll take a look!

@pakt
Copy link
Author

pakt commented Jul 28, 2017

I'm using Visual Studio Community 2015, v14.

@athre0z
Copy link
Member

athre0z commented Jul 28, 2017

stdint.h and the assert should be easy fixes. Regarding vsprintf, we'll sooner or later just get rid of the dependencies on the CRT formatting functions. For now, I'll probably hack together something similar to what Capstone does.

The lack of DriverEntry, I wouldn't call that a bug. I mean, compiling Zydis to a driver on its own doesn't make a lot of sense. DriverEntry should be defined by whatever driver is using Zydis by statically linking against it. Correct me if I'm wrong, maybe I'm missing something, I don't have a lot of experience with Windows drivers.

@pakt
Copy link
Author

pakt commented Jul 28, 2017

Regarding DriverEntry: I had a driver with custom entry point (different than DriverEntry). Citing my capstone issue:

Using cs_malloc in your driver can trigger (KernelBufferOverflowLib); to be linked in. (KernelBufferOverflowLib); uses a wrapper for DriverEntry. If there is no DriverEntry defined in your code, linking fails. Solution is to use "DriverEntry" as your entrypoint (custom name will fail).

@athre0z
Copy link
Member

athre0z commented Jul 28, 2017

My WDK managed to corrupt itself during installation, I can't even build freshly created driver solutions. Thanks MS. I'll remove all Windows SDKs and WDKs I have installed, apply fresh ones and try again. May take a while. Pretty sure I'll end up trashing my entire VM, setting up a new one.

@pakt
Copy link
Author

pakt commented Jul 28, 2017

I followed "Build" instructions from here and it worked.

@athre0z
Copy link
Member

athre0z commented Jul 28, 2017

Alright, reinstalling SDKs and WDKs fixed my issue.

I have now implemented various ifdefs and a format function that I hope works based on RtlStringCchVPrintfEx. I'll commit to develop branch in a second. I haven't tried actually loading the driver since that's quite a hassle IIRC, so it'd be nice if you could report back if you encounter any further issues. You'll have to add ZYDIS_WINKERNEL and ZYDIS_ENABLE_FEATURE_DECODER preprocessor defines to your project.

I suppose you just C&Ped the source and include files into your project for now? I'm not quite sure what's the best way of supporting linkage against Windows drivers right now. CMake doesn't seem to support generating VS projects with the WDK toolset and manually maintaining VS projects isn't my favorite either.

This whole DriverEntry thing, I still don't think that's an issue on our side. If users decide to use non-standard EPs for their drivers, it's also their responsibility to deal with the resulting issues. I think that's more like an issue that I'd report to MS. In the end, that should rather be supported by KernelBufferOverflowLib, not by any lib that miiight possibly cause linkage against it.

@athre0z
Copy link
Member

athre0z commented Aug 11, 2017

I'll close this for now. If you encounter any other problems, feel free to create a new issue or to reopen this one!

@athre0z athre0z closed this as completed Aug 11, 2017
@Mattiwatti
Copy link
Contributor

ZYDIS_WINKERNEL can be inferred from _KERNEL_MODE, which MSVC sets for kernel mode projects (passing /kernel to cl.exe is enough for this, you don't need a full-blown WDK project).

Some other things I came across that may be of use (I am working on a Windows driver using Zydis at the moment, which is how I found this):

  • ZYDIS_ASSERT is defined to be empty if ZYDIS_WINKERNEL is defined, but this can be replaced with NT_ASSERT(condition), or NT_ASSERT_NOASSUME for asserts that should always execute. This does come at the cost of including <wdm.h>, but you're going to have a hard time compiling a driver without including wdm.h anyway.
  • The same goes for ZYDIS_UNREACHABLE: __assume(0) compiles in MSVC in both user and kernel mode. But maybe there's another reason for leaving this macro empty that I'm missing.
  • In CommonTypes.h, it is assumed that having ZYDIS_WINKERNEL without ZYDIS_MSVC is a faulty build configuration. However it has been possible to compile drivers with Clang/LLVM for quite some time :) (both Clang/C2 and the 'proper' LLVM toolchain.) It's not something I would recommend for production drivers, but Clang's static analysis is unparalleled so I make very frequent use of it. Unfortunately the clang-cl frontend does not recognise MSVC's /kernel switch, so you have to manually define _KERNEL_MODE. Naturally, doing this in an MSVC project results in a fatal error for defining a reserved macro. Sigh...
  • The 'correct' way to compile a static library intended for kernel mode use is to create a .vcxproj with PlatformToolset = WindowsKernelModeDriver10.0, DriverType = WDM and ConfigurationType = StaticLibrary. This obviously requires the WDK, is not remotely CMake-compatible and I have trouble believing it provides any actual benefit over simply adding /kernel to the compiler flags for a static library. But, if you try to link a driver against a static library that was not compiled with /kernel, the linker will flat out refuse the .lib as it cannot guarantee that it will work correctly in kernel mode. So that part does matter.

Re: DriverEntry: it is correct that the standard WDK build configuration hijacks the driver entry point to be GsDriverEntry in order to initialize stack cookie support. Counterintuitively it is not enough to disable stack cookies to actually get rid of the forced implant in your driver when you link against KernelBufferOverflowK.lib. So the solution is to (1) disable /GS cookie support, (2) remove KernelBufferOverflowK.lib from the linker inputs and (3) explicitly set the driver entry point to the standard DriverEntry in the linker options under 'advanced'. This will reduce your driver file size by a few KB free of charge, as well as make it easier to step through in a debugger. Prefast and Clang are much better ways to prevent stack overflows since they add no runtime overhead and they also don't have the BSOD/kernel panic + reboot cycle overhead that stack cookies do.

@athre0z
Copy link
Member

athre0z commented Nov 24, 2017

Thanks for the very informative post!

Our general design approach in regard of supporting more exotic build targets is not to implement shims for specific platforms, but to just make Zydis less dependant on the build environment in general. We already removed most dependencies on libc (e.g. sprint functions for formatting), to a level where only memcpy, memset and strlen are left, plus quite some stdint.h types. For those three functions, we'll probably add a ZYDIS_NO_LIBC switch, so that the libc ones are used when available (they have intrinsic optimization in all modern compilers that we don't want to miss if they are present) and custom ones if not. As a result, we'll not only be able to support Windows kernel drivers, but also UEFI drivers, Linux KOs, macOS Kexts, bootloaders, etc..

However, we're also not opposed to adding Windows driver specific switches as well after we achieved the generic no-libc support and your info will definitely come in handy for that!

@Mattiwatti
Copy link
Contributor

Yes, very true. So long as you minimise dependencies you can get C code to run on pretty much any platform. I don't have any Linux kernel mode experience, but I have written UEFI drivers in the past and even there (by "there" I mean in EDK2, which is what I used) you have some basic essentials such as printf/sprintf type functions and even a small library for an interactive shell. (Though of course they have to be named AsciiPrint, CatVSPrint and so on because it wouldn't be cool to use the same names everyone has been using since the 1970s.)

As for memset and co, I haven't done a very thorough benchmark or anything, but I was actually pleasantly surprised by ucrtbase.dll's implementations (you need >= 10.0.15063.0, it has a newer memcpy/memmove). It actually beat out my hand rolled AVX memcpy by a hair for sizes under 16 bytes (on bigger sizes mine still won handily of course, but that's to be expected.) On the whole I prefer having fewer dependencies though for the same reasons you mentioned. Zydis is already doing very well in this regard though, I've had to manually add a grand total of one snprintf declaration so far. Could be worse 😄

@Mattiwatti
Copy link
Contributor

Oh yeah, I'd be happy to contribute user/kernel mode Visual Studio project files if there is interest in it (meaning proper ones, not the garbage CMake generates). I know that the feelings w.r.t. doing this are mixed among CMake users, after all CMake can technically make Visual Studio project files so it is duplicating part of the build system. But non-VS users will never know the pain of having to hand-edit 15 XML files to 'un-CMake' them.

@athre0z
Copy link
Member

athre0z commented Nov 24, 2017

UEFI is a little special in that it actually does provide a near-complete libc when you link against edk2/StdLib/LibC (with the "right" function names). They needed that stuff themselves to get OpenSSL etc. running, which in turn they need for their crypto module. 😄

The snprintf reference is gone since v2.0-alpha2, on the latest develop only the 3 mentioned function remain (unless I missed any other when I quickly dropped our DLL into IDA). Like you mentioned, different than the formatting functions that even gave us a 50% formatting performance boost when we replaced them, these three are known to be very well optimized (and often even just replaced by compiler intrinsics if the size is known at translation time). That's why we will only use our own implementations when libc ones aren't present.

We wrote the majority of this lib using VS, with those generated VS files. Quite honestly, we even prefer those. I've been using VS for C/C++ development for the past 8 years or so and it has always been a huge pain in the ass to configure even trivial things like "hey, link me against that target". Manually configuring include/lib pathes for all combinations of Release/Debug/Win32/Win64 like it's 1999 is just ridiculous. Generally, VS' C/C++ project settings feel like they have been stuck in time compared to what you get for the .NET languages. If you have issues with the lack of source file filtering in the solution explorer, just add ZYDIS_DEV_MODE. CMake then tucks the files into VS filters according to the directory structure on disk.

Creating "real" VS files isn't much of an issue, maintaining them .. meh. Also, you always have that issue with what VS version to choose. No matter what you pick, people are going to be pissed. Too old? "You dinosaurs, use VS2017!" Too new? "How am I supposed to use that in my VS2008 enterprise code?!"

@Mattiwatti
Copy link
Contributor

Oh yes, it's hard to deny the VS project file system is part (or the main cause) of the problem. I actually think CMake is the closest thing to a "good" C/C++ cross-platform solution to the "how do I build this" problem we've ever had. But that doesn't mean I think its VS project output is acceptable, and I definitely don't agree that it is preferable to doing it the 1999 way. That's not CMake's fault per se, as it is tasked with doing something that is essentially impossible. But it does mean that every time I see an open source project use (only) CMake, I curse loudly and waste half an hour or so, depending on the size of the project, to unfuck it.

I'm also not saying adding a second build system is nirvana. In the best case scenario it adds a burden to the project developers because now they have to maintain another build system, and in the worst case scenario one of the two goes stale and unmaintained over time, which causes issues for users if they are trying method X to compile as described in the README and getting nowhere.

Still, maintaining a second build system is exactly what I do for many projects! (even ones I contribute to like x64dbg and ScyllaHide, because they use VS2013 and I use VS2017.) In those cases I create a local branch and have to pick from one of two evils, depending on what build system the project uses:

  • Edit the VS project files to suit my preferences, and deal with the occasional merge conflict, or
  • Generate the project files with CMake (meaning there are no originals to diff against), get rid of the CMake parts in them and edit them to suit my preferences, and then... update the project files by hand whenever I pull, by trying to divine what CMake would have produced from some change X to CMakeLists.txt in each commit since I last pulled from upstream.

The second approach is clearly insane. Yet I still prefer doing that to working with CMake directly. (To be clear, I'm still talking about VS - I have no issues at all with CMake on e.g. Linux with either GCC or Clang.)

The list of issues I've had with the combination of Visual Studio and CMake is endless. From the pointless build targets it adds, the missing (before) or terrible (now) support of custom build toolsets such as Clang/LLVM or the Intel compiler toolset, the fact that it generates hundreds of temporary files and directories seemingly at random when you hit build after making no changes at all, to completely regenerating and overwriting every single .vcxproj file in the solution (destroying my local changes) because of a syntax error in CMakeLists.txt. I could go on and on. But I think my opinion on this is clear, so I'll stop ranting now :P

Anyway, should you decide to add VS files at some point in the future regardless, I'd be happy to maintain them (just ping me whenever CMakeLists.txt gets updated or something - I'm sure this should be possible to automate). That's what I'd essentially be doing regardless, and it makes no difference to me whether my project files are on a local private branch or a public one.

@athre0z
Copy link
Member

athre0z commented Nov 26, 2017

Well, if you're willing to maintain that stuff, that's fine with us. If you'd ever decide to stop updating them however, we'd probably discard it again. I assume you'd have to create two distinct VS solutions for user and kernel-mode? Also, VS2017 ones? Feel free to drop a PR, we'll just bump it every time we change something in CMakeLists.txt that we think might affect you. I'd suggest to put that stuff into a directory called vs to avoid pollution of the root directory.

@athre0z
Copy link
Member

athre0z commented Nov 26, 2017

Also, on develop, ZYDIS_NO_LIBC is now replacing ZYDIS_WINKERNEL. I did a quick test with an OS X driver and everything seemed to be working fine so far. The switch enables custom (and very rudimentary) implementations of memset, memcpy and strlen and defines our fixed size integer types using compiler built-ins rather than stdint.h and stddef.h. Currently works with MSVC, Clang and GCC.

@Mattiwatti
Copy link
Contributor

Alright, cool. I suppose develop is the preferred branch for me to make a PR against? I've been using master up until now, but it's no problem to switch.

As for multiple VS solutions: I think it would be best to keep it to one solution, with multiple configurations:

  • Static user mode (the default)
  • Dynamic user mode
  • Static kernel mode (this would only compile the Zydis project with no tools/examples; or maybe a kernel mode example could be added that would be built if this configuration is selected)

The user mode configurations would be VS2017 with the v141 toolset. I don't think requiring VS2017 is an issue really, the Community edition is free after all. And it's still possible to open newer solution files with older VS versions and downgrade the toolset version if there is a need to use a specific VS version.

I'm not 100% sure what to do with the kernel mode configuration: looking at e.g. Capstone I see that they have a separate capstone_static_winkernel project file. But I'm not sure as to why that is. They do have one extra compilation unit in the kernel mode project, but that could have just as easily been achieved with the 'exclude from build' setting. (In general I would recommend against doing this at all though - it should be perfectly possible to use the same set of source files for all build configurations.)

I will have to do some testing w.r.t. this, but I'm fairly certain it's possible to create a hybrid user/kernel mode project that does not require the WDK unless kernel mode is selected. If not, then there will have to be separate zydis and zydis_kernel projects, but I'd like to avoid this if possible.

@Mattiwatti
Copy link
Contributor

An update on this: it is indeed possible to create a combined user (static/dynamic) and kernel mode project without causing issues for those who don't have the WDK installed. I currently have this set up and working on the develop branch, with the following solution configurations:

  • Debug / Release
  • Debug DLL / Release DLL
  • Debug Kernel / Release Kernel

The first two build Zydis using the user mode CRT and use either static or dynamic linking. The kernel mode option builds Zydis with the WDK toolset, the /kernel flag set and ZYDIS_NO_LIBC defined. I also have a ZydisWinKernel sample driver project that is compiled in this configuration for testing. The tools and examples are not built in the kernel mode configuration (technically they could be, but I can easily see that being confusing rather than helpful).

Does this sound OK? If so, I'll create a PR for it. (probably without the sample driver project to start, as I don't know how to go about writing the CMake equivalent of this.)

Oh yeah: I did have to remove the <stdint.h> #include from LibC.h in order to get ZYDIS_NO_LIBC to work in kernel mode, because the KM CRT does not have this header. But as far as I can tell this header doesn't need stdint.h anymore, because it now uses ZydisU8 and other such typedefs.

@athre0z
Copy link
Member

athre0z commented Nov 28, 2017 via email

@Mattiwatti
Copy link
Contributor

OK, PR added!

To make Zydis itself kernel mode compatible - yes, /kernel is technically all that is required. But realistically ZYDIS_NO_LIBC is also needed since we can assume that the project is being built with the WDK toolset. If not, that leads to some other issues due to user mode DLLs 'contaminating' the .lib file which will cause the linker to barf when linking a .sys file even though /kernel was set. This is why the obscurish 'omit default library name' (/Zl) flag is set in the MSVC projects for both user and kernel static builds - it prevents 'forwarding' of library names that the user of the library may not have. The line

vsnprintf(buf, size, format, args);

for example would normally cause some reference to vcruntime140.dll or ucrtbase.dll to be added to the .lib when compiled with a user mode toolset. Specifying /Zl means that this doesn't happen, and that the linker creating the final executable has to figure out where to get the function. This is good because vsnprintf is available in kernel mode as well, just from ntoskrnl.exe rather than a user mode DLL.

Using the full WDK toolchain is the most fool-proof way to ensure compability, because it does indeed change all of the include paths, standard linker inputs and so on. There are still some ways to check for zero dependencies though by building a minimal user mode program that does not link against anything other than Zydis. This is quite cumbersome to set up however. I think at that point you might be better off just installing the WDK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants