Some resources can be removed asynchronously, for example, resources
provided by a hot-pluggable device like USB.  When holding a reference
to such a resource, it's possible for the resource to be removed and
its memory freed, leading to use-after-free errors on subsequent access.

Introduce the ref_proxy library to establish weak references to such
resources.  It allows a resource consumer to safely attempt to access a
resource that might be freed at any time by the resource provider.

The implementation uses a provider/consumer model built on Sleepable
RCU (SRCU) to guarantee safe memory access:

 - A resource provider allocates a struct ref_proxy_provider and
   initializes it with a pointer to the resource.

 - A resource consumer that wants to access the resource allocates a
   struct ref_proxy handle which holds a reference to the provider.

 - To access the resource, the consumer uses ref_proxy_get().  This
   function enters an SRCU read-side critical section and returns the
   pointer to the resource.  If the provider has already freed the
   resource, it returns NULL.  After use, the consumer calls
   ref_proxy_put() to exit the SRCU critical section.  The
   REF_PROXY_GET() is a convenient helper for doing that.

 - When the provider needs to remove the resource, it calls
   ref_proxy_provider_free().  This function sets the internal resource
   pointer to NULL and then calls synchronize_srcu() to wait for all
   current readers to finish before the resource can be completely torn
   down.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 include/linux/ref_proxy.h |  37 ++++++++
 lib/Kconfig               |   3 +
 lib/Makefile              |   1 +
 lib/ref_proxy.c           | 184 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 225 insertions(+)
 create mode 100644 include/linux/ref_proxy.h
 create mode 100644 lib/ref_proxy.c

diff --git a/include/linux/ref_proxy.h b/include/linux/ref_proxy.h
new file mode 100644
index 000000000000..16ff29169272
--- /dev/null
+++ b/include/linux/ref_proxy.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __LINUX_REF_PROXY_H
+#define __LINUX_REF_PROXY_H
+
+#include <linux/cleanup.h>
+
+struct device;
+struct ref_proxy;
+struct ref_proxy_provider;
+
+struct ref_proxy_provider *ref_proxy_provider_alloc(void *ref);
+void ref_proxy_provider_free(struct ref_proxy_provider *rpp);
+struct ref_proxy_provider *devm_ref_proxy_provider_alloc(struct device *dev,
+							 void *ref);
+
+struct ref_proxy *ref_proxy_alloc(struct ref_proxy_provider *rpp);
+void ref_proxy_free(struct ref_proxy *proxy);
+void __rcu *ref_proxy_get(struct ref_proxy *proxy);
+void ref_proxy_put(struct ref_proxy *proxy);
+
+DEFINE_FREE(ref_proxy, struct ref_proxy *, if (_T) ref_proxy_put(_T))
+
+#define _REF_PROXY_GET(_proxy, _name, _label, _ref) \
+	for (struct ref_proxy *_name __free(ref_proxy) = _proxy;	\
+	     (_ref = ref_proxy_get(_name)) || true; ({ goto _label; }))	\
+		if (0) {						\
+_label:									\
+			break;						\
+		} else
+
+#define REF_PROXY_GET(_proxy, _ref)					\
+	_REF_PROXY_GET(_proxy, __UNIQUE_ID(proxy_name),			\
+		       __UNIQUE_ID(label), _ref)
+
+#endif /* __LINUX_REF_PROXY_H */
+
diff --git a/lib/Kconfig b/lib/Kconfig
index c483951b624f..18237a766606 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -583,6 +583,9 @@ config STACKDEPOT_MAX_FRAMES
 	default 64
 	depends on STACKDEPOT
 
+config REF_PROXY
+	bool
+
 config REF_TRACKER
 	bool
 	depends on STACKTRACE_SUPPORT
diff --git a/lib/Makefile b/lib/Makefile
index 392ff808c9b9..e8ad6f67cee9 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -258,6 +258,7 @@ KASAN_SANITIZE_stackdepot.o := n
 KMSAN_SANITIZE_stackdepot.o := n
 KCOV_INSTRUMENT_stackdepot.o := n
 
+obj-$(CONFIG_REF_PROXY) += ref_proxy.o
 obj-$(CONFIG_REF_TRACKER) += ref_tracker.o
 
 libfdt_files = fdt.o fdt_ro.o fdt_wip.o fdt_rw.o fdt_sw.o fdt_strerror.o \
diff --git a/lib/ref_proxy.c b/lib/ref_proxy.c
new file mode 100644
index 000000000000..49940bea651c
--- /dev/null
+++ b/lib/ref_proxy.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/device.h>
+#include <linux/kref.h>
+#include <linux/ref_proxy.h>
+#include <linux/slab.h>
+#include <linux/srcu.h>
+
+/**
+ * struct ref_proxy_provider - A handle for resource provider.
+ * @srcu: The SRCU to protect the resource.
+ * @ref:  The pointer of resource.  It can point to anything.
+ * @kref: The refcount for this handle.
+ */
+struct ref_proxy_provider {
+	struct srcu_struct srcu;
+	void __rcu *ref;
+	struct kref kref;
+};
+
+/**
+ * struct ref_proxy - A handle for resource consumer.
+ * @rpp: The pointer of resource provider.
+ * @idx: The index for the RCU critical section.
+ */
+struct ref_proxy {
+	struct ref_proxy_provider *rpp;
+	int idx;
+};
+
+/**
+ * ref_proxy_provider_alloc() - Allocate struct ref_proxy_provider.
+ * @ref: The pointer of resource.
+ *
+ * This holds an initial refcount to the struct.
+ *
+ * Return: The pointer of struct ref_proxy_provider.  NULL on errors.
+ */
+struct ref_proxy_provider *ref_proxy_provider_alloc(void *ref)
+{
+	struct ref_proxy_provider *rpp;
+
+	rpp = kzalloc(sizeof(*rpp), GFP_KERNEL);
+	if (!rpp)
+		return NULL;
+
+	init_srcu_struct(&rpp->srcu);
+	rcu_assign_pointer(rpp->ref, ref);
+	synchronize_srcu(&rpp->srcu);
+	kref_init(&rpp->kref);
+
+	return rpp;
+}
+EXPORT_SYMBOL(ref_proxy_provider_alloc);
+
+static void ref_proxy_provider_release(struct kref *kref)
+{
+	struct ref_proxy_provider *rpp = container_of(kref,
+			struct ref_proxy_provider, kref);
+
+	cleanup_srcu_struct(&rpp->srcu);
+	kfree(rpp);
+}
+
+/**
+ * ref_proxy_provider_free() - Free struct ref_proxy_provider.
+ * @rpp: The pointer of resource provider.
+ *
+ * This sets the resource `(struct ref_proxy_provider *)->ref` to NULL to
+ * indicate the resource has gone.
+ *
+ * This drops the refcount to the resource provider.  If it is the final
+ * reference, ref_proxy_provider_release() will be called to free the struct.
+ */
+void ref_proxy_provider_free(struct ref_proxy_provider *rpp)
+{
+	rcu_assign_pointer(rpp->ref, NULL);
+	synchronize_srcu(&rpp->srcu);
+	kref_put(&rpp->kref, ref_proxy_provider_release);
+}
+EXPORT_SYMBOL(ref_proxy_provider_free);
+
+static void devm_ref_proxy_provider_free(void *data)
+{
+	struct ref_proxy_provider *rpp = data;
+
+	ref_proxy_provider_free(rpp);
+}
+
+/**
+ * devm_ref_proxy_provider_alloc() - Dev-managed ref_proxy_provider_alloc().
+ * @dev: The device.
+ * @ref: The pointer of resource.
+ *
+ * This holds an initial refcount to the struct.
+ *
+ * Return: The pointer of struct ref_proxy_provider.  NULL on errors.
+ */
+struct ref_proxy_provider *devm_ref_proxy_provider_alloc(struct device *dev,
+							 void *ref)
+{
+	struct ref_proxy_provider *rpp;
+
+	rpp = ref_proxy_provider_alloc(ref);
+	if (rpp)
+		if (devm_add_action_or_reset(dev, devm_ref_proxy_provider_free,
+					     rpp))
+			return NULL;
+
+	return rpp;
+}
+EXPORT_SYMBOL(devm_ref_proxy_provider_alloc);
+
+/**
+ * ref_proxy_alloc() - Allocate struct ref_proxy_provider.
+ * @rpp: The pointer of resource provider.
+ *
+ * This holds a refcount to the resource provider.
+ *
+ * Return: The pointer of struct ref_proxy_provider.  NULL on errors.
+ */
+struct ref_proxy *ref_proxy_alloc(struct ref_proxy_provider *rpp)
+{
+	struct ref_proxy *proxy;
+
+	proxy = kzalloc(sizeof(*proxy), GFP_KERNEL);
+	if (!proxy)
+		return NULL;
+
+	proxy->rpp = rpp;
+	kref_get(&rpp->kref);
+
+	return proxy;
+}
+EXPORT_SYMBOL(ref_proxy_alloc);
+
+/**
+ * ref_proxy_free() - Free struct ref_proxy.
+ * @proxy: The pointer of struct ref_proxy.
+ *
+ * This drops a refcount to the resource provider.  If it is the final
+ * reference, ref_proxy_provider_release() will be called to free the struct.
+ */
+void ref_proxy_free(struct ref_proxy *proxy)
+{
+	struct ref_proxy_provider *rpp = proxy->rpp;
+
+	kref_put(&rpp->kref, ref_proxy_provider_release);
+	kfree(proxy);
+}
+EXPORT_SYMBOL(ref_proxy_free);
+
+/**
+ * ref_proxy_get() - Get the resource.
+ * @proxy: The pointer of struct ref_proxy.
+ *
+ * This tries to de-reference to the resource and enters a RCU critical
+ * section.
+ *
+ * Return: The pointer to the resource.  NULL if the resource has gone.
+ */
+void __rcu *ref_proxy_get(struct ref_proxy *proxy)
+{
+	struct ref_proxy_provider *rpp = proxy->rpp;
+
+	proxy->idx = srcu_read_lock(&rpp->srcu);
+	return rcu_dereference(rpp->ref);
+}
+EXPORT_SYMBOL(ref_proxy_get);
+
+/**
+ * ref_proxy_put() - Put the resource.
+ * @proxy: The pointer of struct ref_proxy.
+ *
+ * Call this function to indicate the resource is no longer used.  It exits
+ * the RCU critical section.
+ */
+void ref_proxy_put(struct ref_proxy *proxy)
+{
+	struct ref_proxy_provider *rpp = proxy->rpp;
+
+	srcu_read_unlock(&rpp->srcu, proxy->idx);
+}
+EXPORT_SYMBOL(ref_proxy_put);
-- 
2.51.0.rc1.163.g2494970778-goog

On Thu Aug 14, 2025 at 11:10 AM CEST, Tzung-Bi Shih wrote:
> Some resources can be removed asynchronously, for example, resources
> provided by a hot-pluggable device like USB.  When holding a reference
> to such a resource, it's possible for the resource to be removed and
> its memory freed, leading to use-after-free errors on subsequent access.
>
> Introduce the ref_proxy library to establish weak references to such
> resources.  It allows a resource consumer to safely attempt to access a
> resource that might be freed at any time by the resource provider.
>
> The implementation uses a provider/consumer model built on Sleepable
> RCU (SRCU) to guarantee safe memory access:
>
>  - A resource provider allocates a struct ref_proxy_provider and
>    initializes it with a pointer to the resource.
>
>  - A resource consumer that wants to access the resource allocates a
>    struct ref_proxy handle which holds a reference to the provider.
>
>  - To access the resource, the consumer uses ref_proxy_get().  This
>    function enters an SRCU read-side critical section and returns the
>    pointer to the resource.  If the provider has already freed the
>    resource, it returns NULL.  After use, the consumer calls
>    ref_proxy_put() to exit the SRCU critical section.  The
>    REF_PROXY_GET() is a convenient helper for doing that.
>
>  - When the provider needs to remove the resource, it calls
>    ref_proxy_provider_free().  This function sets the internal resource
>    pointer to NULL and then calls synchronize_srcu() to wait for all
>    current readers to finish before the resource can be completely torn
>    down.

As mentioned in a sub-thread [1], this is pretty much what we already do in Rust
with Revocable [2].

The Rust struct Devres [3] is built on Revocable, such that a device resource is
only accessible for drivers as long as the device is actually bound to the
driver. Once the device is unbound the resource is "revoked" and drivers are
prevented from subsequent accesses.

I think it's be good to have a common naming scheme for this, hence I propose to
name this struct revocable instead.

Besides that, I'm not exactly sure I understand why we need two structs for this.
struct ref_proxy seems unnecessary. I think we should remove it and rename
struct ref_proxy_provider to struct revocable. Or do I miss anything?

I think it would also be nice to have some more high level documentation on how
it works and how it interacts with devres in the source file, which should be
referenced in Documentation/.

[1] https://lore.kernel.org/lkml/DC22V4IMAJ1Q.3HJUK21LRN5D5@kernel.org/
[2] https://rust.docs.kernel.org/kernel/revocable/struct.Revocable.html
[3] https://rust.docs.kernel.org/kernel/devres/struct.Devres.html

> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
>  include/linux/ref_proxy.h |  37 ++++++++
>  lib/Kconfig               |   3 +
>  lib/Makefile              |   1 +
>  lib/ref_proxy.c           | 184 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 225 insertions(+)
>  create mode 100644 include/linux/ref_proxy.h
>  create mode 100644 lib/ref_proxy.c

I think we should name this revocable and move it to drivers/base/.

> diff --git a/lib/ref_proxy.c b/lib/ref_proxy.c
> new file mode 100644
> index 000000000000..49940bea651c
> --- /dev/null
> +++ b/lib/ref_proxy.c
> @@ -0,0 +1,184 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/device.h>
> +#include <linux/kref.h>
> +#include <linux/ref_proxy.h>
> +#include <linux/slab.h>
> +#include <linux/srcu.h>
> +
> +/**
> + * struct ref_proxy_provider - A handle for resource provider.
> + * @srcu: The SRCU to protect the resource.
> + * @ref:  The pointer of resource.  It can point to anything.
> + * @kref: The refcount for this handle.
> + */
> +struct ref_proxy_provider {
> +	struct srcu_struct srcu;
> +	void __rcu *ref;
> +	struct kref kref;
> +};
> +
> +/**
> + * struct ref_proxy - A handle for resource consumer.
> + * @rpp: The pointer of resource provider.
> + * @idx: The index for the RCU critical section.
> + */
> +struct ref_proxy {
> +	struct ref_proxy_provider *rpp;
> +	int idx;
> +};
> +
> +/**
> + * ref_proxy_provider_alloc() - Allocate struct ref_proxy_provider.
> + * @ref: The pointer of resource.

We should call the pointer res if it's the pointer to the resource.

> + *
> + * This holds an initial refcount to the struct.
> + *
> + * Return: The pointer of struct ref_proxy_provider.  NULL on errors.
> + */
> +struct ref_proxy_provider *ref_proxy_provider_alloc(void *ref)

The ref_proxy_provider owns the resource now, so when the ref_proxy_provider
gets revoked (through the devres callback) the resource must be released, right?
Where is this done? Who is responsible to do so? Shouldn't this function take a
release callback that is called once the ref_proxy_provider is revoked?

> +{
> +	struct ref_proxy_provider *rpp;
> +
> +	rpp = kzalloc(sizeof(*rpp), GFP_KERNEL);
> +	if (!rpp)
> +		return NULL;
> +
> +	init_srcu_struct(&rpp->srcu);
> +	rcu_assign_pointer(rpp->ref, ref);
> +	synchronize_srcu(&rpp->srcu);
> +	kref_init(&rpp->kref);
> +
> +	return rpp;
> +}
> +EXPORT_SYMBOL(ref_proxy_provider_alloc);
> +
> +static void ref_proxy_provider_release(struct kref *kref)
> +{
> +	struct ref_proxy_provider *rpp = container_of(kref,
> +			struct ref_proxy_provider, kref);
> +
> +	cleanup_srcu_struct(&rpp->srcu);

As mentioned above, why aren't we releasing the resource here?

> +	kfree(rpp);
> +}
> +
> +/**
> + * ref_proxy_provider_free() - Free struct ref_proxy_provider.
> + * @rpp: The pointer of resource provider.
> + *
> + * This sets the resource `(struct ref_proxy_provider *)->ref` to NULL to
> + * indicate the resource has gone.
> + *
> + * This drops the refcount to the resource provider.  If it is the final
> + * reference, ref_proxy_provider_release() will be called to free the struct.
> + */
> +void ref_proxy_provider_free(struct ref_proxy_provider *rpp)
> +{
> +	rcu_assign_pointer(rpp->ref, NULL);
> +	synchronize_srcu(&rpp->srcu);

This is called for *every* resource that has been registered with the
ref_proxy_provider for a certain device when it is unbound.

We have the same problem in Rust, and I have a task on my list to address this.
Please see my reply in the sub-thread.

> +	kref_put(&rpp->kref, ref_proxy_provider_release);
> +}
> +EXPORT_SYMBOL(ref_proxy_provider_free);

<snip>

> +/**
> + * ref_proxy_get() - Get the resource.
> + * @proxy: The pointer of struct ref_proxy.
> + *
> + * This tries to de-reference to the resource and enters a RCU critical
> + * section.
> + *
> + * Return: The pointer to the resource.  NULL if the resource has gone.
> + */
> +void __rcu *ref_proxy_get(struct ref_proxy *proxy)

We should call this try_access() rather than get().

> +{
> +	struct ref_proxy_provider *rpp = proxy->rpp;
> +
> +	proxy->idx = srcu_read_lock(&rpp->srcu);
> +	return rcu_dereference(rpp->ref);
> +}
> +EXPORT_SYMBOL(ref_proxy_get);
> +
> +/**
> + * ref_proxy_put() - Put the resource.
> + * @proxy: The pointer of struct ref_proxy.
> + *
> + * Call this function to indicate the resource is no longer used.  It exits
> + * the RCU critical section.
> + */
> +void ref_proxy_put(struct ref_proxy *proxy)

I think this should rather be something like release() instead. We're not
dropping a reference count, but releasing a lock.

> +{
> +	struct ref_proxy_provider *rpp = proxy->rpp;
> +
> +	srcu_read_unlock(&rpp->srcu, proxy->idx);
> +}
> +EXPORT_SYMBOL(ref_proxy_put);
> -- 
> 2.51.0.rc1.163.g2494970778-goog

On Thu, Aug 14, 2025 at 12:55:55PM +0200, Danilo Krummrich wrote:
> On Thu Aug 14, 2025 at 11:10 AM CEST, Tzung-Bi Shih wrote:
> As mentioned in a sub-thread [1], this is pretty much what we already do in Rust
> with Revocable [2].
> 
> The Rust struct Devres [3] is built on Revocable, such that a device resource is
> only accessible for drivers as long as the device is actually bound to the
> driver. Once the device is unbound the resource is "revoked" and drivers are
> prevented from subsequent accesses.
> 
> I think it's be good to have a common naming scheme for this, hence I propose to
> name this struct revocable instead.

Sure, will address all review comments and fix in the next version.

> Besides that, I'm not exactly sure I understand why we need two structs for this.
> struct ref_proxy seems unnecessary. I think we should remove it and rename
> struct ref_proxy_provider to struct revocable. Or do I miss anything?

srcu_read_lock() returns an index [4].  The struct ref_proxy is necessary
for tracking the index for leaving the critical section.

[4] https://elixir.bootlin.com/linux/v6.16/source/kernel/rcu/srcutree.c#L750

> > + *
> > + * This holds an initial refcount to the struct.
> > + *
> > + * Return: The pointer of struct ref_proxy_provider.  NULL on errors.
> > + */
> > +struct ref_proxy_provider *ref_proxy_provider_alloc(void *ref)
> 
> The ref_proxy_provider owns the resource now, so when the ref_proxy_provider
> gets revoked (through the devres callback) the resource must be released, right?
> Where is this done? Who is responsible to do so? Shouldn't this function take a
> release callback that is called once the ref_proxy_provider is revoked?

Thank you, that's a valuable suggestion.  While both approaches are valid,
the current implementation is based on a clear separation of ownership.

The design is that struct ref_proxy_provider doesn't own the resource.
Instead, the resource provider (e.g., cros_ec_spi) is responsible for the
full lifecycle:
* It owns and ultimately releases the resource (e.g., [5]).
* It calls ref_proxy_provider_alloc() to expose the resource.
* It calls ref_proxy_provider_free() to revoke access to the resource.

By doing so, the resource provider doesn't need to create a bunch of
void (*release)(void *) callbacks (if multiple resources are exposing).

[5] https://elixir.bootlin.com/linux/v6.16/source/drivers/platform/chrome/cros_ec_spi.c#L752

On Thu, Aug 14, 2025 at 09:10:18AM +0000, Tzung-Bi Shih wrote:
> Some resources can be removed asynchronously, for example, resources
> provided by a hot-pluggable device like USB.  When holding a reference
> to such a resource, it's possible for the resource to be removed and
> its memory freed, leading to use-after-free errors on subsequent access.
> 
> Introduce the ref_proxy library to establish weak references to such
> resources.  It allows a resource consumer to safely attempt to access a
> resource that might be freed at any time by the resource provider.
> 
> The implementation uses a provider/consumer model built on Sleepable
> RCU (SRCU) to guarantee safe memory access:
> 
>  - A resource provider allocates a struct ref_proxy_provider and
>    initializes it with a pointer to the resource.
> 
>  - A resource consumer that wants to access the resource allocates a
>    struct ref_proxy handle which holds a reference to the provider.
> 
>  - To access the resource, the consumer uses ref_proxy_get().  This
>    function enters an SRCU read-side critical section and returns the
>    pointer to the resource.  If the provider has already freed the
>    resource, it returns NULL.  After use, the consumer calls
>    ref_proxy_put() to exit the SRCU critical section.  The
>    REF_PROXY_GET() is a convenient helper for doing that.
> 
>  - When the provider needs to remove the resource, it calls
>    ref_proxy_provider_free().  This function sets the internal resource
>    pointer to NULL and then calls synchronize_srcu() to wait for all
>    current readers to finish before the resource can be completely torn
>    down.

I've added Danilo here, as hopefully this is doing much the same thing
that his rust code does, but I think it's using different names?
Danilo, any ideas if this matches up with what we have in the driver
core rust code now, and would it help out with the drm drivers as well?

thanks,

greg k-h

On Thu Aug 14, 2025 at 12:05 PM CEST, Greg KH wrote:
> I've added Danilo here, as hopefully this is doing much the same thing
> that his rust code does, but I think it's using different names?

It does exactly the same thing as we do in Rust with Devres [1] and
Revocable [2].

The only difference is that this uses SRCU whereas the Rust stuff uses RCU.

One reason Rust is using RCU rather than SRCU is that in almost all cases
we can avoid the need for synchronization at all, by having a proof through the
type system that we're accessing the resource from a context where we can
guarantee that the device is bound, i.e. we have the guarantee that it can't be
unbound concurrently.

While for the few cases we hit where we can't proove that the device is bound
anyways, we don't need a sleepable context and rather keep the critical section
short in order to not delay device unbind too much.

> Danilo, any ideas if this matches up with what we have in the driver
> core rust code now, and would it help out with the drm drivers as well?

As mentioned it matches almost exactly. However, I don't think it'd be worth
building abstractions for it in Rust and use those instead, we can do it with
less code and in a more ideomatic way in Rust right away.

However, *both* this and the Rust Revocable suffer from the same problem, which
I have on my list to solve. That is, we have a synchronize_{srcu,rcu}() call for
every devres node being dropped on driver unbind. Where, instead, we want devres
to provide two callbacks; the first one where we revoke the resource (in C this
would be the call to rcu_assign_pointer(rpp->ref, NULL)), subsequently call
synchronize_{srcu,rcu}() exactly *once* and then have the second callback to
free / drop all the resources.

I'll also reply to the patch itself.

[1] https://rust.docs.kernel.org/kernel/devres/struct.Devres.html
[2] https://rust.docs.kernel.org/kernel/revocable/struct.Revocable.html

On Thu, Aug 14, 2025 at 09:10:18AM +0000, Tzung-Bi Shih wrote:
> Some resources can be removed asynchronously, for example, resources
> provided by a hot-pluggable device like USB.  When holding a reference
> to such a resource, it's possible for the resource to be removed and
> its memory freed, leading to use-after-free errors on subsequent access.
> 
> Introduce the ref_proxy library to establish weak references to such
> resources.  It allows a resource consumer to safely attempt to access a
> resource that might be freed at any time by the resource provider.
> 
> The implementation uses a provider/consumer model built on Sleepable
> RCU (SRCU) to guarantee safe memory access:
> 
>  - A resource provider allocates a struct ref_proxy_provider and
>    initializes it with a pointer to the resource.
> 
>  - A resource consumer that wants to access the resource allocates a
>    struct ref_proxy handle which holds a reference to the provider.
> 
>  - To access the resource, the consumer uses ref_proxy_get().  This
>    function enters an SRCU read-side critical section and returns the
>    pointer to the resource.  If the provider has already freed the
>    resource, it returns NULL.  After use, the consumer calls
>    ref_proxy_put() to exit the SRCU critical section.  The
>    REF_PROXY_GET() is a convenient helper for doing that.
> 
>  - When the provider needs to remove the resource, it calls
>    ref_proxy_provider_free().  This function sets the internal resource
>    pointer to NULL and then calls synchronize_srcu() to wait for all
>    current readers to finish before the resource can be completely torn
>    down.

Shouldn't this documentation go into the code files as well?  Ideally in
kernel doc format so that everyone can read it in the kernel
documentation output?


> 
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
>  include/linux/ref_proxy.h |  37 ++++++++
>  lib/Kconfig               |   3 +
>  lib/Makefile              |   1 +
>  lib/ref_proxy.c           | 184 ++++++++++++++++++++++++++++++++++++++

Who is going to be the maintainer of these new files?

>  4 files changed, 225 insertions(+)
>  create mode 100644 include/linux/ref_proxy.h
>  create mode 100644 lib/ref_proxy.c
> 
> diff --git a/include/linux/ref_proxy.h b/include/linux/ref_proxy.h
> new file mode 100644
> index 000000000000..16ff29169272
> --- /dev/null
> +++ b/include/linux/ref_proxy.h
> @@ -0,0 +1,37 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

No copyright line?  That's bold, given your employer's normal rules :)

> +
> +#ifndef __LINUX_REF_PROXY_H
> +#define __LINUX_REF_PROXY_H
> +
> +#include <linux/cleanup.h>
> +
> +struct device;
> +struct ref_proxy;
> +struct ref_proxy_provider;
> +
> +struct ref_proxy_provider *ref_proxy_provider_alloc(void *ref);
> +void ref_proxy_provider_free(struct ref_proxy_provider *rpp);
> +struct ref_proxy_provider *devm_ref_proxy_provider_alloc(struct device *dev,
> +							 void *ref);
> +
> +struct ref_proxy *ref_proxy_alloc(struct ref_proxy_provider *rpp);
> +void ref_proxy_free(struct ref_proxy *proxy);
> +void __rcu *ref_proxy_get(struct ref_proxy *proxy);
> +void ref_proxy_put(struct ref_proxy *proxy);
> +
> +DEFINE_FREE(ref_proxy, struct ref_proxy *, if (_T) ref_proxy_put(_T))
> +
> +#define _REF_PROXY_GET(_proxy, _name, _label, _ref) \
> +	for (struct ref_proxy *_name __free(ref_proxy) = _proxy;	\
> +	     (_ref = ref_proxy_get(_name)) || true; ({ goto _label; }))	\
> +		if (0) {						\
> +_label:									\
> +			break;						\
> +		} else
> +
> +#define REF_PROXY_GET(_proxy, _ref)					\
> +	_REF_PROXY_GET(_proxy, __UNIQUE_ID(proxy_name),			\
> +		       __UNIQUE_ID(label), _ref)
> +
> +#endif /* __LINUX_REF_PROXY_H */
> +
> diff --git a/lib/Kconfig b/lib/Kconfig
> index c483951b624f..18237a766606 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -583,6 +583,9 @@ config STACKDEPOT_MAX_FRAMES
>  	default 64
>  	depends on STACKDEPOT
>  
> +config REF_PROXY
> +	bool
> +
>  config REF_TRACKER
>  	bool
>  	depends on STACKTRACE_SUPPORT
> diff --git a/lib/Makefile b/lib/Makefile
> index 392ff808c9b9..e8ad6f67cee9 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -258,6 +258,7 @@ KASAN_SANITIZE_stackdepot.o := n
>  KMSAN_SANITIZE_stackdepot.o := n
>  KCOV_INSTRUMENT_stackdepot.o := n
>  
> +obj-$(CONFIG_REF_PROXY) += ref_proxy.o
>  obj-$(CONFIG_REF_TRACKER) += ref_tracker.o
>  
>  libfdt_files = fdt.o fdt_ro.o fdt_wip.o fdt_rw.o fdt_sw.o fdt_strerror.o \
> diff --git a/lib/ref_proxy.c b/lib/ref_proxy.c
> new file mode 100644
> index 000000000000..49940bea651c
> --- /dev/null
> +++ b/lib/ref_proxy.c
> @@ -0,0 +1,184 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/device.h>

As this deals with struct device, shouldn't it go into drivers/base/ ?

> +#include <linux/kref.h>
> +#include <linux/ref_proxy.h>
> +#include <linux/slab.h>
> +#include <linux/srcu.h>
> +
> +/**
> + * struct ref_proxy_provider - A handle for resource provider.
> + * @srcu: The SRCU to protect the resource.
> + * @ref:  The pointer of resource.  It can point to anything.
> + * @kref: The refcount for this handle.
> + */
> +struct ref_proxy_provider {
> +	struct srcu_struct srcu;
> +	void __rcu *ref;
> +	struct kref kref;
> +};
> +
> +/**
> + * struct ref_proxy - A handle for resource consumer.
> + * @rpp: The pointer of resource provider.
> + * @idx: The index for the RCU critical section.
> + */
> +struct ref_proxy {
> +	struct ref_proxy_provider *rpp;
> +	int idx;
> +};
> +
> +/**
> + * ref_proxy_provider_alloc() - Allocate struct ref_proxy_provider.
> + * @ref: The pointer of resource.
> + *
> + * This holds an initial refcount to the struct.
> + *
> + * Return: The pointer of struct ref_proxy_provider.  NULL on errors.
> + */
> +struct ref_proxy_provider *ref_proxy_provider_alloc(void *ref)
> +{
> +	struct ref_proxy_provider *rpp;
> +
> +	rpp = kzalloc(sizeof(*rpp), GFP_KERNEL);
> +	if (!rpp)
> +		return NULL;
> +
> +	init_srcu_struct(&rpp->srcu);
> +	rcu_assign_pointer(rpp->ref, ref);
> +	synchronize_srcu(&rpp->srcu);
> +	kref_init(&rpp->kref);
> +
> +	return rpp;
> +}
> +EXPORT_SYMBOL(ref_proxy_provider_alloc);

EXPORT_SYMBOL_GPL()?  I have to ask.

> +
> +static void ref_proxy_provider_release(struct kref *kref)
> +{
> +	struct ref_proxy_provider *rpp = container_of(kref,
> +			struct ref_proxy_provider, kref);
> +
> +	cleanup_srcu_struct(&rpp->srcu);
> +	kfree(rpp);
> +}
> +
> +/**
> + * ref_proxy_provider_free() - Free struct ref_proxy_provider.
> + * @rpp: The pointer of resource provider.
> + *
> + * This sets the resource `(struct ref_proxy_provider *)->ref` to NULL to
> + * indicate the resource has gone.
> + *
> + * This drops the refcount to the resource provider.  If it is the final
> + * reference, ref_proxy_provider_release() will be called to free the struct.
> + */
> +void ref_proxy_provider_free(struct ref_proxy_provider *rpp)
> +{
> +	rcu_assign_pointer(rpp->ref, NULL);
> +	synchronize_srcu(&rpp->srcu);
> +	kref_put(&rpp->kref, ref_proxy_provider_release);
> +}
> +EXPORT_SYMBOL(ref_proxy_provider_free);
> +
> +static void devm_ref_proxy_provider_free(void *data)
> +{
> +	struct ref_proxy_provider *rpp = data;
> +
> +	ref_proxy_provider_free(rpp);
> +}
> +
> +/**
> + * devm_ref_proxy_provider_alloc() - Dev-managed ref_proxy_provider_alloc().
> + * @dev: The device.
> + * @ref: The pointer of resource.
> + *
> + * This holds an initial refcount to the struct.
> + *
> + * Return: The pointer of struct ref_proxy_provider.  NULL on errors.
> + */
> +struct ref_proxy_provider *devm_ref_proxy_provider_alloc(struct device *dev,
> +							 void *ref)
> +{
> +	struct ref_proxy_provider *rpp;
> +
> +	rpp = ref_proxy_provider_alloc(ref);
> +	if (rpp)
> +		if (devm_add_action_or_reset(dev, devm_ref_proxy_provider_free,
> +					     rpp))
> +			return NULL;
> +
> +	return rpp;
> +}
> +EXPORT_SYMBOL(devm_ref_proxy_provider_alloc);

Do we really need a devm version?  That feels odd as this should be
doing almost the same thing that devm does, right?  How do they interact
properly?

And do you have a selftest for this thing anywhere?

thanks,

greg k-h

On Thu, Aug 14, 2025 at 12:03:11PM +0200, Greg KH wrote:
> On Thu, Aug 14, 2025 at 09:10:18AM +0000, Tzung-Bi Shih wrote:
> > +/**
> > + * devm_ref_proxy_provider_alloc() - Dev-managed ref_proxy_provider_alloc().
> > + * @dev: The device.
> > + * @ref: The pointer of resource.
> > + *
> > + * This holds an initial refcount to the struct.
> > + *
> > + * Return: The pointer of struct ref_proxy_provider.  NULL on errors.
> > + */
> > +struct ref_proxy_provider *devm_ref_proxy_provider_alloc(struct device *dev,
> > +							 void *ref)
> > +{
> > +	struct ref_proxy_provider *rpp;
> > +
> > +	rpp = ref_proxy_provider_alloc(ref);
> > +	if (rpp)
> > +		if (devm_add_action_or_reset(dev, devm_ref_proxy_provider_free,
> > +					     rpp))
> > +			return NULL;
> > +
> > +	return rpp;
> > +}
> > +EXPORT_SYMBOL(devm_ref_proxy_provider_alloc);
> 
> Do we really need a devm version?  That feels odd as this should be
> doing almost the same thing that devm does, right?  How do they interact
> properly?

ref_proxy is similar to devm.  The key difference is their lifetime: a
devm resource is freed when the device detaches, whereas a
ref_proxy_provider persists as long as it has references from ref_proxy.

Since the resource in my use case is provided by the device, it's more
intuitive to tie the resource's lifetime to the device's.

This devm helper further simplifies the provider code.  Otherwise, the
provider needs to call ref_proxy_provider_free() at its error handling
paths or device .remove() callback.

> And do you have a selftest for this thing anywhere?

Will address all review comments and add tests using kselftest and/or KUnit
in the next version.