revocable: fix SRCU index corruption by requiring caller-provided storage

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 <johan@kernel.org>
Closes: https://lore.kernel.org/all/20260124170535.11756-4-johan@kernel.org/
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
Link: https://patch.msgid.link/20260129143733.45618-4-tzungbi@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

1 files changed, 38 insertions, 16 deletions

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 <linux/cleanup.h>
 
 struct device;
-struct revocable;
 struct revocable_provider;
 
+/**
+ * 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);
 
-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->srcu);
 void revocable_withdraw_access(struct revocable *rev) __releases(&rev->rp->srcu);
 
-DEFINE_FREE(access_rev, struct revocable *, if (_T) revocable_withdraw_access(_T))
+DEFINE_FREE(access_rev, struct revocable *, {
+	if ((_T)->idx != -1)
+		revocable_withdraw_access(_T);
+	if ((_T)->rp)
+		revocable_deinit(_T);
+})
+
+#define _REVOCABLE_TRY_ACCESS_WITH(_rp, _rev, _res)				\
+	struct revocable _rev = {.rp = NULL, .idx = -1};			\
+	struct revocable *__UNIQUE_ID(name) __free(access_rev) = &_rev;		\
+	_res = revocable_init(_rp, &_rev) ? NULL : revocable_try_access(&_rev)
 
 /**
  * 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 that
- * revocable_withdraw_access() is always called, even in the case of an early
- * exit.
+ * corresponding revocable_withdraw_access() and revocable_deinit() are called,
+ * even in the case of an early exit.
  *
  * It creates a local variable in the current scope.  @_res is populated with
  * the result of revocable_try_access().  The consumer code **must** check if
@@ -41,13 +60,15 @@ DEFINE_FREE(access_rev, struct revocable *, if (_T) revocable_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) = _rev;		\
-	_res = revocable_try_access(_rev)
+#define REVOCABLE_TRY_ACCESS_WITH(_rp, _res)					\
+	_REVOCABLE_TRY_ACCESS_WITH(_rp, __UNIQUE_ID(name), _res)
 
-#define _REVOCABLE_TRY_ACCESS_SCOPED(_rev, _label, _res)			\
-	for (struct revocable *__UNIQUE_ID(name) __free(access_rev) = _rev;	\
-	     (_res = revocable_try_access(_rev)) || true; ({ goto _label; }))	\
+#define _REVOCABLE_TRY_ACCESS_SCOPED(_rp, _rev, _label, _res)			\
+	for (struct revocable _rev = {.rp = NULL, .idx = -1},			\
+			      *__UNIQUE_ID(name) __free(access_rev) = &_rev;	\
+	     (_res = revocable_init(_rp, &_rev) ? NULL :			\
+		     revocable_try_access(&_rev)) || true;			\
+	     ({ goto _label; }))						\
 		if (0) {							\
 _label:										\
 			break;							\
@@ -55,13 +76,14 @@ _label:										\
 
 /**
  * REVOCABLE_TRY_ACCESS_SCOPED() - 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.
  *
  * 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)
 
 #endif /* __LINUX_REVOCABLE_H */