From: Lee Jones <lee@kernel.org>
To: lee@kernel.org
Cc: stable@vger.kernel.org,
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
Marek Szyprowski <m.szyprowski@samsung.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: [PATCH v4.19.y 4/6] rpmsg: Fix calling device_lock() on non-initialized device
Date: Tue, 31 Oct 2023 09:30:13 +0000 [thread overview]
Message-ID: <20231031093018.2233640-4-lee@kernel.org> (raw)
In-Reply-To: <20231031093018.2233640-1-lee@kernel.org>
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
commit bb17d110cbf270d5247a6e261c5ad50e362d1675 upstream.
driver_set_override() helper uses device_lock() so it should not be
called before rpmsg_register_device() (which calls device_register()).
Effect can be seen with CONFIG_DEBUG_MUTEXES:
DEBUG_LOCKS_WARN_ON(lock->magic != lock)
WARNING: CPU: 3 PID: 57 at kernel/locking/mutex.c:582 __mutex_lock+0x1ec/0x430
...
Call trace:
__mutex_lock+0x1ec/0x430
mutex_lock_nested+0x44/0x50
driver_set_override+0x124/0x150
qcom_glink_native_probe+0x30c/0x3b0
glink_rpm_probe+0x274/0x350
platform_probe+0x6c/0xe0
really_probe+0x17c/0x3d0
__driver_probe_device+0x114/0x190
driver_probe_device+0x3c/0xf0
...
Refactor the rpmsg_register_device() function to use two-step device
registering (initialization + add) and call driver_set_override() in
proper moment.
This moves the code around, so while at it also NULL-ify the
rpdev->driver_override in error path to be sure it won't be kfree()
second time.
Fixes: 42cd402b8fd4 ("rpmsg: Fix kfree() of static memory on setting driver_override")
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Link: https://lore.kernel.org/r/20220429195946.1061725-2-krzysztof.kozlowski@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Lee Jones <lee@kernel.org>
Change-Id: I37a8db647cfe57fa14deae16bd09cfaa525c897b
---
drivers/rpmsg/rpmsg_core.c | 33 ++++++++++++++++++++++++++++++---
drivers/rpmsg/rpmsg_internal.h | 14 +-------------
include/linux/rpmsg.h | 8 ++++++++
3 files changed, 39 insertions(+), 16 deletions(-)
diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c
index 19c7df92c63e0..3d6427b0edc41 100644
--- a/drivers/rpmsg/rpmsg_core.c
+++ b/drivers/rpmsg/rpmsg_core.c
@@ -525,24 +525,51 @@ static struct bus_type rpmsg_bus = {
.remove = rpmsg_dev_remove,
};
-int rpmsg_register_device(struct rpmsg_device *rpdev)
+/*
+ * A helper for registering rpmsg device with driver override and name.
+ * Drivers should not be using it, but instead rpmsg_register_device().
+ */
+int rpmsg_register_device_override(struct rpmsg_device *rpdev,
+ const char *driver_override)
{
struct device *dev = &rpdev->dev;
int ret;
+ if (driver_override)
+ strcpy(rpdev->id.name, driver_override);
+
dev_set_name(&rpdev->dev, "%s.%s.%d.%d", dev_name(dev->parent),
rpdev->id.name, rpdev->src, rpdev->dst);
rpdev->dev.bus = &rpmsg_bus;
- ret = device_register(&rpdev->dev);
+ device_initialize(dev);
+ if (driver_override) {
+ ret = driver_set_override(dev, &rpdev->driver_override,
+ driver_override,
+ strlen(driver_override));
+ if (ret) {
+ dev_err(dev, "device_set_override failed: %d\n", ret);
+ return ret;
+ }
+ }
+
+ ret = device_add(dev);
if (ret) {
- dev_err(dev, "device_register failed: %d\n", ret);
+ dev_err(dev, "device_add failed: %d\n", ret);
+ kfree(rpdev->driver_override);
+ rpdev->driver_override = NULL;
put_device(&rpdev->dev);
}
return ret;
}
+EXPORT_SYMBOL(rpmsg_register_device_override);
+
+int rpmsg_register_device(struct rpmsg_device *rpdev)
+{
+ return rpmsg_register_device_override(rpdev, NULL);
+}
EXPORT_SYMBOL(rpmsg_register_device);
/*
diff --git a/drivers/rpmsg/rpmsg_internal.h b/drivers/rpmsg/rpmsg_internal.h
index 0b5085ecb815d..ebd53616ef5dc 100644
--- a/drivers/rpmsg/rpmsg_internal.h
+++ b/drivers/rpmsg/rpmsg_internal.h
@@ -83,19 +83,7 @@ struct device *rpmsg_find_device(struct device *parent,
*/
static inline int rpmsg_chrdev_register_device(struct rpmsg_device *rpdev)
{
- int ret;
-
- strcpy(rpdev->id.name, "rpmsg_chrdev");
- ret = driver_set_override(&rpdev->dev, &rpdev->driver_override,
- rpdev->id.name, strlen(rpdev->id.name));
- if (ret)
- return ret;
-
- ret = rpmsg_register_device(rpdev);
- if (ret)
- kfree(rpdev->driver_override);
-
- return ret;
+ return rpmsg_register_device_override(rpdev, "rpmsg_ctrl");
}
#endif
diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h
index 6e7690e20dc51..267533fecbdd9 100644
--- a/include/linux/rpmsg.h
+++ b/include/linux/rpmsg.h
@@ -115,6 +115,8 @@ struct rpmsg_driver {
#if IS_ENABLED(CONFIG_RPMSG)
+int rpmsg_register_device_override(struct rpmsg_device *rpdev,
+ const char *driver_override);
int register_rpmsg_device(struct rpmsg_device *dev);
void unregister_rpmsg_device(struct rpmsg_device *dev);
int __register_rpmsg_driver(struct rpmsg_driver *drv, struct module *owner);
@@ -139,6 +141,12 @@ __poll_t rpmsg_poll(struct rpmsg_endpoint *ept, struct file *filp,
#else
+static inline int rpmsg_register_device_override(struct rpmsg_device *rpdev,
+ const char *driver_override)
+{
+ return -ENXIO;
+}
+
static inline int register_rpmsg_device(struct rpmsg_device *dev)
{
return -ENXIO;
--
2.42.0.820.g83a721a137-goog
next prev parent reply other threads:[~2023-10-31 9:30 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-31 9:30 [PATCH v4.19.y 1/6] driver: platform: Add helper for safer setting of driver_override Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 2/6] rpmsg: Constify local variable in field store macro Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 3/6] rpmsg: Fix kfree() of static memory on setting driver_override Lee Jones
2023-10-31 9:30 ` Lee Jones [this message]
2023-10-31 9:30 ` [PATCH v4.19.y 5/6] rpmsg: glink: Release driver_override Lee Jones
2023-10-31 9:30 ` [PATCH v4.19.y 6/6] rpmsg: Fix possible refcount leak in rpmsg_register_device_override() Lee Jones
2023-10-31 9:36 ` [PATCH v4.19.y 1/6] driver: platform: Add helper for safer setting of driver_override Lee Jones
-- strict thread matches above, loose matches on Subject: below --
2023-10-31 11:39 Lee Jones
2023-10-31 11:39 ` [PATCH v4.19.y 4/6] rpmsg: Fix calling device_lock() on non-initialized device Lee Jones
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231031093018.2233640-4-lee@kernel.org \
--to=lee@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=krzysztof.kozlowski@linaro.org \
--cc=m.szyprowski@samsung.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.