| Message ID | 20220329124017.737571-39-glider@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add KernelMemorySanitizer infrastructure | expand |
On Tue, Mar 29, 2022 at 02:40:07PM +0200, Alexander Potapenko wrote: > KMSAN inserts API function calls in a lot of places (function entries > and exits, local variables, memory accesses), so they may get called > from the uaccess regions as well. That's insufficient. Explain how you did the right thing and made these functions actually safe to be called in this context. > Signed-off-by: Alexander Potapenko <glider@google.com> > --- > Link: https://linux-review.googlesource.com/id/I242bc9816273fecad4ea3d977393784396bb3c35 > --- > tools/objtool/check.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/tools/objtool/check.c b/tools/objtool/check.c > index 7c33ec67c4a95..8518eaf05bff0 100644 > --- a/tools/objtool/check.c > +++ b/tools/objtool/check.c > @@ -943,6 +943,25 @@ static const char *uaccess_safe_builtin[] = { > "__sanitizer_cov_trace_cmp4", > "__sanitizer_cov_trace_cmp8", > "__sanitizer_cov_trace_switch", > + /* KMSAN */ > + "kmsan_copy_to_user", > + "kmsan_report", > + "kmsan_unpoison_memory", > + "__msan_chain_origin", > + "__msan_get_context_state", > + "__msan_instrument_asm_store", > + "__msan_metadata_ptr_for_load_1", > + "__msan_metadata_ptr_for_load_2", > + "__msan_metadata_ptr_for_load_4", > + "__msan_metadata_ptr_for_load_8", > + "__msan_metadata_ptr_for_load_n", > + "__msan_metadata_ptr_for_store_1", > + "__msan_metadata_ptr_for_store_2", > + "__msan_metadata_ptr_for_store_4", > + "__msan_metadata_ptr_for_store_8", > + "__msan_metadata_ptr_for_store_n", > + "__msan_poison_alloca", > + "__msan_warning", > /* UBSAN */ > "ubsan_type_mismatch_common", > "__ubsan_handle_type_mismatch", > -- > 2.35.1.1021.g381101b075-goog >
On Wed, Mar 30, 2022 at 10:46 AM Peter Zijlstra <peterz@infradead.org> wrote: > On Tue, Mar 29, 2022 at 02:40:07PM +0200, Alexander Potapenko wrote: > > KMSAN inserts API function calls in a lot of places (function entries > > and exits, local variables, memory accesses), so they may get called > > from the uaccess regions as well. > > That's insufficient. Explain how you did the right thing and made these > functions actually safe to be called in this context. > > KMSAN API functions are used to update the metadata (shadow/origin pages) for kernel memory accesses. The metadata pages for kernel pointers are also located in the kernel memory, so touching them is not a problem. For userspace pointers, no metadata is allocated. If an API function is supposed to read or modify the metadata, it does so for kernel pointers and ignores userspace pointers. If an API function is supposed to return a pair of metadata pointers for the instrumentation to use (like all __msan_metadata_ptr_for_TYPE_SIZE() functions do), it returns the allocated metadata for kernel pointers and special dummy buffers residing in the kernel memory for userspace pointers. As a result, none of KMSAN API functions perform userspace accesses, but since they might be called from UACCESS regions they use user_access_save/restore(). Does this make sense? > > Signed-off-by: Alexander Potapenko <glider@google.com> > > --- > > Link: > https://linux-review.googlesource.com/id/I242bc9816273fecad4ea3d977393784396bb3c35 > > --- > > tools/objtool/check.c | 19 +++++++++++++++++++ > > 1 file changed, 19 insertions(+) > > > > diff --git a/tools/objtool/check.c b/tools/objtool/check.c > > index 7c33ec67c4a95..8518eaf05bff0 100644 > > --- a/tools/objtool/check.c > > +++ b/tools/objtool/check.c > > @@ -943,6 +943,25 @@ static const char *uaccess_safe_builtin[] = { > > "__sanitizer_cov_trace_cmp4", > > "__sanitizer_cov_trace_cmp8", > > "__sanitizer_cov_trace_switch", > > + /* KMSAN */ > > + "kmsan_copy_to_user", > > + "kmsan_report", > > + "kmsan_unpoison_memory", > > + "__msan_chain_origin", > > + "__msan_get_context_state", > > + "__msan_instrument_asm_store", > > + "__msan_metadata_ptr_for_load_1", > > + "__msan_metadata_ptr_for_load_2", > > + "__msan_metadata_ptr_for_load_4", > > + "__msan_metadata_ptr_for_load_8", > > + "__msan_metadata_ptr_for_load_n", > > + "__msan_metadata_ptr_for_store_1", > > + "__msan_metadata_ptr_for_store_2", > > + "__msan_metadata_ptr_for_store_4", > > + "__msan_metadata_ptr_for_store_8", > > + "__msan_metadata_ptr_for_store_n", > > + "__msan_poison_alloca", > > + "__msan_warning", > > /* UBSAN */ > > "ubsan_type_mismatch_common", > > "__ubsan_handle_type_mismatch", > > -- > > 2.35.1.1021.g381101b075-goog > > >
On Thu, Apr 14, 2022 at 05:30:34PM +0200, Alexander Potapenko wrote: > On Wed, Mar 30, 2022 at 10:46 AM Peter Zijlstra <peterz@infradead.org> > wrote: > > > On Tue, Mar 29, 2022 at 02:40:07PM +0200, Alexander Potapenko wrote: > > > KMSAN inserts API function calls in a lot of places (function entries > > > and exits, local variables, memory accesses), so they may get called > > > from the uaccess regions as well. > > > > That's insufficient. Explain how you did the right thing and made these > > functions actually safe to be called in this context. > > > As a result, none of KMSAN API functions perform userspace accesses, but > since they might be called from UACCESS regions they > use user_access_save/restore(). ^ That.. very good. Thanks!
diff --git a/tools/objtool/check.c b/tools/objtool/check.c index 7c33ec67c4a95..8518eaf05bff0 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -943,6 +943,25 @@ static const char *uaccess_safe_builtin[] = { "__sanitizer_cov_trace_cmp4", "__sanitizer_cov_trace_cmp8", "__sanitizer_cov_trace_switch", + /* KMSAN */ + "kmsan_copy_to_user", + "kmsan_report", + "kmsan_unpoison_memory", + "__msan_chain_origin", + "__msan_get_context_state", + "__msan_instrument_asm_store", + "__msan_metadata_ptr_for_load_1", + "__msan_metadata_ptr_for_load_2", + "__msan_metadata_ptr_for_load_4", + "__msan_metadata_ptr_for_load_8", + "__msan_metadata_ptr_for_load_n", + "__msan_metadata_ptr_for_store_1", + "__msan_metadata_ptr_for_store_2", + "__msan_metadata_ptr_for_store_4", + "__msan_metadata_ptr_for_store_8", + "__msan_metadata_ptr_for_store_n", + "__msan_poison_alloca", + "__msan_warning", /* UBSAN */ "ubsan_type_mismatch_common", "__ubsan_handle_type_mismatch",
KMSAN inserts API function calls in a lot of places (function entries and exits, local variables, memory accesses), so they may get called from the uaccess regions as well. Signed-off-by: Alexander Potapenko <glider@google.com> --- Link: https://linux-review.googlesource.com/id/I242bc9816273fecad4ea3d977393784396bb3c35 --- tools/objtool/check.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)