From patchwork Fri Nov 1 13:08:44 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hyeonggon Yoo <42.hyeyoo@gmail.com> X-Patchwork-Id: 13859340 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 47D37E6B271 for ; Fri, 1 Nov 2024 13:09:02 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 908F06B008A; Fri, 1 Nov 2024 09:09:01 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8B9056B008C; Fri, 1 Nov 2024 09:09:01 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 781596B0093; Fri, 1 Nov 2024 09:09:01 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 593586B008A for ; Fri, 1 Nov 2024 09:09:01 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id BBB9BC024C for ; Fri, 1 Nov 2024 13:09:00 +0000 (UTC) X-FDA: 82737555852.18.DDD3A1D Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) by imf23.hostedemail.com (Postfix) with ESMTP id 866A0140010 for ; Fri, 1 Nov 2024 13:08:40 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=HbM6tcSS; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf23.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.214.178 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1730466457; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-transfer-encoding:content-transfer-encoding: in-reply-to:references:dkim-signature; bh=zLDYkUugUVex3m6C8OIePDelk/lr6BlNA0TOsAqbir8=; b=0kLbm0XMoZl15neDafCfT09XnL18s0sWAAfLOaYrL6v6kcd8sKWo6tdDcDEl3vWTTD9WWf 71hy+9Chc4TlwVTIUbdFucWm4DkgVP0HpnQA7aMtFYP1Xf5bmKwnLxr0ZzpII81s34eUuD z6a06kTPxtcIfTLpSGJJFniSOTQeJeY= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=HbM6tcSS; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf23.hostedemail.com: domain of 42.hyeyoo@gmail.com designates 209.85.214.178 as permitted sender) smtp.mailfrom=42.hyeyoo@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1730466457; a=rsa-sha256; cv=none; b=UgHUklmZb8+T4oisKIY8ujw1O/XLrhq8Sj5Ot777j1dJ6N+0JR2OfxPJKH4NRc7Q2/aaIt Itt+v26guLNDR70bSJIwfZ0RrY9mg1XZFjLcB9Ar0MCPoF1G/zoV6MMSylL+sOTGIE1+vx ABcrjspoWZoszcrsrGeS7F7z1j8XzWw= Received: by mail-pl1-f178.google.com with SMTP id d9443c01a7336-20c8c50fdd9so20818085ad.0 for ; Fri, 01 Nov 2024 06:08:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1730466537; x=1731071337; darn=kvack.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=zLDYkUugUVex3m6C8OIePDelk/lr6BlNA0TOsAqbir8=; b=HbM6tcSSNFqej+UTBgE99f6AbEEKfj36oHom3z0PGLyxXNf63pAe85TujS75xvAQEF sPZqFwDdZ5QrfCVc1jIXkTKj1lurGiyi9Nea+VQh/0gXM2bcRzrDtUWlNPj02nfb1Lcd 5g460UQlv82kMaI398JwfqsqRcoxg/VrCo3nK5wr95UnDnm5WNwtkroxlbO1csbE7EnW pPFv1+05Ro7qYhHlS6QDj0U50U0LJ5boTlKvUCl4Bt05HvrWb1TowxY8uNXCI7e9a0hh yw8zbnoHiH7FynCLm7O5gUXAVizUmIhfBTg7FWeI/ciuloCbNBBn2LWphlIXf78KWVx6 fK0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730466537; x=1731071337; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=zLDYkUugUVex3m6C8OIePDelk/lr6BlNA0TOsAqbir8=; b=SD6zZhmfvnAc/Hsf3C07ME4H8cDUu+/zvNgLE6DvwfFnnsG4y5Vq8RnBbpITo1FXxC FzyPaHy5/zkgnvdPH9EJdvJpapTg0Qqw5/i9/HLC2pLpH3z3ATyOGY+CQALoIoyxBIK+ UC5ySON2BNFWv/zjfPzEzZOV1WCWV/+J9RRtphhr8DNbBfLS+D/jV2F2ArCkBOqlBW8l UkXbysgT7dvOkS8hH7JRZmIcEwtv5qIrN97yVN9tBPIOBNHVw37blIQh3/JT4uL12huL 0y7+x7QSHApuNSm26wc0Q6RCkwZyK90b998Na+xEwKH8TUTVJzAAp/dv0n2gmnXp6GuM MeVA== X-Forwarded-Encrypted: i=1; AJvYcCVz2EIie8A2bRngEXFyXBhiABiDMms9pt3prVcvEad+8hPQchjkX83VuyxjqRxvD5KH1r92+JuuUA==@kvack.org X-Gm-Message-State: AOJu0YzmShtrPMQMIstRmZ8kmrxNrbOSFP6wATkBhnqTY2iEtEEmdB2N O5DXQRUZETt+cQD//+uVJftnHna23MyQAFfAI+7zfVUNSfC7GcEY X-Google-Smtp-Source: AGHT+IGDjzffyadZbrJBxHVYA/B3oGtXlrWVG8rX7QgoRohFLiHD5GNG1Ok2XyfnygwpxcPmVV4eLQ== X-Received: by 2002:a17:902:d4cc:b0:20b:831f:e8f7 with SMTP id d9443c01a7336-211193ffaf4mr57309835ad.11.1730466537226; Fri, 01 Nov 2024 06:08:57 -0700 (PDT) Received: from fedora.. ([1.245.180.67]) by smtp.gmail.com with ESMTPSA id 41be03b00d2f7-7ee452ae3a0sm2429400a12.37.2024.11.01.06.08.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 01 Nov 2024 06:08:56 -0700 (PDT) From: Hyeonggon Yoo <42.hyeyoo@gmail.com> To: Christoph Lameter , Pekka Enberg , David Rientjes , Joonsoo Kim , Andrew Morton , Vlastimil Babka , Roman Gushchin Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>, linux-mm@kvack.org Subject: [PATCH v1] mm/slab: Allow cache creation to proceed even if sysfs registration fails Date: Fri, 1 Nov 2024 22:08:44 +0900 Message-ID: <20241101130845.19100-1-42.hyeyoo@gmail.com> X-Mailer: git-send-email 2.45.0 MIME-Version: 1.0 X-Rspam-User: X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 866A0140010 X-Stat-Signature: sx35osqw6dqm9ytj6f4j8fzx5g15gfh8 X-HE-Tag: 1730466520-710288 X-HE-Meta: U2FsdGVkX1/EMjWxumS7Eh6KnTNsna46rE0yQleR2OcLKZojNV2+gskqEC4zt7LpdNUP7KYB1mOaWUyinBznrSjIa/NJyWOU1fXMElKcI2SjiODQMsOVce0c1XuuLdNUT4rUbShl97ScbihVZW3L77lriKDRDbRHN5NnttbOCRFKkwD93VsWzjboIZbPfcq2/BwJ9aYQWuCA5jCtVxaq8rmxdPjE2GTLZc7B+TmLqAgn5QIHiq460NynbY8XGvDeLhz4bfbVCbOqYNKx9ZXWTqDqdAASMVL0A2gRczk231YE1Siml8qKE+c+AM9cR+x8KQMGBvNKXiDOBzcVbVTK+axNB+VXY/kmdosmhu+afLj5TAkUUg5HsV+z6i/4e2ZVcF1ArFr10c1KX8cw+c7ja2xxH4eqltjz4ZxjWAzlIiGy8a5YerH19c+7a0FRMyJHJ+4KZ3Z7ScLm2HEOLuWQ2THnosBMwaVvHZYNWRNCvvi80SLcKVbRBl1auZJuD78TAKMwf1tkR8J6S4VHtyQ2KU26OAZ3TFoSjRtYDKFtC2QL9D67XcW8g1y6iXccrwBpiwQofbW/RehjMFO2FoeNh8AbWpsqL2tYWeiS1Oznzewkuu8XlQI/5p4noIxm9wTIZsKpQHHSZnSGI3QsHqdzqSqIwdOgDTGG4pStBBQlUlkTGWR4M6CdjCUUEUH8k5tXle12bxy4hFsLjdLEdWPb24GiQ+UnTfYpx4VzguDYoeS/sxJckk2kMIQ0Q+X7uxcL6kYOUSzmRcSz4OCm/bPn1OPGkasslblHzI0HhRH+Uj52BxeiDo/INwTGHr46pBejCceTAyZjt9U/foRAbxJNsxI1iY/88EePSuk0fBjgjs0cXoU1EHJICXV4B/pKlNPZ47LbXbJhCiGq5nWFvaBJq4tf+UwBDqmEqGCvzDFHd8k44AwB/s3IybabkC8s4dga+Ti4n+5K1TBQSKEi59I KaVCH1YR MVv1Xx9pjC1ivNAR4ocnGmyt+2gRJqiwJUGdPYc/TnoC0RYmcs7E3WRhiIt1Lb3mALFNHtUmbuRkqpmyaYg0wf5jwN2AH+FuD9Mg04SpcF+IhqD9DF31WGd+BuxtTAsbXngaodLKwtUZB25F3EUc3+OHovi6NElOmPbBU6YF1MTRiXeV2KLELqoz3++z8fncRI5Hqfv7SSkUviUU10l9LATVniNu80pzCF9n5DvJB0YlV0+FN/8R2VlDsz/gZM0cXvKa4z4RRoSPHBa2U7jzOH13BWog7OT0ACI5OSLqGPj2u5WokQJrvYbpl+7rj9iX4MTe8p/fmTikVanrKa5qD8IivLT3sxZkmr9lLCrtoUCuAmz3zg2yWuTG1Qt4eQBhsogWgsWUFP4ScCjDGC6Q2DPbgl2NDxQMm9Xs4LURhbSMTfsO2ZiCObeqQ0C+GAUKg5a/DDgLq245Z3ubbch0NAAyX0GDWnmyP4JmS 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: List-Subscribe: List-Unsubscribe: When kobject_init_and_add() fails during cache creation, kobj->name can be leaked because SLUB does not call kobject_put(), which should be invoked per the kobject API documentation. This has a bit of historical context, though; SLUB does not call kobject_put() to avoid double-free for struct kmem_cache because 1) simply calling it would free all resources related to the cache, and 2) struct kmem_cache descriptor is always freed by cache_cache()'s error handling path, causing struct kmem_cache to be freed twice. This issue can be reproduced by creating new slab caches while applying failslab for kernfs_node_cache. This makes kobject_add_varg() succeed, but causes kobject_add_internal() to fail in kobject_init_and_add() during cache creation. Historically, this issue has attracted developers' attention several times. Each time a fix addressed either the leak or the double-free, it caused the other issue. Let's summarize a bit of history here: The leak has existed since the early days of SLUB. Commit 54b6a731025f ("slub: fix leak of 'name' in sysfs_slab_add") introduced a double-free bug while fixing the leak. Commit 80da026a8e5d ("mm/slub: fix slab double-free in case of duplicate sysfs filename") re-introduced the leak while fixing the double-free error. Commit dde3c6b72a16 ("mm/slub: fix a memory leak in sysfs_slab_add()") fixed the memory leak, but it was later reverted by commit 757fed1d0898 ("Revert "mm/slub: fix a memory leak in sysfs_slab_add()"") to avoid the double-free error. This is where we are now: we've chosen a memory leak over a double-free. To resolve this memory leak, skip creating sysfs files if it fails and continue with cache creation regardless (as suggested by Christoph). This resolves the memory leak because both the cache and the kobject remain alive on kobject_init_and_add() failure. If SLUB tries to create an alias for a cache without sysfs files, its symbolic link will not be generated. Since a slab cache might not have associated sysfs files, call kobject_del() only if such files exist. Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> --- RFC -> v1: Make sysfs optional instead of destroying the cache on sysfs errors. (Suggested by Christoph) mm/slub.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/mm/slub.c b/mm/slub.c index 151a987dc3a0..b4b211468f77 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -6116,7 +6116,8 @@ __kmem_cache_alias(const char *name, unsigned int size, unsigned int align, s = find_mergeable(size, align, flags, name, ctor); if (s) { if (sysfs_slab_alias(s, name)) - return NULL; + pr_err("SLUB: Unable to add slab alias %s to sysfs\n", + name); s->refcount++; @@ -6204,9 +6205,15 @@ int do_kmem_cache_create(struct kmem_cache *s, const char *name, goto out; } + /* + * Failing to create sysfs files is not critical to SLUB functionality. + * If it fails, proceed with cache creation without these files. + */ err = sysfs_slab_add(s); - if (err) - goto out; + if (err) { + err = 0; + pr_err("SLUB: Unable to add slab %s to sysfs\n", s->name); + } if (s->flags & SLAB_STORE_USER) debugfs_slab_add(s); @@ -7276,7 +7283,8 @@ static int sysfs_slab_add(struct kmem_cache *s) void sysfs_slab_unlink(struct kmem_cache *s) { - kobject_del(&s->kobj); + if (s->kobj.state_in_sysfs) + kobject_del(&s->kobj); } void sysfs_slab_release(struct kmem_cache *s) @@ -7305,6 +7313,11 @@ static int sysfs_slab_alias(struct kmem_cache *s, const char *name) * If we have a leftover link then remove it. */ sysfs_remove_link(&slab_kset->kobj, name); + /* + * The original cache may have failed to generate sysfs file. + * In that case, sysfs_create_link() returns -ENOENT and + * symbolic link creation is skipped. + */ return sysfs_create_link(&slab_kset->kobj, &s->kobj, name); }