This provide option to control sanity of read and write operations
Both read and write instrumentation increase size of uImage, So using
this option read or write instrumentation can be avoided if not required.
Useful in case of module sanity, using this uImage sanity can be avoided.
Also user space ASAN provides this support for read/write instrumentation
control.
Signed-off-by: Vaneet narang <[email protected]>
Signed-off-by: Maninder Singh <[email protected]>
Reviewed-by: Ajeet Yadav <[email protected]>
---
v1 -> v2: Added Documentation for the same.
Documentation/dev-tools/kasan.rst | 16 ++++++++++++++++
lib/Kconfig.kasan | 16 ++++++++++++++++
scripts/Makefile.kasan | 4 ++++
3 files changed, 36 insertions(+)
diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
index f7a18f2..b8147df 100644
--- a/Documentation/dev-tools/kasan.rst
+++ b/Documentation/dev-tools/kasan.rst
@@ -40,6 +40,22 @@ similar to the following to the respective kernel Makefile:
KASAN_SANITIZE := n
+Control Over Read/Write Instrumentation of kernel::
+
+- To Disable Read Instrumentation of kernel with:
+
+ CONFIG_KASAN_READS = n
+
+Because in some cases we need to check only memory write sanitization
+for better performance, read instrumentation can be disabled.
+
+- To Disable Write Instrumentation of kernel with:
+
+ CONFIG_KASAN_WRITES = n
+
+In case when to instrument only external modules, not the entire kernel
+for read or write intrumentation or both.
+
Error reports
~~~~~~~~~~~~~
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index bd38aab..37d1de9 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -45,6 +45,22 @@ config KASAN_INLINE
endchoice
+config KASAN_READS
+ prompt "Read instrumentation"
+ bool
+ default y
+ depends on KASAN
+ help
+ This configuration controls the sanity of memory read.
+
+config KASAN_WRITES
+ prompt "Write instrumentation"
+ bool
+ default y
+ depends on KASAN
+ help
+ This configuration controls the sanity of memory write.
+
config TEST_KASAN
tristate "Module for testing kasan for bug detection"
depends on m && KASAN
diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
index 37323b0..a61b18e 100644
--- a/scripts/Makefile.kasan
+++ b/scripts/Makefile.kasan
@@ -29,3 +29,7 @@ else
endif
endif
endif
+
+CFLAGS_KASAN += $(call cc-option, \
+ $(if $(CONFIG_KASAN_READS),, --param asan-instrument-reads=0) \
+ $(if $(CONFIG_KASAN_WRITES),, --param asan-instrument-writes=0))
--
1.9.1
On Tue, Dec 13, 2016 at 6:57 AM, Maninder Singh <[email protected]> wrote:
> This provide option to control sanity of read and write operations
> Both read and write instrumentation increase size of uImage, So using
> this option read or write instrumentation can be avoided if not required.
> Useful in case of module sanity, using this uImage sanity can be avoided.
>
> Also user space ASAN provides this support for read/write instrumentation
> control.
>
> Signed-off-by: Vaneet narang <[email protected]>
> Signed-off-by: Maninder Singh <[email protected]>
> Reviewed-by: Ajeet Yadav <[email protected]>
> ---
> v1 -> v2: Added Documentation for the same.
>
> Documentation/dev-tools/kasan.rst | 16 ++++++++++++++++
> lib/Kconfig.kasan | 16 ++++++++++++++++
> scripts/Makefile.kasan | 4 ++++
> 3 files changed, 36 insertions(+)
>
> diff --git a/Documentation/dev-tools/kasan.rst b/Documentation/dev-tools/kasan.rst
> index f7a18f2..b8147df 100644
> --- a/Documentation/dev-tools/kasan.rst
> +++ b/Documentation/dev-tools/kasan.rst
> @@ -40,6 +40,22 @@ similar to the following to the respective kernel Makefile:
>
> KASAN_SANITIZE := n
>
> +Control Over Read/Write Instrumentation of kernel::
double colon at the end of line
> +
> +- To Disable Read Instrumentation of kernel with:
Strange choice of capital letters.
> +
> + CONFIG_KASAN_READS = n
Are configs ever set to = n? I can't find any cases in my .config file.
I thought configs are disabled with:
# CONFIG_KASAN_READS is not set
> +
> +Because in some cases we need to check only memory write sanitization
> +for better performance, read instrumentation can be disabled.
> +
> +- To Disable Write Instrumentation of kernel with:
I am not a native speaker but this looks malformed.
I would say either "Disable write instrumentation of kernel with" or
"To disable write instrumentation of kernel set"
> + CONFIG_KASAN_WRITES = n
> +
> +In case when to instrument only external modules, not the entire kernel
> +for read or write intrumentation or both.
> +
I propose something along these lines:
--- a/Documentation/dev-tools/kasan.rst
+++ b/Documentation/dev-tools/kasan.rst
@@ -40,6 +40,14 @@ similar to the following to the respective kernel Makefile:
KASAN_SANITIZE := n
+Sometimes it may be useful to disable instrumentation of reads, or writes
+or both for the entire kernel. For example, if binary size is a concern,
+it may be useful to disable instrumentation of reads to reduce binary size but
+still catch more harmful bugs on writes. Or, if one is interested only in
+sanitization of a particular module and performance is a concern, she can
+disable instrumentation of both reads and writes for kernel code.
+Instrumentation can be disabled with CONFIG_KASAN_READS and
CONFIG_KASAN_WRITES.
+
Error reports
On Tue, Dec 13, 2016 at 10:20 AM, Andrey Ryabinin
<[email protected]> wrote:
>
>
> On 12/13/2016 11:58 AM, Dmitry Vyukov wrote:
>
>> --- a/Documentation/dev-tools/kasan.rst
>> +++ b/Documentation/dev-tools/kasan.rst
>> @@ -40,6 +40,14 @@ similar to the following to the respective kernel Makefile:
>>
>> KASAN_SANITIZE := n
>>
>> +Sometimes it may be useful to disable instrumentation of reads, or writes
>> +or both for the entire kernel. For example, if binary size is a concern,
>> +it may be useful to disable instrumentation of reads to reduce binary size but
>> +still catch more harmful bugs on writes. Or, if one is interested only in
>> +sanitization of a particular module and performance is a concern, she can
>> +disable instrumentation of both reads and writes for kernel code.
>> +Instrumentation can be disabled with CONFIG_KASAN_READS and
>> CONFIG_KASAN_WRITES.
>> +
>
> I don't understand this. How this can be related to modules? Configs are global.
> You can't just disable/enable config per module.
Build everything without instrumentation. Then enable instrumentation
and do "make lib/test_kasan.ko".
Or build everything, copy out bzImage, change config, build everything again.
On 12/13/2016 11:58 AM, Dmitry Vyukov wrote:
> --- a/Documentation/dev-tools/kasan.rst
> +++ b/Documentation/dev-tools/kasan.rst
> @@ -40,6 +40,14 @@ similar to the following to the respective kernel Makefile:
>
> KASAN_SANITIZE := n
>
> +Sometimes it may be useful to disable instrumentation of reads, or writes
> +or both for the entire kernel. For example, if binary size is a concern,
> +it may be useful to disable instrumentation of reads to reduce binary size but
> +still catch more harmful bugs on writes. Or, if one is interested only in
> +sanitization of a particular module and performance is a concern, she can
> +disable instrumentation of both reads and writes for kernel code.
> +Instrumentation can be disabled with CONFIG_KASAN_READS and
> CONFIG_KASAN_WRITES.
> +
I don't understand this. How this can be related to modules? Configs are global.
You can't just disable/enable config per module.
> Error reports
>
On 12/13/2016 12:38 PM, Dmitry Vyukov wrote:
> On Tue, Dec 13, 2016 at 10:20 AM, Andrey Ryabinin
> <[email protected]> wrote:
>>
>>
>> On 12/13/2016 11:58 AM, Dmitry Vyukov wrote:
>>
>>> --- a/Documentation/dev-tools/kasan.rst
>>> +++ b/Documentation/dev-tools/kasan.rst
>>> @@ -40,6 +40,14 @@ similar to the following to the respective kernel Makefile:
>>>
>>> KASAN_SANITIZE := n
>>>
>>> +Sometimes it may be useful to disable instrumentation of reads, or writes
>>> +or both for the entire kernel. For example, if binary size is a concern,
>>> +it may be useful to disable instrumentation of reads to reduce binary size but
>>> +still catch more harmful bugs on writes. Or, if one is interested only in
>>> +sanitization of a particular module and performance is a concern, she can
>>> +disable instrumentation of both reads and writes for kernel code.
>>> +Instrumentation can be disabled with CONFIG_KASAN_READS and
>>> CONFIG_KASAN_WRITES.
>>> +
>>
>> I don't understand this. How this can be related to modules? Configs are global.
>> You can't just disable/enable config per module.
>
>
> Build everything without instrumentation. Then enable instrumentation
> and do "make lib/test_kasan.ko".
> Or build everything, copy out bzImage, change config, build everything again.
Yeah, this is soooooo convenient...
Seriously speaking, per-file instrumentation is absolutely irrelevant to this patch and should have been
addressed from a different angle. E.g. see how UBSAN/GCOV/KCOV do that.
As for this patch, I'd say only one option would be enough - KASAN_DONT_SANITIZE_READS.
Nobody wants to sanitize only reads without writes, right? Writes are fewer and more dangerous.
On Tue, Dec 13, 2016 at 2:59 PM, Andrey Ryabinin
<[email protected]> wrote:
> On 12/13/2016 12:38 PM, Dmitry Vyukov wrote:
>> On Tue, Dec 13, 2016 at 10:20 AM, Andrey Ryabinin
>> <[email protected]> wrote:
>>>
>>>
>>> On 12/13/2016 11:58 AM, Dmitry Vyukov wrote:
>>>
>>>> --- a/Documentation/dev-tools/kasan.rst
>>>> +++ b/Documentation/dev-tools/kasan.rst
>>>> @@ -40,6 +40,14 @@ similar to the following to the respective kernel Makefile:
>>>>
>>>> KASAN_SANITIZE := n
>>>>
>>>> +Sometimes it may be useful to disable instrumentation of reads, or writes
>>>> +or both for the entire kernel. For example, if binary size is a concern,
>>>> +it may be useful to disable instrumentation of reads to reduce binary size but
>>>> +still catch more harmful bugs on writes. Or, if one is interested only in
>>>> +sanitization of a particular module and performance is a concern, she can
>>>> +disable instrumentation of both reads and writes for kernel code.
>>>> +Instrumentation can be disabled with CONFIG_KASAN_READS and
>>>> CONFIG_KASAN_WRITES.
>>>> +
>>>
>>> I don't understand this. How this can be related to modules? Configs are global.
>>> You can't just disable/enable config per module.
>>
>>
>> Build everything without instrumentation. Then enable instrumentation
>> and do "make lib/test_kasan.ko".
>> Or build everything, copy out bzImage, change config, build everything again.
>
> Yeah, this is soooooo convenient...
>
> Seriously speaking, per-file instrumentation is absolutely irrelevant to this patch and should have been
> addressed from a different angle. E.g. see how UBSAN/GCOV/KCOV do that.
KASAN already has that functionality (i.e. KASAN_SANITIZE_main.o :=
n). But that functionality is intended for cases when we want to
persistently disable instrumentation of some files (e.g. if they cause
crashes of false positives). CONFIG_KASAN_READS/WRITES is intended for
situations when one wants to disable instrumentation wholesale.
> As for this patch, I'd say only one option would be enough - KASAN_DONT_SANITIZE_READS.
> Nobody wants to sanitize only reads without writes, right? Writes are fewer and more dangerous.
I've asked this question in v1. See the case related to modules -- one
can use completely uninstrumented kernel, but load an instrumented
modules.
On 12/13/2016 05:13 PM, Dmitry Vyukov wrote:
> On Tue, Dec 13, 2016 at 2:59 PM, Andrey Ryabinin
> <[email protected]> wrote:
>> On 12/13/2016 12:38 PM, Dmitry Vyukov wrote:
>>> On Tue, Dec 13, 2016 at 10:20 AM, Andrey Ryabinin
>>> <[email protected]> wrote:
>>>>
>>>>
>>>> On 12/13/2016 11:58 AM, Dmitry Vyukov wrote:
>>>>
>>>>> --- a/Documentation/dev-tools/kasan.rst
>>>>> +++ b/Documentation/dev-tools/kasan.rst
>>>>> @@ -40,6 +40,14 @@ similar to the following to the respective kernel Makefile:
>>>>>
>>>>> KASAN_SANITIZE := n
>>>>>
>>>>> +Sometimes it may be useful to disable instrumentation of reads, or writes
>>>>> +or both for the entire kernel. For example, if binary size is a concern,
>>>>> +it may be useful to disable instrumentation of reads to reduce binary size but
>>>>> +still catch more harmful bugs on writes. Or, if one is interested only in
>>>>> +sanitization of a particular module and performance is a concern, she can
>>>>> +disable instrumentation of both reads and writes for kernel code.
>>>>> +Instrumentation can be disabled with CONFIG_KASAN_READS and
>>>>> CONFIG_KASAN_WRITES.
>>>>> +
>>>>
>>>> I don't understand this. How this can be related to modules? Configs are global.
>>>> You can't just disable/enable config per module.
>>>
>>>
>>> Build everything without instrumentation. Then enable instrumentation
>>> and do "make lib/test_kasan.ko".
>>> Or build everything, copy out bzImage, change config, build everything again.
>>
>> Yeah, this is soooooo convenient...
>>
>> Seriously speaking, per-file instrumentation is absolutely irrelevant to this patch and should have been
>> addressed from a different angle. E.g. see how UBSAN/GCOV/KCOV do that.
>
>
> KASAN already has that functionality (i.e. KASAN_SANITIZE_main.o :=
> n). But that functionality is intended for cases when we want to
> persistently disable instrumentation of some files (e.g. if they cause
> crashes of false positives). CONFIG_KASAN_READS/WRITES is intended for
> situations when one wants to disable instrumentation wholesale.
>
I'm talking about UBSAN_SANITIZE_ALL/KCOV_INSTRUMENT_ALL/GCOV_PROFILE_ALL
KASAN doesn't have something similar. I didn't add this because IMO it's not very useful for KASAN.
One may have a bug in instrumented code, but it can be easily missed if access is done in generic
code. Very simple example is passing invalid pointer in strcpy()
>
>> As for this patch, I'd say only one option would be enough - KASAN_DONT_SANITIZE_READS.
>> Nobody wants to sanitize only reads without writes, right? Writes are fewer and more dangerous.
>
> I've asked this question in v1. See the case related to modules -- one
> can use completely uninstrumented kernel, but load an instrumented
> modules.
>
I get it, but again, it's not the right way to address this problem.
On Fri, Dec 16, 2016 at 11:29 AM, Vaneet Narang <[email protected]> wrote:
>
> Hi Andrey,
>
>
> There are times when requirement is only to find write issues so user should have flexibility to
> skip read instrumentation for better performance with KASAN enabled build to find realtime
> issues as well.
>
> >> crashes of false positives). CONFIG_KASAN_READS/WRITES is intended for
> >> situations when one wants to disable instrumentation wholesale.
> >>
> >
> >I'm talking about UBSAN_SANITIZE_ALL/KCOV_INSTRUMENT_ALL/GCOV_PROFILE_ALL
> >KASAN doesn't have something similar. I didn't add this because IMO it's not very useful for KASAN.
> >One may have a bug in instrumented code, but it can be easily missed if access is done in generic
> >code. Very simple example is passing invalid pointer in strcpy()
>
>
> Old toolchains already add asan hooks before strcpy, strcmp. Please check assembly generated
> for module with below code using gcc 4.9.
>
> static noinline void __init test_kasan_module(void) {
> strcpy(test, "testing strcpy");
> }
>
> 0000000000001108 <test_kasan_module>:
> 1108: a9bd7bfd stp x29, x30, [sp,#-48]!
> 110c: d28001e1 mov x1, #0xf // #15
> 1110: 910003fd mov x29, sp
> 1114: a90153f3 stp x19, x20, [sp,#16]
> 1118: 58000253 ldr x19, 1160 <test_kasan_module+0x58>
> 111c: f90013f5 str x21, [sp,#32]
> 1120: aa1303e0 mov x0, x19
> 1124: 94000000 bl 0 <__asan_loadN> // Instrumented Read of size 15
> 1128: 58000215 ldr x21, 1168 <test_kasan_module+0x60>
> 112c: d28001e1 mov x1, #0xf // #15
> 1130: 910042b4 add x20, x21, #0x10
> 1134: aa1403e0 mov x0, x20
> 1138: 94000000 bl 0 <__asan_storeN> // Instrumented Write of size 15
> 113c: f9400260 ldr x0, [x19]
> 1140: f9000aa0 str x0, [x21,#16]
> 1144: f8407260 ldr x0, [x19,#7]
> 1148: f80172a0 str x0, [x21,#23]
> 114c: a94153f3 ldp x19, x20, [sp,#16]
>
>
>
> Similar behaviour for strcmp, memset, memcpy ... but with latest compiler 6.2,
> this implementation is removed from compiler in this case we can define wrappers
> in kasan.c for these function like we are already doing for memcpy, memmove, memset
>
>
One option would be to simply compile kernel as:
$ make CC="gcc --param asan-instrument-reads=0"