* [PATCH 1/3] lib: Add ref_proxy module
2025-08-14 9:10 [PATCH 0/3] platform/chrome: Fix a possible UAF via ref_proxy Tzung-Bi Shih
@ 2025-08-14 9:10 ` Tzung-Bi Shih
2025-08-14 10:03 ` Greg KH
` (2 more replies)
2025-08-14 9:10 ` [PATCH 2/3] platform/chrome: Protect cros_ec_device lifecycle with ref_proxy Tzung-Bi Shih
2025-08-14 9:10 ` [PATCH 3/3] platform/chrome: cros_ec_chardev: Consume cros_ec_device via ref_proxy Tzung-Bi Shih
2 siblings, 3 replies; 13+ messages in thread
From: Tzung-Bi Shih @ 2025-08-14 9:10 UTC (permalink / raw)
To: bleung; +Cc: tzungbi, dawidn, chrome-platform, akpm, gregkh, linux-kernel
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] lib: Add ref_proxy module
2025-08-14 9:10 ` [PATCH 1/3] lib: Add ref_proxy module Tzung-Bi Shih
@ 2025-08-14 10:03 ` Greg KH
2025-08-15 5:35 ` Tzung-Bi Shih
2025-08-14 10:05 ` Greg KH
2025-08-14 10:55 ` Danilo Krummrich
2 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2025-08-14 10:03 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: bleung, dawidn, chrome-platform, akpm, linux-kernel
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] lib: Add ref_proxy module
2025-08-14 10:03 ` Greg KH
@ 2025-08-15 5:35 ` Tzung-Bi Shih
0 siblings, 0 replies; 13+ messages in thread
From: Tzung-Bi Shih @ 2025-08-15 5:35 UTC (permalink / raw)
To: Greg KH; +Cc: bleung, dawidn, chrome-platform, akpm, linux-kernel
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.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] lib: Add ref_proxy module
2025-08-14 9:10 ` [PATCH 1/3] lib: Add ref_proxy module Tzung-Bi Shih
2025-08-14 10:03 ` Greg KH
@ 2025-08-14 10:05 ` Greg KH
2025-08-14 10:27 ` Danilo Krummrich
2025-08-14 10:55 ` Danilo Krummrich
2 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2025-08-14 10:05 UTC (permalink / raw)
To: Tzung-Bi Shih, Danilo Krummrich
Cc: bleung, dawidn, chrome-platform, akpm, linux-kernel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] lib: Add ref_proxy module
2025-08-14 10:05 ` Greg KH
@ 2025-08-14 10:27 ` Danilo Krummrich
0 siblings, 0 replies; 13+ messages in thread
From: Danilo Krummrich @ 2025-08-14 10:27 UTC (permalink / raw)
To: Greg KH; +Cc: Tzung-Bi Shih, bleung, dawidn, chrome-platform, akpm,
linux-kernel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] lib: Add ref_proxy module
2025-08-14 9:10 ` [PATCH 1/3] lib: Add ref_proxy module Tzung-Bi Shih
2025-08-14 10:03 ` Greg KH
2025-08-14 10:05 ` Greg KH
@ 2025-08-14 10:55 ` Danilo Krummrich
2025-08-15 5:36 ` Tzung-Bi Shih
2 siblings, 1 reply; 13+ messages in thread
From: Danilo Krummrich @ 2025-08-14 10:55 UTC (permalink / raw)
To: Tzung-Bi Shih; +Cc: bleung, dawidn, chrome-platform, akpm, gregkh, linux-kernel
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] lib: Add ref_proxy module
2025-08-14 10:55 ` Danilo Krummrich
@ 2025-08-15 5:36 ` Tzung-Bi Shih
0 siblings, 0 replies; 13+ messages in thread
From: Tzung-Bi Shih @ 2025-08-15 5:36 UTC (permalink / raw)
To: Danilo Krummrich
Cc: bleung, dawidn, chrome-platform, akpm, gregkh, linux-kernel
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] platform/chrome: Protect cros_ec_device lifecycle with ref_proxy
2025-08-14 9:10 [PATCH 0/3] platform/chrome: Fix a possible UAF via ref_proxy Tzung-Bi Shih
2025-08-14 9:10 ` [PATCH 1/3] lib: Add ref_proxy module Tzung-Bi Shih
@ 2025-08-14 9:10 ` Tzung-Bi Shih
2025-08-15 13:37 ` kernel test robot
2025-08-14 9:10 ` [PATCH 3/3] platform/chrome: cros_ec_chardev: Consume cros_ec_device via ref_proxy Tzung-Bi Shih
2 siblings, 1 reply; 13+ messages in thread
From: Tzung-Bi Shih @ 2025-08-14 9:10 UTC (permalink / raw)
To: bleung; +Cc: tzungbi, dawidn, chrome-platform, akpm, gregkh, linux-kernel
The cros_ec_device can be unregistered when the underlying device is
removed. Other kernel drivers that interact with the EC may hold a
pointer to the cros_ec_device, creating a risk of a use-after-free
error if the EC device is removed while still being referenced.
To prevent this, leverage the ref_proxy library and convert the
underlying device drivers to resource providers of cros_ec_device.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/platform/chrome/Kconfig | 8 ++++++--
drivers/platform/chrome/cros_ec_i2c.c | 5 +++++
drivers/platform/chrome/cros_ec_ishtp.c | 5 +++++
drivers/platform/chrome/cros_ec_lpc.c | 5 +++++
drivers/platform/chrome/cros_ec_rpmsg.c | 5 +++++
drivers/platform/chrome/cros_ec_spi.c | 4 ++++
drivers/platform/chrome/cros_ec_uart.c | 5 +++++
include/linux/platform_data/cros_ec_proto.h | 3 +++
8 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 2281d6dacc9b..fe7b219093e9 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -88,7 +88,7 @@ config CROS_EC
config CROS_EC_I2C
tristate "ChromeOS Embedded Controller (I2C)"
depends on CROS_EC && I2C
-
+ select REF_PROXY
help
If you say Y here, you get support for talking to the ChromeOS
EC through an I2C bus. This uses a simple byte-level protocol with
@@ -98,6 +98,7 @@ config CROS_EC_I2C
config CROS_EC_RPMSG
tristate "ChromeOS Embedded Controller (rpmsg)"
depends on CROS_EC && RPMSG && OF
+ select REF_PROXY
help
If you say Y here, you get support for talking to the ChromeOS EC
through rpmsg. This uses a simple byte-level protocol with a
@@ -111,6 +112,7 @@ config CROS_EC_ISHTP
tristate "ChromeOS Embedded Controller (ISHTP)"
depends on CROS_EC
depends on INTEL_ISH_HID
+ select REF_PROXY
help
If you say Y here, you get support for talking to the ChromeOS EC
firmware running on Intel Integrated Sensor Hub (ISH), using the
@@ -123,7 +125,7 @@ config CROS_EC_ISHTP
config CROS_EC_SPI
tristate "ChromeOS Embedded Controller (SPI)"
depends on CROS_EC && SPI
-
+ select REF_PROXY
help
If you say Y here, you get support for talking to the ChromeOS EC
through a SPI bus, using a byte-level protocol. Since the EC's
@@ -133,6 +135,7 @@ config CROS_EC_SPI
config CROS_EC_UART
tristate "ChromeOS Embedded Controller (UART)"
depends on CROS_EC && ACPI && SERIAL_DEV_BUS
+ select REF_PROXY
help
If you say Y here, you get support for talking to the ChromeOS EC
through a UART, using a byte-level protocol.
@@ -144,6 +147,7 @@ config CROS_EC_LPC
tristate "ChromeOS Embedded Controller (LPC)"
depends on CROS_EC && ACPI && (X86 || COMPILE_TEST)
depends on HAS_IOPORT
+ select REF_PROXY
help
If you say Y here, you get support for talking to the ChromeOS EC
over an LPC bus, including the LPC Microchip EC (MEC) variant.
diff --git a/drivers/platform/chrome/cros_ec_i2c.c b/drivers/platform/chrome/cros_ec_i2c.c
index 38af97cdaab2..d9ecf27585f1 100644
--- a/drivers/platform/chrome/cros_ec_i2c.c
+++ b/drivers/platform/chrome/cros_ec_i2c.c
@@ -12,6 +12,7 @@
#include <linux/platform_data/cros_ec_commands.h>
#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
+#include <linux/ref_proxy.h>
#include <linux/slab.h>
#include "cros_ec.h"
@@ -296,6 +297,10 @@ static int cros_ec_i2c_probe(struct i2c_client *client)
if (!ec_dev)
return -ENOMEM;
+ ec_dev->ref_proxy_provider = devm_ref_proxy_provider_alloc(dev, ec_dev);
+ if (!ec_dev->ref_proxy_provider)
+ return -ENOMEM;
+
i2c_set_clientdata(client, ec_dev);
ec_dev->dev = dev;
ec_dev->priv = client;
diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c
index 7e7190b30cbb..0b74a5b16b52 100644
--- a/drivers/platform/chrome/cros_ec_ishtp.c
+++ b/drivers/platform/chrome/cros_ec_ishtp.c
@@ -12,6 +12,7 @@
#include <linux/pci.h>
#include <linux/platform_data/cros_ec_commands.h>
#include <linux/platform_data/cros_ec_proto.h>
+#include <linux/ref_proxy.h>
#include <linux/intel-ish-client-if.h>
#include "cros_ec.h"
@@ -547,6 +548,10 @@ static int cros_ec_dev_init(struct ishtp_cl_data *client_data)
if (!ec_dev)
return -ENOMEM;
+ ec_dev->ref_proxy_provider = devm_ref_proxy_provider_alloc(dev, ec_dev);
+ if (!ec_dev->ref_proxy_provider)
+ return -ENOMEM;
+
client_data->ec_dev = ec_dev;
dev->driver_data = ec_dev;
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 7d9a78289c96..8b5de1ad5f2f 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -23,6 +23,7 @@
#include <linux/platform_device.h>
#include <linux/printk.h>
#include <linux/reboot.h>
+#include <linux/ref_proxy.h>
#include <linux/suspend.h>
#include "cros_ec.h"
@@ -641,6 +642,10 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
if (!ec_dev)
return -ENOMEM;
+ ec_dev->ref_proxy_provider = devm_ref_proxy_provider_alloc(dev, ec_dev);
+ if (!ec_dev->ref_proxy_provider)
+ return -ENOMEM;
+
platform_set_drvdata(pdev, ec_dev);
ec_dev->dev = dev;
ec_dev->phys_name = dev_name(dev);
diff --git a/drivers/platform/chrome/cros_ec_rpmsg.c b/drivers/platform/chrome/cros_ec_rpmsg.c
index bc2666491db1..c9be9ba63ead 100644
--- a/drivers/platform/chrome/cros_ec_rpmsg.c
+++ b/drivers/platform/chrome/cros_ec_rpmsg.c
@@ -10,6 +10,7 @@
#include <linux/platform_data/cros_ec_commands.h>
#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
+#include <linux/ref_proxy.h>
#include <linux/rpmsg.h>
#include <linux/slab.h>
@@ -220,6 +221,10 @@ static int cros_ec_rpmsg_probe(struct rpmsg_device *rpdev)
if (!ec_dev)
return -ENOMEM;
+ ec_dev->ref_proxy_provider = devm_ref_proxy_provider_alloc(dev, ec_dev);
+ if (!ec_dev->ref_proxy_provider)
+ return -ENOMEM;
+
ec_rpmsg = devm_kzalloc(dev, sizeof(*ec_rpmsg), GFP_KERNEL);
if (!ec_rpmsg)
return -ENOMEM;
diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
index 8ca0f854e7ac..b0f33f02ec24 100644
--- a/drivers/platform/chrome/cros_ec_spi.c
+++ b/drivers/platform/chrome/cros_ec_spi.c
@@ -10,6 +10,7 @@
#include <linux/platform_data/cros_ec_commands.h>
#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
+#include <linux/ref_proxy.h>
#include <linux/slab.h>
#include <linux/spi/spi.h>
#include <uapi/linux/sched/types.h>
@@ -752,6 +753,9 @@ static int cros_ec_spi_probe(struct spi_device *spi)
ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
if (!ec_dev)
return -ENOMEM;
+ ec_dev->ref_proxy_provider = devm_ref_proxy_provider_alloc(dev, ec_dev);
+ if (!ec_dev->ref_proxy_provider)
+ return -ENOMEM;
/* Check for any DT properties */
cros_ec_spi_dt_probe(ec_spi, dev);
diff --git a/drivers/platform/chrome/cros_ec_uart.c b/drivers/platform/chrome/cros_ec_uart.c
index 19c179d49c90..ce080b464977 100644
--- a/drivers/platform/chrome/cros_ec_uart.c
+++ b/drivers/platform/chrome/cros_ec_uart.c
@@ -13,6 +13,7 @@
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_data/cros_ec_proto.h>
+#include <linux/ref_proxy.h>
#include <linux/serdev.h>
#include <linux/slab.h>
#include <uapi/linux/sched/types.h>
@@ -263,6 +264,10 @@ static int cros_ec_uart_probe(struct serdev_device *serdev)
if (!ec_dev)
return -ENOMEM;
+ ec_dev->ref_proxy_provider = devm_ref_proxy_provider_alloc(dev, ec_dev);
+ if (!ec_dev->ref_proxy_provider)
+ return -ENOMEM;
+
serdev_device_set_drvdata(serdev, ec_dev);
init_waitqueue_head(&ec_uart->response.wait_queue);
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
index 3ec24f445c29..9f5c8fb353b6 100644
--- a/include/linux/platform_data/cros_ec_proto.h
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -158,6 +158,7 @@ struct cros_ec_command {
* @pd: The platform_device used by the mfd driver to interface with the
* PD behind an EC.
* @panic_notifier: EC panic notifier.
+ * @ref_proxy_provider: The ref_proxy_provider to this device.
*/
struct cros_ec_device {
/* These are used by other drivers that want to talk to the EC */
@@ -203,6 +204,8 @@ struct cros_ec_device {
struct platform_device *pd;
struct blocking_notifier_head panic_notifier;
+
+ struct ref_proxy_provider *ref_proxy_provider;
};
/**
--
2.51.0.rc1.163.g2494970778-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/3] platform/chrome: Protect cros_ec_device lifecycle with ref_proxy
2025-08-14 9:10 ` [PATCH 2/3] platform/chrome: Protect cros_ec_device lifecycle with ref_proxy Tzung-Bi Shih
@ 2025-08-15 13:37 ` kernel test robot
0 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2025-08-15 13:37 UTC (permalink / raw)
To: Tzung-Bi Shih, bleung
Cc: oe-kbuild-all, tzungbi, dawidn, chrome-platform, akpm, gregkh,
linux-kernel
Hi Tzung-Bi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on chrome-platform/for-next]
[also build test WARNING on chrome-platform/for-firmware-next akpm-mm/mm-nonmm-unstable akpm-mm/mm-everything linus/master v6.17-rc1 next-20250815]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Tzung-Bi-Shih/lib-Add-ref_proxy-module/20250814-172126
base: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
patch link: https://lore.kernel.org/r/20250814091020.1302888-3-tzungbi%40kernel.org
patch subject: [PATCH 2/3] platform/chrome: Protect cros_ec_device lifecycle with ref_proxy
config: sparc64-randconfig-r113-20250815 (https://download.01.org/0day-ci/archive/20250815/202508152116.hQq8Sw2Y-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 93d24b6b7b148c47a2fa228a4ef31524fa1d9f3f)
reproduce: (https://download.01.org/0day-ci/archive/20250815/202508152116.hQq8Sw2Y-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508152116.hQq8Sw2Y-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> lib/ref_proxy.c:167:16: sparse: sparse: incorrect type in return expression (different address spaces) @@ expected void [noderef] __rcu * @@ got void * @@
lib/ref_proxy.c:167:16: sparse: expected void [noderef] __rcu *
lib/ref_proxy.c:167:16: sparse: got void *
lib/ref_proxy.c: note: in included file (through include/linux/notifier.h, include/linux/memory_hotplug.h, include/linux/mmzone.h, ...):
include/linux/srcu.h:373:9: sparse: sparse: context imbalance in 'ref_proxy_put' - unexpected unlock
vim +167 lib/ref_proxy.c
f2304a9508b177 Tzung-Bi Shih 2025-08-14 152
f2304a9508b177 Tzung-Bi Shih 2025-08-14 153 /**
f2304a9508b177 Tzung-Bi Shih 2025-08-14 154 * ref_proxy_get() - Get the resource.
f2304a9508b177 Tzung-Bi Shih 2025-08-14 155 * @proxy: The pointer of struct ref_proxy.
f2304a9508b177 Tzung-Bi Shih 2025-08-14 156 *
f2304a9508b177 Tzung-Bi Shih 2025-08-14 157 * This tries to de-reference to the resource and enters a RCU critical
f2304a9508b177 Tzung-Bi Shih 2025-08-14 158 * section.
f2304a9508b177 Tzung-Bi Shih 2025-08-14 159 *
f2304a9508b177 Tzung-Bi Shih 2025-08-14 160 * Return: The pointer to the resource. NULL if the resource has gone.
f2304a9508b177 Tzung-Bi Shih 2025-08-14 161 */
f2304a9508b177 Tzung-Bi Shih 2025-08-14 162 void __rcu *ref_proxy_get(struct ref_proxy *proxy)
f2304a9508b177 Tzung-Bi Shih 2025-08-14 163 {
f2304a9508b177 Tzung-Bi Shih 2025-08-14 164 struct ref_proxy_provider *rpp = proxy->rpp;
f2304a9508b177 Tzung-Bi Shih 2025-08-14 165
f2304a9508b177 Tzung-Bi Shih 2025-08-14 166 proxy->idx = srcu_read_lock(&rpp->srcu);
f2304a9508b177 Tzung-Bi Shih 2025-08-14 @167 return rcu_dereference(rpp->ref);
f2304a9508b177 Tzung-Bi Shih 2025-08-14 168 }
f2304a9508b177 Tzung-Bi Shih 2025-08-14 169 EXPORT_SYMBOL(ref_proxy_get);
f2304a9508b177 Tzung-Bi Shih 2025-08-14 170
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] platform/chrome: cros_ec_chardev: Consume cros_ec_device via ref_proxy
2025-08-14 9:10 [PATCH 0/3] platform/chrome: Fix a possible UAF via ref_proxy Tzung-Bi Shih
2025-08-14 9:10 ` [PATCH 1/3] lib: Add ref_proxy module Tzung-Bi Shih
2025-08-14 9:10 ` [PATCH 2/3] platform/chrome: Protect cros_ec_device lifecycle with ref_proxy Tzung-Bi Shih
@ 2025-08-14 9:10 ` Tzung-Bi Shih
2025-08-15 21:06 ` kernel test robot
2025-08-16 11:46 ` kernel test robot
2 siblings, 2 replies; 13+ messages in thread
From: Tzung-Bi Shih @ 2025-08-14 9:10 UTC (permalink / raw)
To: bleung; +Cc: tzungbi, dawidn, chrome-platform, akpm, gregkh, linux-kernel
The cros_ec_chardev driver provides a character device interface to the
ChromeOS EC. A file handle to this device can remain open in userspace
even if the underlying EC device is removed.
This creates a classic use-after-free vulnerability. Any file operation
(ioctl, release, etc.) on the open handle after the EC device has gone
would access a stale pointer, leading to a system crash.
To prevent this, leverage the ref_proxy library and convert
cros_ec_chardev to a resource consumer of cros_ec_device.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/platform/chrome/cros_ec_chardev.c | 125 +++++++++++++++-------
1 file changed, 85 insertions(+), 40 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
index c9d80ad5b57e..f4aa70c8b6d4 100644
--- a/drivers/platform/chrome/cros_ec_chardev.c
+++ b/drivers/platform/chrome/cros_ec_chardev.c
@@ -22,6 +22,7 @@
#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
#include <linux/poll.h>
+#include <linux/ref_proxy.h>
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/uaccess.h>
@@ -32,7 +33,7 @@
#define CROS_MAX_EVENT_LEN PAGE_SIZE
struct chardev_priv {
- struct cros_ec_device *ec_dev;
+ struct ref_proxy *ec_dev_proxy;
struct notifier_block notifier;
wait_queue_head_t wait_event;
unsigned long event_mask;
@@ -55,6 +56,7 @@ static int ec_get_version(struct chardev_priv *priv, char *str, int maxlen)
};
struct ec_response_get_version *resp;
struct cros_ec_command *msg;
+ struct cros_ec_device __rcu *ec_dev;
int ret;
msg = kzalloc(sizeof(*msg) + sizeof(*resp), GFP_KERNEL);
@@ -64,12 +66,19 @@ static int ec_get_version(struct chardev_priv *priv, char *str, int maxlen)
msg->command = EC_CMD_GET_VERSION + priv->cmd_offset;
msg->insize = sizeof(*resp);
- ret = cros_ec_cmd_xfer_status(priv->ec_dev, msg);
- if (ret < 0) {
- snprintf(str, maxlen,
- "Unknown EC version, returned error: %d\n",
- msg->result);
- goto exit;
+ REF_PROXY_GET(priv->ec_dev_proxy, ec_dev) {
+ if (!ec_dev) {
+ ret = -ENODEV;
+ goto exit;
+ }
+
+ ret = cros_ec_cmd_xfer_status(ec_dev, msg);
+ if (ret < 0) {
+ snprintf(str, maxlen,
+ "Unknown EC version, returned error: %d\n",
+ msg->result);
+ goto exit;
+ }
}
resp = (struct ec_response_get_version *)msg->data;
@@ -92,22 +101,30 @@ static int cros_ec_chardev_mkbp_event(struct notifier_block *nb,
{
struct chardev_priv *priv = container_of(nb, struct chardev_priv,
notifier);
- struct cros_ec_device *ec_dev = priv->ec_dev;
+ struct cros_ec_device __rcu *ec_dev;
struct ec_event *event;
- unsigned long event_bit = 1 << ec_dev->event_data.event_type;
- int total_size = sizeof(*event) + ec_dev->event_size;
+ unsigned long event_bit;
+ int total_size;
+
+ REF_PROXY_GET(priv->ec_dev_proxy, ec_dev) {
+ if (!ec_dev)
+ return NOTIFY_DONE;
- if (!(event_bit & priv->event_mask) ||
- (priv->event_len + total_size) > CROS_MAX_EVENT_LEN)
- return NOTIFY_DONE;
+ event_bit = 1 << ec_dev->event_data.event_type;
+ total_size = sizeof(*event) + ec_dev->event_size;
- event = kzalloc(total_size, GFP_KERNEL);
- if (!event)
- return NOTIFY_DONE;
+ if (!(event_bit & priv->event_mask) ||
+ (priv->event_len + total_size) > CROS_MAX_EVENT_LEN)
+ return NOTIFY_DONE;
- event->size = ec_dev->event_size;
- event->event_type = ec_dev->event_data.event_type;
- memcpy(event->data, &ec_dev->event_data.data, ec_dev->event_size);
+ event = kzalloc(total_size, GFP_KERNEL);
+ if (!event)
+ return NOTIFY_DONE;
+
+ event->size = ec_dev->event_size;
+ event->event_type = ec_dev->event_data.event_type;
+ memcpy(event->data, &ec_dev->event_data.data, ec_dev->event_size);
+ }
spin_lock(&priv->wait_event.lock);
list_add_tail(&event->node, &priv->events);
@@ -166,7 +183,12 @@ static int cros_ec_chardev_open(struct inode *inode, struct file *filp)
if (!priv)
return -ENOMEM;
- priv->ec_dev = ec_dev;
+ priv->ec_dev_proxy = ref_proxy_alloc(ec_dev->ref_proxy_provider);
+ if (!priv->ec_dev_proxy) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
priv->cmd_offset = ec->cmd_offset;
filp->private_data = priv;
INIT_LIST_HEAD(&priv->events);
@@ -178,9 +200,14 @@ static int cros_ec_chardev_open(struct inode *inode, struct file *filp)
&priv->notifier);
if (ret) {
dev_err(ec_dev->dev, "failed to register event notifier\n");
- kfree(priv);
+ goto err;
}
+ return 0;
+err:
+ if (priv->ec_dev_proxy)
+ ref_proxy_free(priv->ec_dev_proxy);
+ kfree(priv);
return ret;
}
@@ -251,11 +278,16 @@ static ssize_t cros_ec_chardev_read(struct file *filp, char __user *buffer,
static int cros_ec_chardev_release(struct inode *inode, struct file *filp)
{
struct chardev_priv *priv = filp->private_data;
- struct cros_ec_device *ec_dev = priv->ec_dev;
+ struct cros_ec_device __rcu *ec_dev;
struct ec_event *event, *e;
- blocking_notifier_chain_unregister(&ec_dev->event_notifier,
- &priv->notifier);
+ REF_PROXY_GET(priv->ec_dev_proxy, ec_dev) {
+ if (ec_dev)
+ blocking_notifier_chain_unregister(
+ &ec_dev->event_notifier,
+ &priv->notifier);
+ }
+ ref_proxy_free(priv->ec_dev_proxy);
list_for_each_entry_safe(event, e, &priv->events, node) {
list_del(&event->node);
@@ -273,6 +305,7 @@ static long cros_ec_chardev_ioctl_xcmd(struct chardev_priv *priv, void __user *a
{
struct cros_ec_command *s_cmd;
struct cros_ec_command u_cmd;
+ struct cros_ec_device __rcu *ec_dev;
long ret;
if (copy_from_user(&u_cmd, arg, sizeof(u_cmd)))
@@ -299,10 +332,17 @@ static long cros_ec_chardev_ioctl_xcmd(struct chardev_priv *priv, void __user *a
}
s_cmd->command += priv->cmd_offset;
- ret = cros_ec_cmd_xfer(priv->ec_dev, s_cmd);
- /* Only copy data to userland if data was received. */
- if (ret < 0)
- goto exit;
+ REF_PROXY_GET(priv->ec_dev_proxy, ec_dev) {
+ if (!ec_dev) {
+ ret = -ENODEV;
+ goto exit;
+ }
+
+ ret = cros_ec_cmd_xfer(ec_dev, s_cmd);
+ /* Only copy data to userland if data was received. */
+ if (ret < 0)
+ goto exit;
+ }
if (copy_to_user(arg, s_cmd, sizeof(*s_cmd) + s_cmd->insize))
ret = -EFAULT;
@@ -313,24 +353,29 @@ static long cros_ec_chardev_ioctl_xcmd(struct chardev_priv *priv, void __user *a
static long cros_ec_chardev_ioctl_readmem(struct chardev_priv *priv, void __user *arg)
{
- struct cros_ec_device *ec_dev = priv->ec_dev;
+ struct cros_ec_device __rcu *ec_dev;
struct cros_ec_readmem s_mem = { };
long num;
- /* Not every platform supports direct reads */
- if (!ec_dev->cmd_readmem)
- return -ENOTTY;
+ REF_PROXY_GET(priv->ec_dev_proxy, ec_dev) {
+ if (!ec_dev)
+ return -ENODEV;
- if (copy_from_user(&s_mem, arg, sizeof(s_mem)))
- return -EFAULT;
+ /* Not every platform supports direct reads */
+ if (!ec_dev->cmd_readmem)
+ return -ENOTTY;
- if (s_mem.bytes > sizeof(s_mem.buffer))
- return -EINVAL;
+ if (copy_from_user(&s_mem, arg, sizeof(s_mem)))
+ return -EFAULT;
- num = ec_dev->cmd_readmem(ec_dev, s_mem.offset, s_mem.bytes,
- s_mem.buffer);
- if (num <= 0)
- return num;
+ if (s_mem.bytes > sizeof(s_mem.buffer))
+ return -EINVAL;
+
+ num = ec_dev->cmd_readmem(ec_dev, s_mem.offset, s_mem.bytes,
+ s_mem.buffer);
+ if (num <= 0)
+ return num;
+ }
if (copy_to_user((void __user *)arg, &s_mem, sizeof(s_mem)))
return -EFAULT;
--
2.51.0.rc1.163.g2494970778-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/3] platform/chrome: cros_ec_chardev: Consume cros_ec_device via ref_proxy
2025-08-14 9:10 ` [PATCH 3/3] platform/chrome: cros_ec_chardev: Consume cros_ec_device via ref_proxy Tzung-Bi Shih
@ 2025-08-15 21:06 ` kernel test robot
2025-08-16 11:46 ` kernel test robot
1 sibling, 0 replies; 13+ messages in thread
From: kernel test robot @ 2025-08-15 21:06 UTC (permalink / raw)
To: Tzung-Bi Shih, bleung
Cc: oe-kbuild-all, tzungbi, dawidn, chrome-platform, akpm, gregkh,
linux-kernel
Hi Tzung-Bi,
kernel test robot noticed the following build errors:
[auto build test ERROR on chrome-platform/for-next]
[also build test ERROR on next-20250815]
[cannot apply to chrome-platform/for-firmware-next akpm-mm/mm-nonmm-unstable akpm-mm/mm-everything linus/master v6.17-rc1]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Tzung-Bi-Shih/lib-Add-ref_proxy-module/20250814-172126
base: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
patch link: https://lore.kernel.org/r/20250814091020.1302888-4-tzungbi%40kernel.org
patch subject: [PATCH 3/3] platform/chrome: cros_ec_chardev: Consume cros_ec_device via ref_proxy
config: hexagon-randconfig-r072-20250815 (https://download.01.org/0day-ci/archive/20250816/202508160456.eajfX7vH-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 93d24b6b7b148c47a2fa228a4ef31524fa1d9f3f)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250816/202508160456.eajfX7vH-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508160456.eajfX7vH-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "ref_proxy_get" [drivers/platform/chrome/cros_ec_chardev.ko] undefined!
>> ERROR: modpost: "ref_proxy_put" [drivers/platform/chrome/cros_ec_chardev.ko] undefined!
>> ERROR: modpost: "ref_proxy_alloc" [drivers/platform/chrome/cros_ec_chardev.ko] undefined!
>> ERROR: modpost: "ref_proxy_free" [drivers/platform/chrome/cros_ec_chardev.ko] undefined!
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] platform/chrome: cros_ec_chardev: Consume cros_ec_device via ref_proxy
2025-08-14 9:10 ` [PATCH 3/3] platform/chrome: cros_ec_chardev: Consume cros_ec_device via ref_proxy Tzung-Bi Shih
2025-08-15 21:06 ` kernel test robot
@ 2025-08-16 11:46 ` kernel test robot
1 sibling, 0 replies; 13+ messages in thread
From: kernel test robot @ 2025-08-16 11:46 UTC (permalink / raw)
To: Tzung-Bi Shih, bleung
Cc: oe-kbuild-all, tzungbi, dawidn, chrome-platform, akpm, gregkh,
linux-kernel
Hi Tzung-Bi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on chrome-platform/for-next]
[also build test WARNING on next-20250815]
[cannot apply to chrome-platform/for-firmware-next akpm-mm/mm-nonmm-unstable akpm-mm/mm-everything linus/master v6.17-rc1]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Tzung-Bi-Shih/lib-Add-ref_proxy-module/20250814-172126
base: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next
patch link: https://lore.kernel.org/r/20250814091020.1302888-4-tzungbi%40kernel.org
patch subject: [PATCH 3/3] platform/chrome: cros_ec_chardev: Consume cros_ec_device via ref_proxy
config: i386-randconfig-061-20250815 (https://download.01.org/0day-ci/archive/20250816/202508161910.b7YHKLdh-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14+deb12u1) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250816/202508161910.b7YHKLdh-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508161910.b7YHKLdh-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/platform/chrome/cros_ec_chardev.c:75:47: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct cros_ec_device *ec_dev @@ got struct cros_ec_device [noderef] __rcu *[assigned] ec_dev @@
drivers/platform/chrome/cros_ec_chardev.c:75:47: sparse: expected struct cros_ec_device *ec_dev
drivers/platform/chrome/cros_ec_chardev.c:75:47: sparse: got struct cros_ec_device [noderef] __rcu *[assigned] ec_dev
>> drivers/platform/chrome/cros_ec_chardev.c:126:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got union ec_response_get_next_data_v3 [noderef] __rcu * @@
drivers/platform/chrome/cros_ec_chardev.c:126:17: sparse: expected void const *
drivers/platform/chrome/cros_ec_chardev.c:126:17: sparse: got union ec_response_get_next_data_v3 [noderef] __rcu *
>> drivers/platform/chrome/cros_ec_chardev.c:126:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected void const * @@ got union ec_response_get_next_data_v3 [noderef] __rcu * @@
drivers/platform/chrome/cros_ec_chardev.c:126:17: sparse: expected void const *
drivers/platform/chrome/cros_ec_chardev.c:126:17: sparse: got union ec_response_get_next_data_v3 [noderef] __rcu *
drivers/platform/chrome/cros_ec_chardev.c:126:17: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void const * @@ got union ec_response_get_next_data_v3 [noderef] __rcu * @@
drivers/platform/chrome/cros_ec_chardev.c:126:17: sparse: expected void const *
drivers/platform/chrome/cros_ec_chardev.c:126:17: sparse: got union ec_response_get_next_data_v3 [noderef] __rcu *
>> drivers/platform/chrome/cros_ec_chardev.c:287:42: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct blocking_notifier_head *nh @@ got struct blocking_notifier_head [noderef] __rcu * @@
drivers/platform/chrome/cros_ec_chardev.c:287:42: sparse: expected struct blocking_notifier_head *nh
drivers/platform/chrome/cros_ec_chardev.c:287:42: sparse: got struct blocking_notifier_head [noderef] __rcu *
drivers/platform/chrome/cros_ec_chardev.c:341:40: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct cros_ec_device *ec_dev @@ got struct cros_ec_device [noderef] __rcu *[assigned] ec_dev @@
drivers/platform/chrome/cros_ec_chardev.c:341:40: sparse: expected struct cros_ec_device *ec_dev
drivers/platform/chrome/cros_ec_chardev.c:341:40: sparse: got struct cros_ec_device [noderef] __rcu *[assigned] ec_dev
>> drivers/platform/chrome/cros_ec_chardev.c:374:43: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct cros_ec_device *ec @@ got struct cros_ec_device [noderef] __rcu *[assigned] ec_dev @@
drivers/platform/chrome/cros_ec_chardev.c:374:43: sparse: expected struct cros_ec_device *ec
drivers/platform/chrome/cros_ec_chardev.c:374:43: sparse: got struct cros_ec_device [noderef] __rcu *[assigned] ec_dev
>> drivers/platform/chrome/cros_ec_chardev.c:113:34: sparse: sparse: dereference of noderef expression
drivers/platform/chrome/cros_ec_chardev.c:114:47: sparse: sparse: dereference of noderef expression
drivers/platform/chrome/cros_ec_chardev.c:124:31: sparse: sparse: dereference of noderef expression
drivers/platform/chrome/cros_ec_chardev.c:125:37: sparse: sparse: dereference of noderef expression
drivers/platform/chrome/cros_ec_chardev.c:126:17: sparse: sparse: dereference of noderef expression
drivers/platform/chrome/cros_ec_chardev.c:365:22: sparse: sparse: dereference of noderef expression
drivers/platform/chrome/cros_ec_chardev.c:374:23: sparse: sparse: dereference of noderef expression
vim +75 drivers/platform/chrome/cros_ec_chardev.c
51
52 static int ec_get_version(struct chardev_priv *priv, char *str, int maxlen)
53 {
54 static const char * const current_image_name[] = {
55 "unknown", "read-only", "read-write", "invalid",
56 };
57 struct ec_response_get_version *resp;
58 struct cros_ec_command *msg;
59 struct cros_ec_device __rcu *ec_dev;
60 int ret;
61
62 msg = kzalloc(sizeof(*msg) + sizeof(*resp), GFP_KERNEL);
63 if (!msg)
64 return -ENOMEM;
65
66 msg->command = EC_CMD_GET_VERSION + priv->cmd_offset;
67 msg->insize = sizeof(*resp);
68
69 REF_PROXY_GET(priv->ec_dev_proxy, ec_dev) {
70 if (!ec_dev) {
71 ret = -ENODEV;
72 goto exit;
73 }
74
> 75 ret = cros_ec_cmd_xfer_status(ec_dev, msg);
76 if (ret < 0) {
77 snprintf(str, maxlen,
78 "Unknown EC version, returned error: %d\n",
79 msg->result);
80 goto exit;
81 }
82 }
83
84 resp = (struct ec_response_get_version *)msg->data;
85 if (resp->current_image >= ARRAY_SIZE(current_image_name))
86 resp->current_image = 3; /* invalid */
87
88 snprintf(str, maxlen, "%s\n%s\n%s\n%s\n", CROS_EC_DEV_VERSION,
89 resp->version_string_ro, resp->version_string_rw,
90 current_image_name[resp->current_image]);
91
92 ret = 0;
93 exit:
94 kfree(msg);
95 return ret;
96 }
97
98 static int cros_ec_chardev_mkbp_event(struct notifier_block *nb,
99 unsigned long queued_during_suspend,
100 void *_notify)
101 {
102 struct chardev_priv *priv = container_of(nb, struct chardev_priv,
103 notifier);
104 struct cros_ec_device __rcu *ec_dev;
105 struct ec_event *event;
106 unsigned long event_bit;
107 int total_size;
108
109 REF_PROXY_GET(priv->ec_dev_proxy, ec_dev) {
110 if (!ec_dev)
111 return NOTIFY_DONE;
112
> 113 event_bit = 1 << ec_dev->event_data.event_type;
114 total_size = sizeof(*event) + ec_dev->event_size;
115
116 if (!(event_bit & priv->event_mask) ||
117 (priv->event_len + total_size) > CROS_MAX_EVENT_LEN)
118 return NOTIFY_DONE;
119
120 event = kzalloc(total_size, GFP_KERNEL);
121 if (!event)
122 return NOTIFY_DONE;
123
124 event->size = ec_dev->event_size;
125 event->event_type = ec_dev->event_data.event_type;
> 126 memcpy(event->data, &ec_dev->event_data.data, ec_dev->event_size);
127 }
128
129 spin_lock(&priv->wait_event.lock);
130 list_add_tail(&event->node, &priv->events);
131 priv->event_len += total_size;
132 wake_up_locked(&priv->wait_event);
133 spin_unlock(&priv->wait_event.lock);
134
135 return NOTIFY_OK;
136 }
137
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread