Add Kernel Adress Sanitization support#191
Add Kernel Adress Sanitization support#191vladandrew wants to merge 36 commits intounikraft:stagingfrom
Conversation
nderjung
left a comment
There was a problem hiding this comment.
Hi @vladandrew, can you please rebase?
@danield20 once Vlad has rebased, can you approve with your signature,
Thanks! 🐒
9f95468 to
d9d5fbd
Compare
nderjung
left a comment
There was a problem hiding this comment.
Hey Vlad! Thanks for introducing this new library, it's pretty cool!
For this PR's archive and for future documentation, could you please add a paragraph or two on how KASAN works and how it will work in Unikraft.
Apart from the notes inline (mainly styling suggestions for better readability), I'm also receiving some compile-time warnings during its compilation:
warning: conflicting types for built-in function '__asan_load1_noabort'; expected
'void(void *)' [-Wbuiltin-declaration-mismatch]
244 | void __asan_load##size##_noabort(uintptr_t addr) \
| ^~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:266:1: note: in expansion of macro 'DEFINE_ASAN_LOAD_STORE'
266 | DEFINE_ASAN_LOAD_STORE(1);
| ^~~~~~~~~~~~~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:248:6: warning: conflicting types for built-in function '__asan_store1_noabort'; expected 'void(void *)' [-Wbuiltin-declaration-mismatch]
248 | void __asan_store##size##_noabort(uintptr_t addr) \
| ^~~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:266:1: note: in expansion of macro 'DEFINE_ASAN_LOAD_STORE'
266 | DEFINE_ASAN_LOAD_STORE(1);
| ^~~~~~~~~~~~~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:244:6: warning: conflicting types for built-in function '__asan_load2_noabort'; expected 'void(void *)' [-Wbuiltin-declaration-mismatch]
244 | void __asan_load##size##_noabort(uintptr_t addr) \
| ^~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:267:1: note: in expansion of macro 'DEFINE_ASAN_LOAD_STORE'
267 | DEFINE_ASAN_LOAD_STORE(2);
| ^~~~~~~~~~~~~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:248:6: warning: conflicting types for built-in function '__asan_store2_noabort'; expected 'void(void *)' [-Wbuiltin-declaration-mismatch]
248 | void __asan_store##size##_noabort(uintptr_t addr) \
| ^~~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:267:1: note: in expansion of macro 'DEFINE_ASAN_LOAD_STORE'
267 | DEFINE_ASAN_LOAD_STORE(2);
| ^~~~~~~~~~~~~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:244:6: warning: conflicting types for built-in function '__asan_load4_noabort'; expected 'void(void *)' [-Wbuiltin-declaration-mismatch]
244 | void __asan_load##size##_noabort(uintptr_t addr) \
| ^~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:268:1: note: in expansion of macro 'DEFINE_ASAN_LOAD_STORE'
268 | DEFINE_ASAN_LOAD_STORE(4);
| ^~~~~~~~~~~~~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:248:6: warning: conflicting types for built-in function '__asan_store4_noabort'; expected 'void(void *)' [-Wbuiltin-declaration-mismatch]
248 | void __asan_store##size##_noabort(uintptr_t addr) \
| ^~~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:268:1: note: in expansion of macro 'DEFINE_ASAN_LOAD_STORE'
268 | DEFINE_ASAN_LOAD_STORE(4);
| ^~~~~~~~~~~~~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:244:6: warning: conflicting types for built-in function '__asan_load8_noabort'; expected 'void(void *)' [-Wbuiltin-declaration-mismatch]
244 | void __asan_load##size##_noabort(uintptr_t addr) \
| ^~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:269:1: note: in expansion of macro 'DEFINE_ASAN_LOAD_STORE'
269 | DEFINE_ASAN_LOAD_STORE(8);
| ^~~~~~~~~~~~~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:248:6: warning: conflicting types for built-in function '__asan_store8_noabort'; expected 'void(void *)' [-Wbuiltin-declaration-mismatch]
248 | void __asan_store##size##_noabort(uintptr_t addr) \
| ^~~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:269:1: note: in expansion of macro 'DEFINE_ASAN_LOAD_STORE'
269 | DEFINE_ASAN_LOAD_STORE(8);
| ^~~~~~~~~~~~~~~~~~~~~~
/usr/src/unikraft/unikraft/lib/kasan/kasan.c:244:6: warning: conflicting types for built-in function '__asan_load16_noabort'; expected 'void(void *)' [-Wbuiltin-declaration-mismatch]
d9d5fbd to
3a4672a
Compare
|
Hi Alex, Thank you for the review, I have solved all your comments. Let me know if there is anything else. |
|
Ok I was able to compile the application against redis. It's the default settings plus Is this now expected behaviour? Before it would look like this (without When i re-compile with When i disable |
|
Hi Alex, In the first case, the behaviour seems right since KASAN detects an invalid access which may be related to the assertion failing in the default case. In the second case, I think that you need to allocate more memory to the VM because KASAN uses part of the heap for the shadow memory. PS: We have tested this implementation with Nginx and Redis for the FlexOS paper. |
When the host indicates support for TSO, enable it and accept large packets when the corresponding uknetbuf flag is set. Signed-off-by: Marco Schlumpp <marco@unikraft.io> Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com> Reviewed-by: Andrei Tatar <andrei@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: unikraft#1088
When the host indicates support for TSO, enable it and accept large packets when the corresponding uknetbuf flag is set. Signed-off-by: Marco Schlumpp <marco@unikraft.io> Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com> Reviewed-by: Andrei Tatar <andrei@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: unikraft#1088
When the host indicates support for TSO, enable it and accept large packets when the corresponding uknetbuf flag is set. Signed-off-by: Marco Schlumpp <marco@unikraft.io> Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com> Reviewed-by: Andrei Tatar <andrei@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: unikraft#1088
In the current form they impose very strong synchronization requirements, making it impossible to implement a more efficient read-write lock design. Most rwlocks sidestep this by either using a weaker `try_upgrade` variant or having a separate `lock_upgradable` function (allowing multiple readers but only one upgradable lock/writer lock). This removes these two functions for now to prevent new code relying on these functions. This change is breaking because it already was part of the previous release. Signed-off-by: Marco Schlumpp <marco@unikraft.io> Reviewed-by: Andrei Tatar <andrei@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: unikraft#1090
Although `checkpatch.pl` checks for commit message lines to not be longer than `75` characters, in the case of `Github`'s UI, a commit subject longer than `70` character results in its line overflowing into the next. E.g. ``` [Selector]/[Component]: Lorem ipsum dolor sit amet, consectetuer adipiscin ``` becomes ``` [Selector]/[Component]: Lorem ipsum dolor sit amet, consectetuer adip... ...iscin ``` Since this is not aesthetically pleasing, enforce a character limit for commit message subjects to not be longer than `70` characters. Signed-off-by: Sergiu Moga <sergiu@unikraft.io> Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com> Reviewed-by: Michalis Pappas <michalis@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: unikraft#1077
This code was imported and this occurrence of the type was not adjusted when the type name changed. Signed-off-by: Marco Schlumpp <marco@unikraft.io> Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com> Reviewed-by: Rares Miculescu <miculescur@gmail.com> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GiHub-Closes: unikraft#1076
This attribute allows to opt-out of the standard C strict aliasing rule. Signed-off-by: Marco Schlumpp <marco@unikraft.io> Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com> Reviewed-by: Rares Miculescu <miculescur@gmail.com> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GiHub-Closes: unikraft#1076
The macro violates the strict aliasing rules. By using the newly introduced may_alias attribute we can prevent miscompilations because of that. Signed-off-by: Marco Schlumpp <marco@unikraft.io> Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com> Reviewed-by: Rares Miculescu <miculescur@gmail.com> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GiHub-Closes: unikraft#1076
Since `start16` related symbols have a relocation mechanism similar, but entirely independent from `ukreloc`, rename all such symbols, macro's and their uses so that they do not contain the `uk_reloc` or the `UKRELOC` substrings. Signed-off-by: Sergiu Moga <sergiu@unikraft.io> Reviewed-by: Razvan Virtan <virtanrazvan@gmail.com> Reviewed-by: Marco Schlumpp <marco@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: unikraft#1113
Allow having the same relocation mechanism of the SIPI vector symbols be used regardless of whether `ukreloc` or `PIE` are enabled or not. With this commit, SMP initialization has now the same SIPI behavior regardless of `ukreloc`/`PIE`, thus dropping any conflicts/dependencies. Signed-off-by: Sergiu Moga <sergiu@unikraft.io> Reviewed-by: Razvan Virtan <virtanrazvan@gmail.com> Reviewed-by: Marco Schlumpp <marco@unikraft.io> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: unikraft#1113
This causes the PCI driver to not initialize on certain systems. The check should uses `==` instead. Signed-off-by: Kha Dinh <khadinh@g.skku.edu> Approved-by: Razvan Deaconescu <razvand@unikraft.io> Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com> Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com> GitHub-Closes: unikraft#1110
When setting `V=0`, we expect the verbosity level to be reduced, and only the build output to be printed, not the build commands. Set `Q = @` by default, in case `V != 0` is set, set `Q` appropriately. Signed-off-by: Stefan Jumarea <stefanjumarea02@gmail.com> GitHub-Fixes: unikraft#676 Reviewed-by: Razvan Virtan <virtanrazvan@gmail.com> Reviewed-by: Robert Kuban <robert.kuban@opensynergy.com> Reviewed-by: Maria Sfiraiala <maria.sfiraiala@gmail.com> Approved-by: Razvan Deaconescu <razvand@unikraft.io> GitHub-Closes: unikraft#1066
Currently `9pfs` returns 0 for any `ioctl` request (except for `TIOCGWINSZ`), which leads to multiple cases of files being interpreted as terminals. You can see this problem when running python scripts, since python uses `ioctl` calls to check if it should open up the interactive interpretor or execute a script. Same thing could happen for other applications that rely on `ioctl` operations being properly executed, so the best way to go would be return `ENOTSUP` (as is used to happen before commit 28d0edf). If the `ioctl` call always return `ENOTSUP`, the Ruby binary compatibility application will fail, since it tries to set O_ASYNC and breaks on error, even if it does not directly depends on that, so for now add a special case for that. Signed-off-by: Stefan Jumarea <stefanjumarea02@gmail.com> Reviewed-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Simon Kuenzer <simon@unikraft.io> GitHub-Closes: unikraft#1098
Some applications (e.g. python) will use `ioctl()` calls to check if an interactive interpretor should be started or if a file should be read. `9pfs` should respond to all terminal-related calls with an `ENOTTY` error, so applications know that they are not running in an interactive environment. In order to detect the ioctl request type, add a new macro, `IOCTL_CMD_ISTYPE(cmd, type)`, which will check the corresponding bytes from the request number (an enhanced version of the Linux `_IOC_TYPE`). Signed-off-by: Stefan Jumarea <stefanjumarea02@gmail.com> Reviewed-by: Sergiu Moga <sergiu@unikraft.io> Approved-by: Simon Kuenzer <simon@unikraft.io> GitHub-Closes: unikraft#1098
dbd073d to
f5d8779
Compare
We add an internal library for KASAN instrumentation. Currently, KASAN is available only under KVM x86_64. We use 16 MBits for the shadow memory and it's mapped after _end. This implementation is inspired from the mimiker operating system. Signed-off-by: Vlad-Andrei Badoiu <vlad_andrei.badoiu@upb.ro>
We reserve the first 16 MBytes of the heap for the shadow memory. Signed-off-by: Vlad-Andrei Badoiu <vlad_andrei.badoiu@upb.ro>
We disable KASAN for the KVM platform code. Signed-off-by: Vlad-Andrei Badoiu <vlad_andrei.badoiu@upb.ro>
Signed-off-by: Vlad-Andrei Badoiu <vlad_andrei.badoiu@upb.ro>
Signed-off-by: Vlad-Andrei Badoiu <vlad_andrei.badoiu@upb.ro>
We mark the memory as valid or invalid in the allocator. This is used by the instrumented code to check for faults in memory usage. Signed-off-by: Vlad-Andrei Badoiu <vlad_andrei.badoiu@upb.ro>
f5d8779 to
ee31c37
Compare
Kernel Address Sanitizer for Unikraft. Currently working only under KVM x86_64. We use the first part of the heap for the shadow memory(e.g. KASAN_MD_SHADOW_SIZE bytes are used for the shadow memory). We instrument the memory allocator to mark memory as either valid or invalid. We use the shadow memory to keep track of our marks.
Kasan instruments all memory accesses such that they are valid, if an memory region marked as invalid is accessed then we call
UK_CRASH~.uk_malloc_ifpagesis instrumented such that the allocated memory is marked as valid and can be accessed,kasan_mark_invalid` marks the memory as invalid and causes the kernel to crash with a helpful message. We add a global KASAN flag and disable KASAN for several internal libraries such as the allocator.Example of KASAN crash output when we do an out of bounds access of an array: