[src/trunk]: src/sys/kern Fix tiny race in pool+KASAN, that resulted in occas...

[maxv <maxv%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/0775e9e7e857
branches:  trunk
changeset: 455594:0775e9e7e857
user:      maxv <maxv%NetBSD.org@localhost>
date:      Sun Apr 07 08:37:38 2019 +0000

description:
Fix tiny race in pool+KASAN, that resulted in occasional false positives.

We were uselessly marking already valid areas as valid. When doing that,
our KASAN code emits two calls to kasan_markmem, and there is a very small
window where the area becomes invalid. So, if the area happens to be
already globally referenced, and if another thread happens to read the
buffer via this reference, we get a false positive.

This happens only with pool_caches that have a pc_ctor that creates a
global reference to the buffer, and there is one single pool_cache that
does that: 'file_cache'.

So now, two changes:

 - In pool_cache_get_slow(), the pool_get() has already redzoned the
   object, so no need to call pool_redzone_fill().

 - In pool_cache_destruct_object1(), don't re-mark the object. If there is
   no ctor pool_put is fine with already-invalid objects, if there is a
   ctor the object was not marked as invalid in the first place; so in
   either case, the re-marking is not needed.

Fixes PR/53674. Although very rare and difficult to reproduce, a local
quarantine patch of mine made the false positives recurrent.

diffstat:

 sys/kern/subr_pool.c |  15 ++-------------
 1 files changed, 2 insertions(+), 13 deletions(-)

diffs (43 lines):

diff -r 3a4bf1e0210b -r 0775e9e7e857 sys/kern/subr_pool.c
--- a/sys/kern/subr_pool.c      Sun Apr 07 08:14:20 2019 +0000
+++ b/sys/kern/subr_pool.c      Sun Apr 07 08:37:38 2019 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: subr_pool.c,v 1.246 2019/03/28 18:12:24 maxv Exp $     */
+/*     $NetBSD: subr_pool.c,v 1.247 2019/04/07 08:37:38 maxv Exp $     */
 
 /*
  * Copyright (c) 1997, 1999, 2000, 2002, 2007, 2008, 2010, 2014, 2015, 2018
@@ -33,7 +33,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: subr_pool.c,v 1.246 2019/03/28 18:12:24 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: subr_pool.c,v 1.247 2019/04/07 08:37:38 maxv Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -2129,16 +2129,6 @@
 static void
 pool_cache_destruct_object1(pool_cache_t pc, void *object)
 {
-       if (pc->pc_pool.pr_redzone) {
-               /*
-                * The object is marked as invalid. Temporarily mark it as
-                * valid for the destructor. pool_put below will re-mark it
-                * as invalid.
-                */
-               kasan_mark(object, pc->pc_pool.pr_reqsize,
-                   pc->pc_pool.pr_reqsize_with_redzone);
-       }
-
        (*pc->pc_dtor)(pc->pc_arg, object);
        pool_put(&pc->pc_pool, object);
 }
@@ -2396,7 +2386,6 @@
        }
 
        FREECHECK_OUT(&pc->pc_freecheck, object);
-       pool_redzone_fill(&pc->pc_pool, object);
        pool_cache_kleak_fill(pc, object);
        return false;
 }