Date: Thu, 27 Jul 2023 20:12:58 GMT From: Mark Johnston <markj@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: ca6cd604c8fc - main - kmsan: Use the correct origin bytes in kmsan_check_arg() Message-ID: <202307272012.36RKCwA6087695@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=ca6cd604c8fcfc27c6468c620a7bee518ca02cde commit ca6cd604c8fcfc27c6468c620a7bee518ca02cde Author: Mark Johnston <markj@FreeBSD.org> AuthorDate: 2023-07-17 13:34:57 +0000 Commit: Mark Johnston <markj@FreeBSD.org> CommitDate: 2023-07-27 20:02:03 +0000 kmsan: Use the correct origin bytes in kmsan_check_arg() Upon discovering a violation kmsan_check_arg() passes a pointer to function parameter shadow state to kmsan_report_hook(). kmsan_report_hook() uses that address to find the origin cells, assuming that the passed address belongs to the kernel map. This has two problems: 1) Function parameter origin state is also located in TLS, not in the origin map, but kmsan_report_hook() doesn't know this. 2) KMSAN TLS for thread0 is statically allocated and thus isn't shadowed (because the kernel itself is not shadowed). These bugs could result in inaccuracies in KMSAN reports, or a page fault when trying to report a KMSAN violation (which by default panics the kernel anyway). Fix the problem by making callers of kmsan_report_hook() provide a pointer to origin cells. Sponsored by: The FreeBSD Foundation --- sys/kern/subr_msan.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/sys/kern/subr_msan.c b/sys/kern/subr_msan.c index ba625a5405c5..debbcb56af2a 100644 --- a/sys/kern/subr_msan.c +++ b/sys/kern/subr_msan.c @@ -165,9 +165,9 @@ kmsan_orig_name(int type) } static void -kmsan_report_hook(const void *addr, size_t size, size_t off, const char *hook) +kmsan_report_hook(const void *addr, msan_orig_t *orig, size_t size, size_t off, + const char *hook) { - msan_orig_t *orig; const char *typename; char *var, *fn; uintptr_t ptr; @@ -181,9 +181,6 @@ kmsan_report_hook(const void *addr, size_t size, size_t off, const char *hook) kmsan_reporting = true; __compiler_membar(); - orig = (msan_orig_t *)kmsan_md_addr_to_orig((vm_offset_t)addr); - orig = (msan_orig_t *)((uintptr_t)orig & MSAN_ORIG_MASK); - if (*orig == 0) { REPORT("MSan: Uninitialized memory in %s, offset %zu", hook, off); @@ -363,6 +360,7 @@ kmsan_meta_copy(void *dst, const void *src, size_t size) static inline void kmsan_shadow_check(uintptr_t addr, size_t size, const char *hook) { + msan_orig_t *orig; uint8_t *shad; size_t i; @@ -375,7 +373,9 @@ kmsan_shadow_check(uintptr_t addr, size_t size, const char *hook) for (i = 0; i < size; i++) { if (__predict_true(shad[i] == 0)) continue; - kmsan_report_hook((const char *)addr + i, size, i, hook); + orig = (msan_orig_t *)kmsan_md_addr_to_orig((vm_offset_t)&shad[i]); + orig = (msan_orig_t *)((uintptr_t)orig & MSAN_ORIG_MASK); + kmsan_report_hook((const char *)addr + i, orig, size, i, hook); break; } } @@ -413,21 +413,24 @@ kmsan_init_ret(size_t n) static void kmsan_check_arg(size_t size, const char *hook) { + msan_orig_t *orig; msan_td_t *mtd; uint8_t *arg; - size_t i; + size_t ctx, i; if (__predict_false(!kmsan_enabled)) return; if (__predict_false(curthread == NULL)) return; mtd = curthread->td_kmsan; - arg = mtd->tls[mtd->ctx].param_shadow; + ctx = mtd->ctx; + arg = mtd->tls[ctx].param_shadow; for (i = 0; i < size; i++) { if (__predict_true(arg[i] == 0)) continue; - kmsan_report_hook((const char *)arg + i, size, i, hook); + orig = &mtd->tls[ctx].param_origin[i / sizeof(msan_orig_t)]; + kmsan_report_hook((const char *)arg + i, orig, size, i, hook); break; } }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202307272012.36RKCwA6087695>