From nobody Sat Feb 7 23:23:05 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id ECD152EBDD0; Thu, 29 Jan 2026 14:38:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769697484; cv=none; b=Aqt5CqWwDxn9QP3lSK5hL2asGfl3nlmst9NSKOprTEflQpp/e9ZQzryUcCKP/fqmHj+lbvk37wDZbBCE7Ly9ez2NjSrh066H0GBZLpeqosUZRzpWaTqfYPrilKWxkdeL1uM05NqTQf4/Tw4qhWhx3k0G7P4y8Tj+JZK4WtZ09Y8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769697484; c=relaxed/simple; bh=C5tmgGU5cz43OD3SA976f+0YYg5ZavulT15yW39I/tQ=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=aKz4DMdlutiLtIPDQssx3DhGy1yCmpRSnnVGhsx8YG5Ktx3iIZk6GBxZfcioo7AeQJSozWH00laixJTxGrGHXxy0AcUjySo7A4SwzUKHJxBhQTtgMJ77rudB7RY7qeBgYdNRFs+Tu/UCtdLAXPk8Vtos4IPvUb2Y5wBv1j1o0hw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=eWxcAga0; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="eWxcAga0" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 856E1C116D0; Thu, 29 Jan 2026 14:38:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1769697483; bh=C5tmgGU5cz43OD3SA976f+0YYg5ZavulT15yW39I/tQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=eWxcAga0I5JSEhPiF7moNaGrYHTM6JUrdQ0jtHKH7gqVQhHscWnYDSW8szUoz6/ZC HKay1egya42/1LFCnGvl19B5TK5pg5UJ33iMAdjWT8PjjMsuUzelNQEovP9qT1u3ct 0SbKTy5lf3YnMvS8pyEhxCZg/mre+2yYINfT98iqZOCx8mzmPPPEzdH48S6imzz2vU LQZoZ7rhJ1efuigZ/umsq194ygC+OxX0j3pfucaWQwQfAgid2UvPYo7gIAjWnABmgY i5n2r0yJTdon/YSnYBt/eNgeDukcJGbH96jYx6jzOv7QZNW5fUa197oTyITIvS7/hD pR+YTsJVIXNHA== From: Tzung-Bi Shih To: Greg Kroah-Hartman , "Rafael J. Wysocki" , Danilo Krummrich Cc: Jonathan Corbet , Shuah Khan , Laurent Pinchart , Bartosz Golaszewski , Wolfram Sang , Jason Gunthorpe , Johan Hovold , linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, chrome-platform@lists.linux.dev, tzungbi@kernel.org Subject: [PATCH 1/4] revocable: Fix races in revocable_alloc() using RCU Date: Thu, 29 Jan 2026 14:37:30 +0000 Message-ID: <20260129143733.45618-2-tzungbi@kernel.org> X-Mailer: git-send-email 2.53.0.rc1.217.geba53bf80e-goog In-Reply-To: <20260129143733.45618-1-tzungbi@kernel.org> References: <20260129143733.45618-1-tzungbi@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" There are two race conditions when allocating a revocable instance: 1. After a struct revocable_provider is revoked, the caller might still hold a dangling pointer to it. A subsequent call to revocable_alloc() can trigger a use-after-free. 2. If revocable_provider_release() runs concurrently with revocable_alloc(), the memory of struct revocable_provider can be accessed during or after kfree(). To fix these: - Manage the lifetime of struct revocable_provider using RCU. Annotate pointers to it with __rcu and use kfree_rcu() for deallocation. - Update revocable_alloc() to safely acquire a reference using RCU primitives. - Update revocable_provider_revoke() to take a double pointer (`**rp`). It atomically NULLs out the caller's pointer before starting revocation. This prevents the caller from holding a dangling pointer. - Drop devm_revocable_provider_alloc(). The devm-managed model cannot support the required double-pointer semantic for safe pointer nulling. Reported-by: Johan Hovold Closes: https://lore.kernel.org/all/aXdy-b3GOJkzGqYo@hovoldconsulting.com/ Signed-off-by: Tzung-Bi Shih --- .../driver-api/driver-model/revocable.rst | 3 - drivers/base/revocable.c | 93 ++++++++++--------- drivers/base/revocable_test.c | 20 ++-- include/linux/revocable.h | 8 +- .../revocable/test_modules/revocable_test.c | 19 ++-- 5 files changed, 72 insertions(+), 71 deletions(-) diff --git a/Documentation/driver-api/driver-model/revocable.rst b/Document= ation/driver-api/driver-model/revocable.rst index 22a442cc8d7f..ab7c0af5fbb6 100644 --- a/Documentation/driver-api/driver-model/revocable.rst +++ b/Documentation/driver-api/driver-model/revocable.rst @@ -76,9 +76,6 @@ For Resource Providers .. kernel-doc:: drivers/base/revocable.c :identifiers: revocable_provider_alloc =20 -.. kernel-doc:: drivers/base/revocable.c - :identifiers: devm_revocable_provider_alloc - .. kernel-doc:: drivers/base/revocable.c :identifiers: revocable_provider_revoke =20 diff --git a/drivers/base/revocable.c b/drivers/base/revocable.c index b068e18a847d..1bcd1cf54764 100644 --- a/drivers/base/revocable.c +++ b/drivers/base/revocable.c @@ -64,11 +64,13 @@ * @srcu: The SRCU to protect the resource. * @res: The pointer of resource. It can point to anything. * @kref: The refcount for this handle. + * @rcu: The RCU to protect pointer to itself. */ struct revocable_provider { struct srcu_struct srcu; void __rcu *res; struct kref kref; + struct rcu_head rcu; }; =20 /** @@ -88,8 +90,9 @@ struct revocable { * This holds an initial refcount to the struct. * * Return: The pointer of struct revocable_provider. NULL on errors. + * It enforces the caller handles the returned pointer in RCU ways. */ -struct revocable_provider *revocable_provider_alloc(void *res) +struct revocable_provider __rcu *revocable_provider_alloc(void *res) { struct revocable_provider *rp; =20 @@ -98,10 +101,10 @@ struct revocable_provider *revocable_provider_alloc(vo= id *res) return NULL; =20 init_srcu_struct(&rp->srcu); - rcu_assign_pointer(rp->res, res); + RCU_INIT_POINTER(rp->res, res); kref_init(&rp->kref); =20 - return rp; + return (struct revocable_provider __rcu *)rp; } EXPORT_SYMBOL_GPL(revocable_provider_alloc); =20 @@ -111,82 +114,80 @@ static void revocable_provider_release(struct kref *k= ref) struct revocable_provider, kref); =20 cleanup_srcu_struct(&rp->srcu); - kfree(rp); + kfree_rcu(rp, rcu); } =20 /** * revocable_provider_revoke() - Revoke the managed resource. - * @rp: The pointer of resource provider. + * @rp_ptr: The pointer of pointer of resource provider. * * This sets the resource `(struct revocable_provider *)->res` to NULL to * indicate the resource has gone. * * This drops the refcount to the resource provider. If it is the final * reference, revocable_provider_release() will be called to free the stru= ct. - */ -void revocable_provider_revoke(struct revocable_provider *rp) -{ - rcu_assign_pointer(rp->res, NULL); - synchronize_srcu(&rp->srcu); - kref_put(&rp->kref, revocable_provider_release); -} -EXPORT_SYMBOL_GPL(revocable_provider_revoke); - -static void devm_revocable_provider_revoke(void *data) -{ - struct revocable_provider *rp =3D data; - - revocable_provider_revoke(rp); -} - -/** - * devm_revocable_provider_alloc() - Dev-managed revocable_provider_alloc(= ). - * @dev: The device. - * @res: The pointer of resource. - * - * It is convenient to allocate providers via this function if the @res is - * also tied to the lifetime of the @dev. revocable_provider_revoke() will - * be called automatically when the device is unbound. * - * This holds an initial refcount to the struct. - * - * Return: The pointer of struct revocable_provider. NULL on errors. + * It enforces the caller to pass a pointer of pointer of resource provide= r so + * that it sets \*rp_ptr to NULL to prevent from keeping a dangling pointe= r. */ -struct revocable_provider *devm_revocable_provider_alloc(struct device *de= v, - void *res) +void revocable_provider_revoke(struct revocable_provider __rcu **rp_ptr) { struct revocable_provider *rp; =20 - rp =3D revocable_provider_alloc(res); + rp =3D rcu_replace_pointer(*rp_ptr, NULL, 1); if (!rp) - return NULL; + return; =20 - if (devm_add_action_or_reset(dev, devm_revocable_provider_revoke, rp)) - return NULL; - - return rp; + rcu_assign_pointer(rp->res, NULL); + synchronize_srcu(&rp->srcu); + kref_put(&rp->kref, revocable_provider_release); } -EXPORT_SYMBOL_GPL(devm_revocable_provider_alloc); +EXPORT_SYMBOL_GPL(revocable_provider_revoke); =20 /** * revocable_alloc() - Allocate struct revocable. - * @rp: The pointer of resource provider. + * @_rp: The pointer of resource provider. * * This holds a refcount to the resource provider. * * Return: The pointer of struct revocable. NULL on errors. */ -struct revocable *revocable_alloc(struct revocable_provider *rp) +struct revocable *revocable_alloc(struct revocable_provider __rcu *_rp) { + struct revocable_provider *rp; struct revocable *rev; =20 + if (!_rp) + return NULL; + + /* + * Enter a read-side critical section. + * + * This prevents kfree_rcu() from freeing the struct revocable_provider + * memory, for the duration of this scope. + */ + scoped_guard(rcu) { + rp =3D rcu_dereference(_rp); + if (!rp) + /* The revocable provider has been revoked. */ + return NULL; + + if (!kref_get_unless_zero(&rp->kref)) + /* + * The revocable provider is releasing (i.e., + * revocable_provider_release() has been called). + */ + return NULL; + } + /* At this point, `rp` is safe to access as holding a kref of it */ + rev =3D kzalloc(sizeof(*rev), GFP_KERNEL); - if (!rev) + if (!rev) { + kref_put(&rp->kref, revocable_provider_release); return NULL; + } =20 rev->rp =3D rp; - kref_get(&rp->kref); - return rev; } EXPORT_SYMBOL_GPL(revocable_alloc); diff --git a/drivers/base/revocable_test.c b/drivers/base/revocable_test.c index 873a44082b6c..1622aae92fd3 100644 --- a/drivers/base/revocable_test.c +++ b/drivers/base/revocable_test.c @@ -21,7 +21,7 @@ =20 static void revocable_test_basic(struct kunit *test) { - struct revocable_provider *rp; + struct revocable_provider __rcu *rp; struct revocable *rev; void *real_res =3D (void *)0x12345678, *res; =20 @@ -36,12 +36,13 @@ static void revocable_test_basic(struct kunit *test) revocable_withdraw_access(rev); =20 revocable_free(rev); - revocable_provider_revoke(rp); + revocable_provider_revoke(&rp); + KUNIT_EXPECT_PTR_EQ(test, unrcu_pointer(rp), NULL); } =20 static void revocable_test_revocation(struct kunit *test) { - struct revocable_provider *rp; + struct revocable_provider __rcu *rp; struct revocable *rev; void *real_res =3D (void *)0x12345678, *res; =20 @@ -55,7 +56,8 @@ static void revocable_test_revocation(struct kunit *test) KUNIT_EXPECT_PTR_EQ(test, res, real_res); revocable_withdraw_access(rev); =20 - revocable_provider_revoke(rp); + revocable_provider_revoke(&rp); + KUNIT_EXPECT_PTR_EQ(test, unrcu_pointer(rp), NULL); =20 res =3D revocable_try_access(rev); KUNIT_EXPECT_PTR_EQ(test, res, NULL); @@ -66,7 +68,7 @@ static void revocable_test_revocation(struct kunit *test) =20 static void revocable_test_try_access_macro(struct kunit *test) { - struct revocable_provider *rp; + struct revocable_provider __rcu *rp; struct revocable *rev; void *real_res =3D (void *)0x12345678, *res; =20 @@ -81,7 +83,8 @@ static void revocable_test_try_access_macro(struct kunit = *test) KUNIT_EXPECT_PTR_EQ(test, res, real_res); } =20 - revocable_provider_revoke(rp); + revocable_provider_revoke(&rp); + KUNIT_EXPECT_PTR_EQ(test, unrcu_pointer(rp), NULL); =20 { REVOCABLE_TRY_ACCESS_WITH(rev, res); @@ -93,7 +96,7 @@ static void revocable_test_try_access_macro(struct kunit = *test) =20 static void revocable_test_try_access_macro2(struct kunit *test) { - struct revocable_provider *rp; + struct revocable_provider __rcu *rp; struct revocable *rev; void *real_res =3D (void *)0x12345678, *res; bool accessed; @@ -111,7 +114,8 @@ static void revocable_test_try_access_macro2(struct kun= it *test) } KUNIT_EXPECT_TRUE(test, accessed); =20 - revocable_provider_revoke(rp); + revocable_provider_revoke(&rp); + KUNIT_EXPECT_PTR_EQ(test, unrcu_pointer(rp), NULL); =20 accessed =3D false; REVOCABLE_TRY_ACCESS_SCOPED(rev, res) { diff --git a/include/linux/revocable.h b/include/linux/revocable.h index 659ba01c58db..d5da3584adbe 100644 --- a/include/linux/revocable.h +++ b/include/linux/revocable.h @@ -13,12 +13,10 @@ struct device; struct revocable; struct revocable_provider; =20 -struct revocable_provider *revocable_provider_alloc(void *res); -void revocable_provider_revoke(struct revocable_provider *rp); -struct revocable_provider *devm_revocable_provider_alloc(struct device *de= v, - void *res); +struct revocable_provider __rcu *revocable_provider_alloc(void *res); +void revocable_provider_revoke(struct revocable_provider __rcu **rp); =20 -struct revocable *revocable_alloc(struct revocable_provider *rp); +struct revocable *revocable_alloc(struct revocable_provider __rcu *rp); void revocable_free(struct revocable *rev); void *revocable_try_access(struct revocable *rev) __acquires(&rev->rp->src= u); void revocable_withdraw_access(struct revocable *rev) __releases(&rev->rp-= >srcu); diff --git a/tools/testing/selftests/drivers/base/revocable/test_modules/re= vocable_test.c b/tools/testing/selftests/drivers/base/revocable/test_module= s/revocable_test.c index 1b0692eb75f3..ae6c67e65f3d 100644 --- a/tools/testing/selftests/drivers/base/revocable/test_modules/revocable= _test.c +++ b/tools/testing/selftests/drivers/base/revocable/test_modules/revocable= _test.c @@ -17,7 +17,7 @@ static struct dentry *debugfs_dir; =20 struct revocable_test_provider_priv { - struct revocable_provider *rp; + struct revocable_provider __rcu *rp; struct dentry *dentry; char res[16]; }; @@ -25,7 +25,7 @@ struct revocable_test_provider_priv { static int revocable_test_consumer_open(struct inode *inode, struct file *= filp) { struct revocable *rev; - struct revocable_provider *rp =3D inode->i_private; + struct revocable_provider __rcu *rp =3D inode->i_private; =20 rev =3D revocable_alloc(rp); if (!rev) @@ -106,8 +106,8 @@ static int revocable_test_provider_release(struct inode= *inode, struct revocable_test_provider_priv *priv =3D filp->private_data; =20 debugfs_remove(priv->dentry); - if (priv->rp) - revocable_provider_revoke(priv->rp); + if (unrcu_pointer(priv->rp)) + revocable_provider_revoke(&priv->rp); kfree(priv); =20 return 0; @@ -137,8 +137,8 @@ static ssize_t revocable_test_provider_write(struct fil= e *filp, * gone. */ if (!strcmp(data, TEST_CMD_RESOURCE_GONE)) { - revocable_provider_revoke(priv->rp); - priv->rp =3D NULL; + revocable_provider_revoke(&priv->rp); + rcu_assign_pointer(priv->rp, NULL); } else { if (priv->res[0] !=3D '\0') return 0; @@ -146,14 +146,15 @@ static ssize_t revocable_test_provider_write(struct f= ile *filp, strscpy(priv->res, data); =20 priv->rp =3D revocable_provider_alloc(&priv->res); - if (!priv->rp) + if (!unrcu_pointer(priv->rp)) return -ENOMEM; =20 priv->dentry =3D debugfs_create_file("consumer", 0400, - debugfs_dir, priv->rp, + debugfs_dir, + unrcu_pointer(priv->rp), &revocable_test_consumer_fops); if (!priv->dentry) { - revocable_provider_revoke(priv->rp); + revocable_provider_revoke(&priv->rp); return -ENOMEM; } } --=20 2.53.0.rc1.217.geba53bf80e-goog From nobody Sat Feb 7 23:23:05 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 195BA2874F5; Thu, 29 Jan 2026 14:38:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769697486; cv=none; b=b6fLxhS3pLK2cWNjOAiNXXgKZ7NSJnv/Wve5joKFTSDUL29ApYXRNxysW4nm/OTqprw0o4dNkugngaXt6W1mWYewHm58dwBn7DLljAvQRWbYtONKWnAVQCXC5ufIvqBar2S65aZ5xY0xXITx5hwNLb7t1NrhF5fTdS8gJIesmOw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769697486; c=relaxed/simple; bh=SUGfZS56d2WKVPFhyl+8hQaQtIjs8UXUWjfjEByZb14=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=irTtTLPZixme2VAk3ZRDe1CHp5ypn5APIhBDwPzlYZx/GPK8GdT8wYNCqbLFrRDV9NkKDW1X/IcrqcUGSpsWsIsabItijyTD8Qd3wS94Vm/U3WekYSkLCNGTvqhbn+MILIDe3L86I2w0kDGe/u+M6ID0uJrcJzxfyV1DwTAXT2Q= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=lT6fub4z; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="lT6fub4z" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E67B8C19422; Thu, 29 Jan 2026 14:38:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1769697486; bh=SUGfZS56d2WKVPFhyl+8hQaQtIjs8UXUWjfjEByZb14=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lT6fub4zYlgrI2eNTkAobf+9E0IMW11C5m7km178JRpAG1K1ZKxajTGNQBpcXA1qE G439qoOy5wd1BMeTklPjcsXYDRN6F8JCEgqc7rh+87UyFb5BqSOJYHO8JJor3kYCft zuk54l3izlFAfDeUKoqbK6B5Ez8ZzdSH+k+yvddVCxwccYTndRdKbDy2Vyfgn+eJhE vzyefc8V1ErDmrxo9ftEYAhpG5gGHc3IPfkSLMDvbG1DoKMf1ttrxJVBywsqiaNCNs uKf2fhuvfaJTlHkkqiZi+YXSqlvnjDZ95HzFQI/6QKJkCSMTJmaZQfsFyYhSXUCTMh mX1MtnC/8dnAg== From: Tzung-Bi Shih To: Greg Kroah-Hartman , "Rafael J. Wysocki" , Danilo Krummrich Cc: Jonathan Corbet , Shuah Khan , Laurent Pinchart , Bartosz Golaszewski , Wolfram Sang , Jason Gunthorpe , Johan Hovold , linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, chrome-platform@lists.linux.dev, tzungbi@kernel.org Subject: [PATCH 2/4] revocable: Add KUnit test for provider lifetime races Date: Thu, 29 Jan 2026 14:37:31 +0000 Message-ID: <20260129143733.45618-3-tzungbi@kernel.org> X-Mailer: git-send-email 2.53.0.rc1.217.geba53bf80e-goog In-Reply-To: <20260129143733.45618-1-tzungbi@kernel.org> References: <20260129143733.45618-1-tzungbi@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Add a test to verify that revocable_alloc() correctly handles race conditions where the provider is being released. The test covers three scenarios: 1. Allocating from a NULL provider. 2. Allocating from a provider that has been detached (pointer is NULL). 3. Allocating from a provider that is in the process of destruction (refcount is 0), simulating a race between revocable_alloc() and revocable_provider_release(). A way to run the test: $ ./tools/testing/kunit/kunit.py run \ --kconfig_add CONFIG_REVOCABLE_KUNIT_TEST=3Dy \ --kconfig_add CONFIG_PROVE_LOCKING=3Dy \ --kconfig_add CONFIG_DEBUG_KERNEL=3Dy \ --kconfig_add CONFIG_DEBUG_INFO=3Dy \ --kconfig_add CONFIG_DEBUG_INFO_DWARF5=3Dy \ --kconfig_add CONFIG_KASAN=3Dy \ --kconfig_add CONFIG_DETECT_HUNG_TASK=3Dy \ --kconfig_add CONFIG_DEFAULT_HUNG_TASK_TIMEOUT=3D"10" \ --arch=3Dx86_64 --raw_output=3Dall \ revocable_test Signed-off-by: Tzung-Bi Shih --- drivers/base/revocable_test.c | 41 +++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/drivers/base/revocable_test.c b/drivers/base/revocable_test.c index 1622aae92fd3..7fc4d6a3dff6 100644 --- a/drivers/base/revocable_test.c +++ b/drivers/base/revocable_test.c @@ -14,9 +14,13 @@ * * - Try Access Macro: Same as "Revocation" but uses the * REVOCABLE_TRY_ACCESS_WITH() and REVOCABLE_TRY_ACCESS_SCOPED(). + * + * - Provider Use-after-free: Verifies revocable_alloc() correctly handles + * race conditions where the provider is being released. */ =20 #include +#include #include =20 static void revocable_test_basic(struct kunit *test) @@ -127,11 +131,48 @@ static void revocable_test_try_access_macro2(struct k= unit *test) revocable_free(rev); } =20 +static void revocable_test_provider_use_after_free(struct kunit *test) +{ + struct revocable_provider __rcu *rp; + struct revocable_provider *old_rp; + void *real_res =3D (void *)0x12345678; + struct revocable *rev; + + rp =3D revocable_provider_alloc(real_res); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, rp); + + rev =3D revocable_alloc(NULL); + KUNIT_EXPECT_PTR_EQ(test, rev, NULL); + + /* Simulate the provider has been freed. */ + old_rp =3D rcu_replace_pointer(rp, NULL, 1); + rev =3D revocable_alloc(rp); + KUNIT_EXPECT_PTR_EQ(test, rev, NULL); + rcu_replace_pointer(rp, old_rp, 1); + + struct { + struct srcu_struct srcu; + void __rcu *res; + struct kref kref; + struct rcu_head rcu; + } *rp_internal =3D (void *)rp; + + /* Simulate the provider is releasing. */ + refcount_set(&rp_internal->kref.refcount, 0); + rev =3D revocable_alloc(rp); + KUNIT_EXPECT_PTR_EQ(test, rev, NULL); + refcount_set(&rp_internal->kref.refcount, 1); + + revocable_provider_revoke(&rp); + KUNIT_EXPECT_PTR_EQ(test, unrcu_pointer(rp), NULL); +} + static struct kunit_case revocable_test_cases[] =3D { KUNIT_CASE(revocable_test_basic), KUNIT_CASE(revocable_test_revocation), KUNIT_CASE(revocable_test_try_access_macro), KUNIT_CASE(revocable_test_try_access_macro2), + KUNIT_CASE(revocable_test_provider_use_after_free), {} }; =20 --=20 2.53.0.rc1.217.geba53bf80e-goog From nobody Sat Feb 7 23:23:05 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C99FD305E2E; Thu, 29 Jan 2026 14:38:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769697488; cv=none; b=hAmZf2B051UBDT9klTE4PE8utYgrB/l+1Y+u4pdTbjjD0fLdWrFsFVflwQUVxPHOQQx56LFq4LQeKPw6Bp8RVMyx66zpHRKuuCG2Lri9Cw0oyITheAe0gyi9WOmJv2tTFGTeZoXXEukXrUurN4fzWRFljerpBHgC2o1+M06JrT4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769697488; c=relaxed/simple; bh=oRiqfZx973OCISQ9UG3qJaNQvrA5pa5gYzVvCxDFQSU=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=kGSqhBO8fSsylqtxfZMlA8GmkevffJJvWvnsJeLt3dbX6VQdA+nwnQD3FDRPD2AfqUeBsXSP2wUd3T3DSxFvXVza2dT4vdgSQ7HQQueoF7SK3jl8m8y28E8BfvPHznzYyvepyCQOc0NH98Adv/qrKnz9gYasA0fqiVi4STxBFZk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=VH0fd2vs; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="VH0fd2vs" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 53450C2BC86; Thu, 29 Jan 2026 14:38:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1769697488; bh=oRiqfZx973OCISQ9UG3qJaNQvrA5pa5gYzVvCxDFQSU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=VH0fd2vshJJ0nIksvD12zR3EzcUWOnnQ5XikiyuOAtrj7bJGHO+z2LWC4sboStBOm xUUlk9rAyL1dbaEC1y5lGIyjIoSQKDFYpJavG7PUkczFYD0/NVgBTtVOg8vQEuBZLO KCy3tovupg5YUHBBqFk5o/8zepyTmkZnfReCkETQdVEFem2AEGZXmGOYp7i6p9FxfV gFZXwnbM2Hzb6626+FzeJ0w9NewBgio73A4QQqIaHWiwfyrIwnkDefId4QWtDsFnew Mn3wAaQz0+C9RHMnCWgmj4N0kAQsktlSzSc+BJB1JyS3GQxOFHabXLGqtkhRTn+xI1 8ETVbF/FlDbPA== From: Tzung-Bi Shih To: Greg Kroah-Hartman , "Rafael J. Wysocki" , Danilo Krummrich Cc: Jonathan Corbet , Shuah Khan , Laurent Pinchart , Bartosz Golaszewski , Wolfram Sang , Jason Gunthorpe , Johan Hovold , linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, chrome-platform@lists.linux.dev, tzungbi@kernel.org Subject: [PATCH 3/4] revocable: fix SRCU index corruption by requiring caller-provided storage Date: Thu, 29 Jan 2026 14:37:32 +0000 Message-ID: <20260129143733.45618-4-tzungbi@kernel.org> X-Mailer: git-send-email 2.53.0.rc1.217.geba53bf80e-goog In-Reply-To: <20260129143733.45618-1-tzungbi@kernel.org> References: <20260129143733.45618-1-tzungbi@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" The struct revocable handle stores the SRCU read-side index (idx) for the duration of a resource access. If multiple threads share the same struct revocable instance, they race on writing to the idx field, corrupting the SRCU state and potentially causing unsafe unlocks. Refactor the API to replace revocable_alloc()/revocable_free() with revocable_init()/revocable_deinit(). This change requires the caller to provide the storage for struct revocable. By moving storage ownership to the caller, the API ensures that concurrent users maintain their own private idx storage, eliminating the race condition. Reported-by: Johan Hovold Closes: https://lore.kernel.org/all/20260124170535.11756-4-johan@kernel.org/ Signed-off-by: Tzung-Bi Shih --- .../driver-api/driver-model/revocable.rst | 14 ++-- drivers/base/revocable.c | 41 ++++------- drivers/base/revocable_test.c | 69 +++++++++---------- include/linux/revocable.h | 54 ++++++++++----- .../revocable/test_modules/revocable_test.c | 37 ++++------ 5 files changed, 102 insertions(+), 113 deletions(-) diff --git a/Documentation/driver-api/driver-model/revocable.rst b/Document= ation/driver-api/driver-model/revocable.rst index ab7c0af5fbb6..350e7faeccdc 100644 --- a/Documentation/driver-api/driver-model/revocable.rst +++ b/Documentation/driver-api/driver-model/revocable.rst @@ -81,14 +81,14 @@ For Resource Providers =20 For Resource Consumers ---------------------- -.. kernel-doc:: drivers/base/revocable.c +.. kernel-doc:: include/linux/revocable.h :identifiers: revocable =20 .. kernel-doc:: drivers/base/revocable.c - :identifiers: revocable_alloc + :identifiers: revocable_init =20 .. kernel-doc:: drivers/base/revocable.c - :identifiers: revocable_free + :identifiers: revocable_deinit =20 .. kernel-doc:: drivers/base/revocable.c :identifiers: revocable_try_access @@ -104,11 +104,11 @@ Example Usage =20 .. code-block:: c =20 - void consumer_use_resource(struct revocable *rev) + void consumer_use_resource(struct revocable_provider *rp) { struct foo_resource *res; =20 - REVOCABLE_TRY_ACCESS_WITH(rev, res); + REVOCABLE_TRY_ACCESS_WITH(rp, res); // Always check if the resource is valid. if (!res) { pr_warn("Resource is not available\n"); @@ -129,11 +129,11 @@ Example Usage =20 .. code-block:: c =20 - void consumer_use_resource(struct revocable *rev) + void consumer_use_resource(struct revocable_provider *rp) { struct foo_resource *res; =20 - REVOCABLE_TRY_ACCESS_SCOPED(rev, res) { + REVOCABLE_TRY_ACCESS_SCOPED(rp, res) { // Always check if the resource is valid. if (!res) { pr_warn("Resource is not available\n"); diff --git a/drivers/base/revocable.c b/drivers/base/revocable.c index 1bcd1cf54764..8532ca6a371c 100644 --- a/drivers/base/revocable.c +++ b/drivers/base/revocable.c @@ -73,16 +73,6 @@ struct revocable_provider { struct rcu_head rcu; }; =20 -/** - * struct revocable - A handle for resource consumer. - * @rp: The pointer of resource provider. - * @idx: The index for the RCU critical section. - */ -struct revocable { - struct revocable_provider *rp; - int idx; -}; - /** * revocable_provider_alloc() - Allocate struct revocable_provider. * @res: The pointer of resource. @@ -145,20 +135,20 @@ void revocable_provider_revoke(struct revocable_provi= der __rcu **rp_ptr) EXPORT_SYMBOL_GPL(revocable_provider_revoke); =20 /** - * revocable_alloc() - Allocate struct revocable. + * revocable_init() - Initialize struct revocable. * @_rp: The pointer of resource provider. + * @rev: The pointer of resource consumer. * * This holds a refcount to the resource provider. * - * Return: The pointer of struct revocable. NULL on errors. + * Return: 0 on success, -errno otherwise. */ -struct revocable *revocable_alloc(struct revocable_provider __rcu *_rp) +int revocable_init(struct revocable_provider __rcu *_rp, struct revocable = *rev) { struct revocable_provider *rp; - struct revocable *rev; =20 if (!_rp) - return NULL; + return -ENODEV; =20 /* * Enter a read-side critical section. @@ -170,43 +160,36 @@ struct revocable *revocable_alloc(struct revocable_pr= ovider __rcu *_rp) rp =3D rcu_dereference(_rp); if (!rp) /* The revocable provider has been revoked. */ - return NULL; + return -ENODEV; =20 if (!kref_get_unless_zero(&rp->kref)) /* * The revocable provider is releasing (i.e., * revocable_provider_release() has been called). */ - return NULL; + return -ENODEV; } /* At this point, `rp` is safe to access as holding a kref of it */ =20 - rev =3D kzalloc(sizeof(*rev), GFP_KERNEL); - if (!rev) { - kref_put(&rp->kref, revocable_provider_release); - return NULL; - } - rev->rp =3D rp; - return rev; + return 0; } -EXPORT_SYMBOL_GPL(revocable_alloc); +EXPORT_SYMBOL_GPL(revocable_init); =20 /** - * revocable_free() - Free struct revocable. + * revocable_deinit() - Deinitialize struct revocable. * @rev: The pointer of struct revocable. * * This drops a refcount to the resource provider. If it is the final * reference, revocable_provider_release() will be called to free the stru= ct. */ -void revocable_free(struct revocable *rev) +void revocable_deinit(struct revocable *rev) { struct revocable_provider *rp =3D rev->rp; =20 kref_put(&rp->kref, revocable_provider_release); - kfree(rev); } -EXPORT_SYMBOL_GPL(revocable_free); +EXPORT_SYMBOL_GPL(revocable_deinit); =20 /** * revocable_try_access() - Try to access the resource. diff --git a/drivers/base/revocable_test.c b/drivers/base/revocable_test.c index 7fc4d6a3dff6..a2818ec01298 100644 --- a/drivers/base/revocable_test.c +++ b/drivers/base/revocable_test.c @@ -15,7 +15,7 @@ * - Try Access Macro: Same as "Revocation" but uses the * REVOCABLE_TRY_ACCESS_WITH() and REVOCABLE_TRY_ACCESS_SCOPED(). * - * - Provider Use-after-free: Verifies revocable_alloc() correctly handles + * - Provider Use-after-free: Verifies revocable_init() correctly handles * race conditions where the provider is being released. */ =20 @@ -26,20 +26,21 @@ static void revocable_test_basic(struct kunit *test) { struct revocable_provider __rcu *rp; - struct revocable *rev; + struct revocable rev; void *real_res =3D (void *)0x12345678, *res; + int ret; =20 rp =3D revocable_provider_alloc(real_res); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, rp); =20 - rev =3D revocable_alloc(rp); - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, rev); + ret =3D revocable_init(rp, &rev); + KUNIT_ASSERT_EQ(test, ret, 0); =20 - res =3D revocable_try_access(rev); + res =3D revocable_try_access(&rev); KUNIT_EXPECT_PTR_EQ(test, res, real_res); - revocable_withdraw_access(rev); + revocable_withdraw_access(&rev); =20 - revocable_free(rev); + revocable_deinit(&rev); revocable_provider_revoke(&rp); KUNIT_EXPECT_PTR_EQ(test, unrcu_pointer(rp), NULL); } @@ -47,43 +48,40 @@ static void revocable_test_basic(struct kunit *test) static void revocable_test_revocation(struct kunit *test) { struct revocable_provider __rcu *rp; - struct revocable *rev; + struct revocable rev; void *real_res =3D (void *)0x12345678, *res; + int ret; =20 rp =3D revocable_provider_alloc(real_res); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, rp); =20 - rev =3D revocable_alloc(rp); - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, rev); + ret =3D revocable_init(rp, &rev); + KUNIT_ASSERT_EQ(test, ret, 0); =20 - res =3D revocable_try_access(rev); + res =3D revocable_try_access(&rev); KUNIT_EXPECT_PTR_EQ(test, res, real_res); - revocable_withdraw_access(rev); + revocable_withdraw_access(&rev); =20 revocable_provider_revoke(&rp); KUNIT_EXPECT_PTR_EQ(test, unrcu_pointer(rp), NULL); =20 - res =3D revocable_try_access(rev); + res =3D revocable_try_access(&rev); KUNIT_EXPECT_PTR_EQ(test, res, NULL); - revocable_withdraw_access(rev); + revocable_withdraw_access(&rev); =20 - revocable_free(rev); + revocable_deinit(&rev); } =20 static void revocable_test_try_access_macro(struct kunit *test) { struct revocable_provider __rcu *rp; - struct revocable *rev; void *real_res =3D (void *)0x12345678, *res; =20 rp =3D revocable_provider_alloc(real_res); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, rp); =20 - rev =3D revocable_alloc(rp); - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, rev); - { - REVOCABLE_TRY_ACCESS_WITH(rev, res); + REVOCABLE_TRY_ACCESS_WITH(rp, res); KUNIT_EXPECT_PTR_EQ(test, res, real_res); } =20 @@ -91,28 +89,22 @@ static void revocable_test_try_access_macro(struct kuni= t *test) KUNIT_EXPECT_PTR_EQ(test, unrcu_pointer(rp), NULL); =20 { - REVOCABLE_TRY_ACCESS_WITH(rev, res); + REVOCABLE_TRY_ACCESS_WITH(rp, res); KUNIT_EXPECT_PTR_EQ(test, res, NULL); } - - revocable_free(rev); } =20 static void revocable_test_try_access_macro2(struct kunit *test) { struct revocable_provider __rcu *rp; - struct revocable *rev; void *real_res =3D (void *)0x12345678, *res; bool accessed; =20 rp =3D revocable_provider_alloc(real_res); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, rp); =20 - rev =3D revocable_alloc(rp); - KUNIT_ASSERT_NOT_ERR_OR_NULL(test, rev); - accessed =3D false; - REVOCABLE_TRY_ACCESS_SCOPED(rev, res) { + REVOCABLE_TRY_ACCESS_SCOPED(rp, res) { KUNIT_EXPECT_PTR_EQ(test, res, real_res); accessed =3D true; } @@ -122,32 +114,31 @@ static void revocable_test_try_access_macro2(struct k= unit *test) KUNIT_EXPECT_PTR_EQ(test, unrcu_pointer(rp), NULL); =20 accessed =3D false; - REVOCABLE_TRY_ACCESS_SCOPED(rev, res) { + REVOCABLE_TRY_ACCESS_SCOPED(rp, res) { KUNIT_EXPECT_PTR_EQ(test, res, NULL); accessed =3D true; } KUNIT_EXPECT_TRUE(test, accessed); - - revocable_free(rev); } =20 static void revocable_test_provider_use_after_free(struct kunit *test) { struct revocable_provider __rcu *rp; struct revocable_provider *old_rp; + struct revocable rev; void *real_res =3D (void *)0x12345678; - struct revocable *rev; + int ret; =20 rp =3D revocable_provider_alloc(real_res); KUNIT_ASSERT_NOT_ERR_OR_NULL(test, rp); =20 - rev =3D revocable_alloc(NULL); - KUNIT_EXPECT_PTR_EQ(test, rev, NULL); + ret =3D revocable_init(NULL, &rev); + KUNIT_EXPECT_NE(test, ret, 0); =20 /* Simulate the provider has been freed. */ old_rp =3D rcu_replace_pointer(rp, NULL, 1); - rev =3D revocable_alloc(rp); - KUNIT_EXPECT_PTR_EQ(test, rev, NULL); + ret =3D revocable_init(rp, &rev); + KUNIT_EXPECT_NE(test, ret, 0); rcu_replace_pointer(rp, old_rp, 1); =20 struct { @@ -159,12 +150,14 @@ static void revocable_test_provider_use_after_free(st= ruct kunit *test) =20 /* Simulate the provider is releasing. */ refcount_set(&rp_internal->kref.refcount, 0); - rev =3D revocable_alloc(rp); - KUNIT_EXPECT_PTR_EQ(test, rev, NULL); + ret =3D revocable_init(rp, &rev); + KUNIT_EXPECT_NE(test, ret, 0); refcount_set(&rp_internal->kref.refcount, 1); =20 revocable_provider_revoke(&rp); KUNIT_EXPECT_PTR_EQ(test, unrcu_pointer(rp), NULL); + ret =3D revocable_init(rp, &rev); + KUNIT_EXPECT_NE(test, ret, 0); } =20 static struct kunit_case revocable_test_cases[] =3D { diff --git a/include/linux/revocable.h b/include/linux/revocable.h index d5da3584adbe..e3d6d2c953a3 100644 --- a/include/linux/revocable.h +++ b/include/linux/revocable.h @@ -10,27 +10,46 @@ #include =20 struct device; -struct revocable; struct revocable_provider; =20 +/** + * struct revocable - A handle for resource consumer. + * @rp: The pointer of resource provider. + * @idx: The index for the RCU critical section. + */ +struct revocable { + struct revocable_provider *rp; + int idx; +}; + struct revocable_provider __rcu *revocable_provider_alloc(void *res); void revocable_provider_revoke(struct revocable_provider __rcu **rp); =20 -struct revocable *revocable_alloc(struct revocable_provider __rcu *rp); -void revocable_free(struct revocable *rev); +int revocable_init(struct revocable_provider __rcu *rp, struct revocable *= rev); +void revocable_deinit(struct revocable *rev); void *revocable_try_access(struct revocable *rev) __acquires(&rev->rp->src= u); void revocable_withdraw_access(struct revocable *rev) __releases(&rev->rp-= >srcu); =20 -DEFINE_FREE(access_rev, struct revocable *, if (_T) revocable_withdraw_acc= ess(_T)) +DEFINE_FREE(access_rev, struct revocable *, { + if ((_T)->idx !=3D -1) + revocable_withdraw_access(_T); + if ((_T)->rp) + revocable_deinit(_T); +}) + +#define _REVOCABLE_TRY_ACCESS_WITH(_rp, _rev, _res) \ + struct revocable _rev =3D {.rp =3D NULL, .idx =3D -1}; \ + struct revocable *__UNIQUE_ID(name) __free(access_rev) =3D &_rev; \ + _res =3D revocable_init(_rp, &_rev) ? NULL : revocable_try_access(&_rev) =20 /** * REVOCABLE_TRY_ACCESS_WITH() - A helper for accessing revocable resource - * @_rev: The consumer's ``struct revocable *`` handle. + * @_rp: The provider's ``struct revocable_provider *`` handle. * @_res: A pointer variable that will be assigned the resource. * * The macro simplifies the access-release cycle for consumers, ensuring t= hat - * revocable_withdraw_access() is always called, even in the case of an ea= rly - * exit. + * corresponding revocable_withdraw_access() and revocable_deinit() are ca= lled, + * even in the case of an early exit. * * It creates a local variable in the current scope. @_res is populated w= ith * the result of revocable_try_access(). The consumer code **must** check= if @@ -41,13 +60,15 @@ DEFINE_FREE(access_rev, struct revocable *, if (_T) rev= ocable_withdraw_access(_T * are allowed before the helper. Otherwise, the compiler fails with * "jump bypasses initialization of variable with __attribute__((cleanup))= ". */ -#define REVOCABLE_TRY_ACCESS_WITH(_rev, _res) \ - struct revocable *__UNIQUE_ID(name) __free(access_rev) =3D _rev; \ - _res =3D revocable_try_access(_rev) +#define REVOCABLE_TRY_ACCESS_WITH(_rp, _res) \ + _REVOCABLE_TRY_ACCESS_WITH(_rp, __UNIQUE_ID(name), _res) =20 -#define _REVOCABLE_TRY_ACCESS_SCOPED(_rev, _label, _res) \ - for (struct revocable *__UNIQUE_ID(name) __free(access_rev) =3D _rev; \ - (_res =3D revocable_try_access(_rev)) || true; ({ goto _label; })) \ +#define _REVOCABLE_TRY_ACCESS_SCOPED(_rp, _rev, _label, _res) \ + for (struct revocable _rev =3D {.rp =3D NULL, .idx =3D -1}, \ + *__UNIQUE_ID(name) __free(access_rev) =3D &_rev; \ + (_res =3D revocable_init(_rp, &_rev) ? NULL : \ + revocable_try_access(&_rev)) || true; \ + ({ goto _label; })) \ if (0) { \ _label: \ break; \ @@ -55,13 +76,14 @@ _label: \ =20 /** * REVOCABLE_TRY_ACCESS_SCOPED() - A helper for accessing revocable resour= ce - * @_rev: The consumer's ``struct revocable *`` handle. + * @_rp: The provider's ``struct revocable_provider *`` handle. * @_res: A pointer variable that will be assigned the resource. * * Similar to REVOCABLE_TRY_ACCESS_WITH() but with an explicit scope from a * temporary ``for`` loop. */ -#define REVOCABLE_TRY_ACCESS_SCOPED(_rev, _res) \ - _REVOCABLE_TRY_ACCESS_SCOPED(_rev, __UNIQUE_ID(label), _res) +#define REVOCABLE_TRY_ACCESS_SCOPED(_rp, _res) \ + _REVOCABLE_TRY_ACCESS_SCOPED(_rp, __UNIQUE_ID(name), \ + __UNIQUE_ID(label), _res) =20 #endif /* __LINUX_REVOCABLE_H */ diff --git a/tools/testing/selftests/drivers/base/revocable/test_modules/re= vocable_test.c b/tools/testing/selftests/drivers/base/revocable/test_module= s/revocable_test.c index ae6c67e65f3d..a560ceda7318 100644 --- a/tools/testing/selftests/drivers/base/revocable/test_modules/revocable= _test.c +++ b/tools/testing/selftests/drivers/base/revocable/test_modules/revocable= _test.c @@ -24,23 +24,7 @@ struct revocable_test_provider_priv { =20 static int revocable_test_consumer_open(struct inode *inode, struct file *= filp) { - struct revocable *rev; - struct revocable_provider __rcu *rp =3D inode->i_private; - - rev =3D revocable_alloc(rp); - if (!rev) - return -ENOMEM; - filp->private_data =3D rev; - - return 0; -} - -static int revocable_test_consumer_release(struct inode *inode, - struct file *filp) -{ - struct revocable *rev =3D filp->private_data; - - revocable_free(rev); + filp->private_data =3D inode->i_private; return 0; } =20 @@ -48,25 +32,33 @@ static ssize_t revocable_test_consumer_read(struct file= *filp, char __user *buf, size_t count, loff_t *offset) { + int ret; char *res; char data[16]; size_t len; - struct revocable *rev =3D filp->private_data; + struct revocable rev; + struct revocable_provider __rcu *rp =3D filp->private_data; =20 switch (*offset) { case 0: - res =3D revocable_try_access(rev); + ret =3D revocable_init(rp, &rev); + if (ret) { + snprintf(data, sizeof(data), "%s", "(null)"); + break; + } + res =3D revocable_try_access(&rev); snprintf(data, sizeof(data), "%s", res ?: "(null)"); - revocable_withdraw_access(rev); + revocable_withdraw_access(&rev); + revocable_deinit(&rev); break; case TEST_MAGIC_OFFSET: { - REVOCABLE_TRY_ACCESS_WITH(rev, res); + REVOCABLE_TRY_ACCESS_WITH(rp, res); snprintf(data, sizeof(data), "%s", res ?: "(null)"); } break; case TEST_MAGIC_OFFSET2: - REVOCABLE_TRY_ACCESS_SCOPED(rev, res) + REVOCABLE_TRY_ACCESS_SCOPED(rp, res) snprintf(data, sizeof(data), "%s", res ?: "(null)"); break; default: @@ -83,7 +75,6 @@ static ssize_t revocable_test_consumer_read(struct file *= filp, =20 static const struct file_operations revocable_test_consumer_fops =3D { .open =3D revocable_test_consumer_open, - .release =3D revocable_test_consumer_release, .read =3D revocable_test_consumer_read, .llseek =3D default_llseek, }; --=20 2.53.0.rc1.217.geba53bf80e-goog From nobody Sat Feb 7 23:23:05 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 329322EBDD0; Thu, 29 Jan 2026 14:38:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769697491; cv=none; b=InlhS24aXjgXd+5/DdJ1Mjylj/iZv15gVfQGW/wPHNdfitgZVebGM0ZBCKG8VoCKzdyJf8h5dCRfO7qPVX1z6rucVmqOqgq6+RZ//zW90hnuZdUfnGom+5YPjh5ksEG2wu7nSWSX/D+3pnDnxKe0vhjNqhY0cJ8gTYs4FCIjhDI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769697491; c=relaxed/simple; bh=dOJ6JKliJBVh7PxmWDkV41V8SWn5nzmEj9VT88rYn38=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=iAqxj7Lrm7u64VwSHa7wat3Kz7I3Ik+UtKXz/PXAwMr7ectlKE8JAUxs2jsssQfQcFEhMN7xlgr36kaXVyYPa80NaQgv7AA5TCHgWasBP4DIB7ETqbXxRZtDooe43DRvpxV1d3PJarx6lDiawZKNHTm9uFeeInLYaDL+wmg4CEE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=GfEHdwX+; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="GfEHdwX+" Received: by smtp.kernel.org (Postfix) with ESMTPSA id B3FA1C19422; Thu, 29 Jan 2026 14:38:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1769697490; bh=dOJ6JKliJBVh7PxmWDkV41V8SWn5nzmEj9VT88rYn38=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=GfEHdwX+ryfGzRkhy9lx5n8b5mUbp2mPpyVwp82fdH5DqnDlnax94XfJPcB7nQcWo GRkmlEo+dGCw1a21lVEQ9HwrohEjA0azwkiMahzwvQC8I25yijxRvnPYKd3/mijRB7 hML6UF0SnfKrsT5bzvQTZPyiBiP2Q6GdEL30s1u3ADgIClqkAMtohuJr4khjIFL4M9 CysURdIEjS8Z21bJsuQJ07YOfUQnSPNMBKtAh9nxmQtFA9yk45L13qqotHYF1Tyeyi 3IxxZp6qEgNqLGNmROQSy56OcA9TzTDIU+rwr7vnQ3FswutkJYRon+bfkx8AcupzFG no8t92vzEvIoA== From: Tzung-Bi Shih To: Greg Kroah-Hartman , "Rafael J. Wysocki" , Danilo Krummrich Cc: Jonathan Corbet , Shuah Khan , Laurent Pinchart , Bartosz Golaszewski , Wolfram Sang , Jason Gunthorpe , Johan Hovold , linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, chrome-platform@lists.linux.dev, tzungbi@kernel.org Subject: [PATCH 4/4] revocable: Add KUnit test for concurrent access Date: Thu, 29 Jan 2026 14:37:33 +0000 Message-ID: <20260129143733.45618-5-tzungbi@kernel.org> X-Mailer: git-send-email 2.53.0.rc1.217.geba53bf80e-goog In-Reply-To: <20260129143733.45618-1-tzungbi@kernel.org> References: <20260129143733.45618-1-tzungbi@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Add a test case to verify correct synchronization between concurrent readers and a revocation. The test setup involves: 1. Consumer 1 enters the critical section (SRCU read lock) and verifies access to the resource. 2. Provider attempts to revoke the resource. This should block until Consumer 1 releases the lock. 3. Consumer 2 attempts to enter the critical section while revocation is pending. It should see the resource as revoked (NULL). 4. Consumer 1 exits, allowing the revocation to complete. This ensures that the SRCU mechanism correctly enforces grace periods and that new readers are properly prevented from accessing the resource once revocation has begun. A way to run the test: $ ./tools/testing/kunit/kunit.py run \ --kconfig_add CONFIG_REVOCABLE_KUNIT_TEST=3Dy \ --kconfig_add CONFIG_PROVE_LOCKING=3Dy \ --kconfig_add CONFIG_DEBUG_KERNEL=3Dy \ --kconfig_add CONFIG_DEBUG_INFO=3Dy \ --kconfig_add CONFIG_DEBUG_INFO_DWARF5=3Dy \ --kconfig_add CONFIG_KASAN=3Dy \ --kconfig_add CONFIG_DETECT_HUNG_TASK=3Dy \ --kconfig_add CONFIG_DEFAULT_HUNG_TASK_TIMEOUT=3D"10" \ --arch=3Dx86_64 --raw_output=3Dall \ revocable_test Signed-off-by: Tzung-Bi Shih --- drivers/base/revocable_test.c | 104 ++++++++++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) diff --git a/drivers/base/revocable_test.c b/drivers/base/revocable_test.c index a2818ec01298..27f5d7d96f4b 100644 --- a/drivers/base/revocable_test.c +++ b/drivers/base/revocable_test.c @@ -17,9 +17,14 @@ * * - Provider Use-after-free: Verifies revocable_init() correctly handles * race conditions where the provider is being released. + * + * - Concurrent Access: Verifies multiple threads can access the resource. */ =20 #include +#include +#include +#include #include #include =20 @@ -160,12 +165,111 @@ static void revocable_test_provider_use_after_free(s= truct kunit *test) KUNIT_EXPECT_NE(test, ret, 0); } =20 +struct test_concurrent_access_context { + struct kunit *test; + struct revocable_provider __rcu *rp; + struct revocable rev; + struct completion started, enter, exit; + struct task_struct *thread; + void *expected_res; +}; + +static int test_concurrent_access_provider(void *data) +{ + struct test_concurrent_access_context *ctx =3D data; + + complete(&ctx->started); + + wait_for_completion(&ctx->enter); + revocable_provider_revoke(&ctx->rp); + KUNIT_EXPECT_PTR_EQ(ctx->test, unrcu_pointer(ctx->rp), NULL); + + return 0; +} + +static int test_concurrent_access_consumer(void *data) +{ + struct test_concurrent_access_context *ctx =3D data; + void *res; + + complete(&ctx->started); + + wait_for_completion(&ctx->enter); + res =3D revocable_try_access(&ctx->rev); + KUNIT_EXPECT_PTR_EQ(ctx->test, res, ctx->expected_res); + + wait_for_completion(&ctx->exit); + revocable_withdraw_access(&ctx->rev); + + return 0; +} + +static void revocable_test_concurrent_access(struct kunit *test) +{ + struct revocable_provider __rcu *rp; + void *real_res =3D (void *)0x12345678; + struct test_concurrent_access_context *ctx; + int ret, i; + + rp =3D revocable_provider_alloc(real_res); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, rp); + + ctx =3D kunit_kmalloc_array(test, 3, sizeof(*ctx), GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, ctx); + + for (i =3D 0; i < 3; ++i) { + ctx[i].test =3D test; + init_completion(&ctx[i].started); + init_completion(&ctx[i].enter); + init_completion(&ctx[i].exit); + + if (i =3D=3D 0) { + ctx[i].rp =3D rp; + ctx[i].thread =3D kthread_run( + test_concurrent_access_provider, ctx + i, + "revocable_provider_%d", i); + } else { + ret =3D revocable_init(rp, &ctx[i].rev); + KUNIT_ASSERT_EQ(test, ret, 0); + + ctx[i].thread =3D kthread_run( + test_concurrent_access_consumer, ctx + i, + "revocable_consumer_%d", i); + } + KUNIT_ASSERT_FALSE(test, IS_ERR(ctx[i].thread)); + + wait_for_completion(&ctx[i].started); + } + ctx[1].expected_res =3D real_res; + ctx[2].expected_res =3D NULL; + + /* consumer1 enters read-side critical section */ + complete(&ctx[1].enter); + msleep(100); + /* provider0 revokes the resource */ + complete(&ctx[0].enter); + msleep(100); + /* consumer2 enters read-side critical section */ + complete(&ctx[2].enter); + msleep(100); + + /* consumer{1,2} exit read-side critical section */ + complete(&ctx[1].exit); + complete(&ctx[2].exit); + + for (i =3D 0; i < 3; ++i) + kthread_stop(ctx[i].thread); + for (i =3D 1; i < 3; ++i) + revocable_deinit(&ctx[i].rev); +} + static struct kunit_case revocable_test_cases[] =3D { KUNIT_CASE(revocable_test_basic), KUNIT_CASE(revocable_test_revocation), KUNIT_CASE(revocable_test_try_access_macro), KUNIT_CASE(revocable_test_try_access_macro2), KUNIT_CASE(revocable_test_provider_use_after_free), + KUNIT_CASE(revocable_test_concurrent_access), {} }; =20 --=20 2.53.0.rc1.217.geba53bf80e-goog