All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kuniyuki Iwashima <kuniyu@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	 Vlastimil Babka <vbabka@kernel.org>,
	Harry Yoo <harry@kernel.org>,
	 Andrey Konovalov <andreyknvl@gmail.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	 Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Hao Li <hao.li@linux.dev>,  Christoph Lameter <cl@gentwo.org>,
	David Rientjes <rientjes@google.com>,
	 Roman Gushchin <roman.gushchin@linux.dev>,
	kasan-dev@googlegroups.com, linux-mm@kvack.org
Subject: Re: [PATCH 2/2] mm/kasan/kunit: Add double-free tests involving kfree_rcu_sheaf().
Date: Tue, 27 Oct 2026 22:16:56 +0000	[thread overview]
Message-ID: <auEiOD0B675XgnSM@google.com> (raw)
In-Reply-To: <CAG_fn=XQXEdKSNG5L-dPMoNA-q_CmQsZ1G8c+jXaGBBWP5Jpmw@mail.gmail.com>

On Tue, 27 Oct 2026 13:48:05 +0100, Alexander Potapenko wrote:
> >                                           ^
> >   ...
> >       ok 2 kmem_cache_double_free_rcu_2
> >
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> Tested-by: Alexander Potapenko <glider@google.com>
> 
> > +       case 3:
> > +               migrate_disable();
> > +               kfree_rcu(p, rcu);
> > +
> > +               for (i = 0; i < cache->sheaf_capacity - 1; i++) {
> > +                       objs[i] = kmem_cache_alloc(cache, GFP_KERNEL);
> > +                       kfree_rcu(objs[i], rcu);
> > +               }
> > +
> > +               KUNIT_EXPECT_KASAN_FAIL(test, kfree_rcu(p, rcu));
> > +               migrate_enable();
> > +
> > +               /*
> > +                * Move the 1st full sheaf to quarantine, keeping the
> > +                * 2nd in pcs->rcu_free.
> > +                */
> > +               rcu_barrier();
> > +               break;
> > +       }
> 
> Unlike the existing double-free tests, this one can misfire on a
> healthy kernel: __kfree_rcu_sheaf() may decline (local_trylock()
> contention,
> alloc_empty_sheaf() failing under memory pressure), and
> kvfree_call_rcu() then falls back to call_rcu(kvfree_rcu_cb), which
> never sets rcu_free and
> ends in a plain kfree().
> 
> Could you drain pending frees unconditionally
> (kvfree_rcu_barrier_on_cache() + rcu_barrier()) right before
> kasan_quarantine_remove_cache()?
> In the good case it's a no-op and the victim check keeps its meaning;
> in the fallback case the stray kfree() hits p while it is still
> poisoned in
> quarantine and is reported as a double-free instead of corrupting the victim.
> 

Calling kvfree_rcu_barrier_on_cache() unconditionally before
kasan_quarantine_remove_cache() would also flush the 2nd sheaf in
pcs->rcu_free while p is still in quarantine, making the victim
check a no-op if the 2nd kfree_rcu() wasn't rejected (e.g. without
patch 1).

Instead, I'll move the kasan_byte_accessible(p) check out of
case 2 like below in v2.  This keeps the 2nd sheaf for the victim
check in the normal case, while draining pending frees if p
hasn't reached quarantine yet due to the fallback path.

This keeps the false-positive UAF w/o patch 1 and double-free
w/ patch 1.

---8<---
diff --git a/mm/kasan/kasan_test_c.c b/mm/kasan/kasan_test_c.c
index 60d25eb7cb01..83c418c0ef87 100644
--- a/mm/kasan/kasan_test_c.c
+++ b/mm/kasan/kasan_test_c.c
@@ -1112,10 +1112,6 @@ static void __kmem_cache_double_free_rcu(struct kunit *test, int mode)
 		/* The actual free order is inverse. */
 		kfree_rcu(p, rcu);
 		KUNIT_EXPECT_KASAN_FAIL(test, kmem_cache_free(cache, p));
-
-		/* Move p to quarantine if kmem_cache_free() was rejected. */
-		if (kasan_byte_accessible(p))
-			kvfree_rcu_barrier_on_cache(cache);
 		break;
 	case 3:
 		migrate_disable();
@@ -1137,6 +1133,13 @@ static void __kmem_cache_double_free_rcu(struct kunit *test, int mode)
 		break;
 	}
 
+	/*
+	 * Move p to quarantine if not poisoned yet (case 2,
+	 * or fallback in case 3).
+	 */
+	if (kasan_byte_accessible(p))
+		kvfree_rcu_barrier_on_cache(cache);
+
 	/*
 	 * Evict p from quarantine and reallocate it as a live victim object
 	 * before the RCU sheaf is flushed.
---8<---


  reply	other threads:[~2026-10-27 22:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-09 21:46 [PATCH 0/2] mm/kasan: Detect double-free involving kfree_rcu_sheaf() Kuniyuki Iwashima
2026-10-09 21:46 ` [PATCH 1/2] " Kuniyuki Iwashima
2026-10-20 12:29   ` Vlastimil Babka (SUSE)
2026-10-27 12:48   ` Alexander Potapenko
2026-10-27 21:51     ` Kuniyuki Iwashima
2026-10-09 21:46 ` [PATCH 2/2] mm/kasan/kunit: Add double-free tests " Kuniyuki Iwashima
2026-10-27 12:48   ` Alexander Potapenko
2026-10-27 22:16     ` Kuniyuki Iwashima [this message]
2026-10-27  6:15 ` [PATCH 0/2] mm/kasan: Detect double-free " Kuniyuki Iwashima

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=auEiOD0B675XgnSM@google.com \
    --to=kuniyu@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@gmail.com \
    --cc=cl@gentwo.org \
    --cc=dvyukov@google.com \
    --cc=glider@google.com \
    --cc=hao.li@linux.dev \
    --cc=harry@kernel.org \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-mm@kvack.org \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=ryabinin.a.a@gmail.com \
    --cc=vbabka@kernel.org \
    --cc=vincenzo.frascino@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.