From patchwork Wed Jun 8 18:22:05 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jann Horn X-Patchwork-Id: 12874476 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7A25DC433EF for ; Wed, 8 Jun 2022 18:22:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EB9496B0071; Wed, 8 Jun 2022 14:22:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E45306B0072; Wed, 8 Jun 2022 14:22:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CE2656B0073; Wed, 8 Jun 2022 14:22:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id B9FDB6B0071 for ; Wed, 8 Jun 2022 14:22:17 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 87D871493 for ; Wed, 8 Jun 2022 18:22:17 +0000 (UTC) X-FDA: 79555888314.11.44C6597 Received: from mail-wr1-f46.google.com (mail-wr1-f46.google.com [209.85.221.46]) by imf30.hostedemail.com (Postfix) with ESMTP id 3EC8780068 for ; Wed, 8 Jun 2022 18:22:17 +0000 (UTC) Received: by mail-wr1-f46.google.com with SMTP id u8so25090688wrm.13 for ; Wed, 08 Jun 2022 11:22:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=CqFfK/K7lvMu2HZST0PsePn93HOEe9I/dMiXVfj24uU=; b=oLqhFZfLib2k1MEIDIJcOGQBCwww65p2chr7rI+RMKUcA1rG3r1MuTFixwFRb3b5sK +CjQxCkw5ZzQS7QGbT3ZHDgI9i2L3+r6YonwRjhqwgoCJ42LK2ygEAS01iDcbDcYaldg PvcIaOEhHRSiUGM6090LrCKZ9GIK8xlxmI4KP6f8eFoYDYM1gZw8GqIYNQ4MkkvO2usV ofjQe5j/XRShIcIc1tnaSN79SkPwk3V2LDIXuelyzvVQVEiKjUSnvppH5R/plCki6/NR V+pOI6Ey/Yp/TZLktgdMM+5MEteWXgmrY3RqsezGjY2Nz/xYqsV/L8dyRxBEdaLx0b33 viLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=CqFfK/K7lvMu2HZST0PsePn93HOEe9I/dMiXVfj24uU=; b=hl75SrIyGXWOkuvDiZkeHi+Ma6C3hiLkXkVEMmJ8RnpbGGKK+a/GyYvp2ijlxgvK8N 5Zqremiky41T7W2fotp+eFDCa39azypwr/CJzIhiTpKWBTt//9m/UmLDZq3vU9iVYXRP T8wkVP5MM0dM3hucQCPtGpEm1PJyhhjJs3V3ODiunWKCz5uyPZqpzo4ROFmbLAZMOxY/ 335JgamscMFX9ZApI3zVc7GbsSvNcHyJSDK16/Y92fbcLqYHKfn+8ZduK0ryFcWtmZp0 fjGiRVkK/sZbMbLmJbZwIhqox45sDWycKs/dlGU8X/riXTfZSKaKrsl0znQHVbo5i6Zm pJ0Q== X-Gm-Message-State: AOAM533862x6+I2iLTSPhl8sl7U0PlDk5cewnO2r3zl7CndnjjOKv9M8 g/mxFzQn/Nk34XrD5suCGCpkkg== X-Google-Smtp-Source: ABdhPJz+WFrYwbvqOsH2U2JLdTRztWjwS7FgP76BcReTg1Gqwlmp2MWWceSAMPxN78PbJ7ikZmdoew== X-Received: by 2002:adf:f38f:0:b0:210:30cf:6e4a with SMTP id m15-20020adff38f000000b0021030cf6e4amr35808095wro.676.1654712535630; Wed, 08 Jun 2022 11:22:15 -0700 (PDT) Received: from localhost ([2a00:79e0:9d:4:5a2c:fb4d:393:7e96]) by smtp.gmail.com with ESMTPSA id f18-20020a5d4dd2000000b0020fd392df33sm22145050wru.29.2022.06.08.11.22.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Jun 2022 11:22:14 -0700 (PDT) From: Jann Horn To: Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Vlastimil Babka Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: [PATCH] mm/slub: add missing TID updates on slab deactivation Date: Wed, 8 Jun 2022 20:22:05 +0200 Message-Id: <20220608182205.2945720-1-jannh@google.com> X-Mailer: git-send-email 2.36.1.476.g0c4daa206d-goog MIME-Version: 1.0 X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 3EC8780068 X-Stat-Signature: e6sg47o8fg8hfp5pwjdqzd4f69he4j4u X-Rspam-User: Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=oLqhFZfL; spf=pass (imf30.hostedemail.com: domain of jannh@google.com designates 209.85.221.46 as permitted sender) smtp.mailfrom=jannh@google.com; dmarc=pass (policy=reject) header.from=google.com X-HE-Tag: 1654712537-671561 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: The fastpath in slab_alloc_node() assumes that c->slab is stable as long as the TID stays the same. However, two places in __slab_alloc() currently don't update the TID when deactivating the CPU slab. If multiple operations race the right way, this could lead to an object getting lost; or, in an even more unlikely situation, it could even lead to an object being freed onto the wrong slab's freelist, messing up the `inuse` counter and eventually causing a page to be freed to the page allocator while it still contains slab objects. (I haven't actually tested these cases though, this is just based on looking at the code. Writing testcases for this stuff seems like it'd be a pain...) The race leading to state inconsistency is (all operations on the same CPU and kmem_cache): - task A: begin do_slab_free(): - read TID - read pcpu freelist (==NULL) - check `slab == c->slab` (true) - [PREEMPT A->B] - task B: begin slab_alloc_node(): - fastpath fails (`c->freelist` is NULL) - enter __slab_alloc() - slub_get_cpu_ptr() (disables preemption) - enter ___slab_alloc() - take local_lock_irqsave() - read c->freelist as NULL - get_freelist() returns NULL - write `c->slab = NULL` - drop local_unlock_irqrestore() - goto new_slab - slub_percpu_partial() is NULL - get_partial() returns NULL - slub_put_cpu_ptr() (enables preemption) - [PREEMPT B->A] - task A: finish do_slab_free(): - this_cpu_cmpxchg_double() succeeds() - [CORRUPT STATE: c->slab==NULL, c->freelist!=NULL] From there, the object on c->freelist will get lost if task B is allowed to continue from here: It will proceed to the retry_load_slab label, set c->slab, then jump to load_freelist, which clobbers c->freelist. But if we instead continue as follows, we get worse corruption: - task A: run __slab_free() on object from other struct slab: - CPU_PARTIAL_FREE case (slab was on no list, is now on pcpu partial) - task A: run slab_alloc_node() with NUMA node constraint: - fastpath fails (c->slab is NULL) - call __slab_alloc() - slub_get_cpu_ptr() (disables preemption) - enter ___slab_alloc() - c->slab is NULL: goto new_slab - slub_percpu_partial() is non-NULL - set c->slab to slub_percpu_partial(c) - [CORRUPT STATE: c->slab points to slab-1, c->freelist has objects from slab-2] - goto redo - node_match() fails - goto deactivate_slab - existing c->freelist is passed into deactivate_slab() - inuse count of slab-1 is decremented to account for object from slab-2 At this point, the inuse count of slab-1 is 1 lower than it should be. This means that if we free all allocated objects in slab-1 except for one, SLUB will think that slab-1 is completely unused, and may free its page, leading to use-after-free. Fixes: c17dda40a6a4e ("slub: Separate out kmem_cache_cpu processing from deactivate_slab") Fixes: 03e404af26dc2 ("slub: fast release on full slab") Cc: stable@vger.kernel.org Signed-off-by: Jann Horn Acked-by: Christoph Lameter Acked-by: David Rientjes Reviewed-by: Muchun Song Tested-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> --- mm/slub.c | 2 ++ 1 file changed, 2 insertions(+) base-commit: 9886142c7a2226439c1e3f7d9b69f9c7094c3ef6 diff --git a/mm/slub.c b/mm/slub.c index e5535020e0fdf..b97fa5e210469 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -2936,6 +2936,7 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node, if (!freelist) { c->slab = NULL; + c->tid = next_tid(c->tid); local_unlock_irqrestore(&s->cpu_slab->lock, flags); stat(s, DEACTIVATE_BYPASS); goto new_slab; @@ -2968,6 +2969,7 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node, freelist = c->freelist; c->slab = NULL; c->freelist = NULL; + c->tid = next_tid(c->tid); local_unlock_irqrestore(&s->cpu_slab->lock, flags); deactivate_slab(s, slab, freelist);