Message ID | 20240613153924.961511-17-iii@linux.ibm.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | kmsan: Enable on s390 | expand |
On Thu, Jun 13, 2024 at 5:39 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > Even though the KMSAN warnings generated by memchr_inv() are suppressed > by metadata_access_enable(), its return value may still be poisoned. > > The reason is that the last iteration of memchr_inv() returns > `*start != value ? start : NULL`, where *start is poisoned. Because of > this, somewhat counterintuitively, the shadow value computed by > visitSelectInst() is equal to `(uintptr_t)start`. > > The intention behind guarding memchr_inv() behind > metadata_access_enable() is to touch poisoned metadata without > triggering KMSAN, so unpoison its return value. What do you think about applying __no_kmsan_checks to these functions instead?
On Tue, 2024-06-18 at 16:38 +0200, Alexander Potapenko wrote: > On Thu, Jun 13, 2024 at 5:39 PM Ilya Leoshkevich <iii@linux.ibm.com> > wrote: > > > > Even though the KMSAN warnings generated by memchr_inv() are > > suppressed > > by metadata_access_enable(), its return value may still be > > poisoned. > > > > The reason is that the last iteration of memchr_inv() returns > > `*start != value ? start : NULL`, where *start is poisoned. Because > > of > > this, somewhat counterintuitively, the shadow value computed by > > visitSelectInst() is equal to `(uintptr_t)start`. > > > > The intention behind guarding memchr_inv() behind > > metadata_access_enable() is to touch poisoned metadata without > > triggering KMSAN, so unpoison its return value. > > What do you think about applying __no_kmsan_checks to these functions > instead? Ok, will do. The __no_kmsan_checks approach is already taken by "mm: kfence: Disable KMSAN when checking the canary", so we might as well be consistent in how we fix these issues.
diff --git a/mm/slub.c b/mm/slub.c index a290f6c63e7b..b9101b2dc9aa 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -1185,6 +1185,7 @@ static int check_bytes_and_report(struct kmem_cache *s, struct slab *slab, metadata_access_enable(); fault = memchr_inv(kasan_reset_tag(start), value, bytes); metadata_access_disable(); + kmsan_unpoison_memory(&fault, sizeof(fault)); if (!fault) return 1; @@ -1291,6 +1292,7 @@ static void slab_pad_check(struct kmem_cache *s, struct slab *slab) metadata_access_enable(); fault = memchr_inv(kasan_reset_tag(pad), POISON_INUSE, remainder); metadata_access_disable(); + kmsan_unpoison_memory(&fault, sizeof(fault)); if (!fault) return; while (end > fault && end[-1] == POISON_INUSE)