[ Upstream commit 65382585f067d4256ba087934f30f85c9b6984de ]
of_parse_phandle() returns a node pointer with refcount
incremented, we should use of_node_put() on it when done.
Fixes: 53e2822e56c7 ("rpmsg: Introduce Qualcomm SMD backend")
Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Link: https://lore.kernel.org/r/20220511120737.57374-1-linmq006@gmail.com
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit d7bd416d35121c95fe47330e09a5c04adbc5f928 ]
rpmsg_register_device_override need to call put_device to free vch when
driver_set_override fails.
Fix this by adding a put_device() to the error path.
Fixes: bb17d110cbf2 ("rpmsg: Fix calling device_lock() on non-initialized device")
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Hangyu Hua <hbh25y@gmail.com>
Link: https://lore.kernel.org/r/20220624024120.11576-1-hbh25y@gmail.com
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit 353d9214682e65c55cdffad8c82139a3321c5f13 ]
During execution of the worker that's used to register rpmsg devices
we are safely locking the channels mutex but, when creating a new
endpoint for such devices, we are registering a IPI on the SCP, which
then makes the SCP to trigger an interrupt, lock its own mutex and in
turn register more subdevices.
This creates a circular locking dependency situation, as the mtk_rpmsg
channels_lock will then depend on the SCP IPI lock.
[ 15.447736] ======================================================
[ 15.460158] WARNING: possible circular locking dependency detected
[ 15.460161] 5.17.0-next-20220324+ #399 Not tainted
[ 15.460165] ------------------------------------------------------
[ 15.460166] kworker/0:3/155 is trying to acquire lock:
[ 15.460170] ffff5b4d0eaf1308 (&scp->ipi_desc[i].lock){+.+.}-{4:4}, at: scp_ipi_lock+0x34/0x50 [mtk_scp_ipi]
[ 15.504958]
[] but task is already holding lock:
[ 15.504960] ffff5b4d0e8f1918 (&mtk_subdev->channels_lock){+.+.}-{4:4}, at: mtk_register_device_work_function+0x50/0x1cc [mtk_rpmsg]
[ 15.504978]
[] which lock already depends on the new lock.
[ 15.504980]
[] the existing dependency chain (in reverse order) is:
[ 15.504982]
[] -> #1 (&mtk_subdev->channels_lock){+.+.}-{4:4}:
[ 15.504990] lock_acquire+0x68/0x84
[ 15.504999] __mutex_lock+0xa4/0x3e0
[ 15.505007] mutex_lock_nested+0x40/0x70
[ 15.505012] mtk_rpmsg_ns_cb+0xe4/0x134 [mtk_rpmsg]
[ 15.641684] mtk_rpmsg_ipi_handler+0x38/0x64 [mtk_rpmsg]
[ 15.641693] scp_ipi_handler+0xbc/0x180 [mtk_scp]
[ 15.663905] mt8192_scp_irq_handler+0x44/0xa4 [mtk_scp]
[ 15.663915] scp_irq_handler+0x6c/0xa0 [mtk_scp]
[ 15.685779] irq_thread_fn+0x34/0xa0
[ 15.685785] irq_thread+0x18c/0x240
[ 15.685789] kthread+0x104/0x110
[ 15.709579] ret_from_fork+0x10/0x20
[ 15.709586]
[] -> #0 (&scp->ipi_desc[i].lock){+.+.}-{4:4}:
[ 15.731271] __lock_acquire+0x11e4/0x1910
[ 15.740367] lock_acquire.part.0+0xd8/0x220
[ 15.749813] lock_acquire+0x68/0x84
[ 15.757861] __mutex_lock+0xa4/0x3e0
[ 15.766084] mutex_lock_nested+0x40/0x70
[ 15.775006] scp_ipi_lock+0x34/0x50 [mtk_scp_ipi]
[ 15.785503] scp_ipi_register+0x40/0xa4 [mtk_scp_ipi]
[ 15.796697] scp_register_ipi+0x1c/0x30 [mtk_scp]
[ 15.807194] mtk_rpmsg_create_ept+0xa0/0x108 [mtk_rpmsg]
[ 15.818912] rpmsg_create_ept+0x44/0x60
[ 15.827660] cros_ec_rpmsg_probe+0x15c/0x1f0
[ 15.837282] rpmsg_dev_probe+0x128/0x1d0
[ 15.846203] really_probe.part.0+0xa4/0x2a0
[ 15.855649] __driver_probe_device+0xa0/0x150
[ 15.865443] driver_probe_device+0x48/0x150
[ 15.877157] __device_attach_driver+0xc0/0x12c
[ 15.889359] bus_for_each_drv+0x80/0xe0
[ 15.900330] __device_attach+0xe4/0x190
[ 15.911303] device_initial_probe+0x1c/0x2c
[ 15.922969] bus_probe_device+0xa8/0xb0
[ 15.933927] device_add+0x3a8/0x8a0
[ 15.944193] device_register+0x28/0x40
[ 15.954970] rpmsg_register_device+0x5c/0xa0
[ 15.966782] mtk_register_device_work_function+0x148/0x1cc [mtk_rpmsg]
[ 15.983146] process_one_work+0x294/0x664
[ 15.994458] worker_thread+0x7c/0x45c
[ 16.005069] kthread+0x104/0x110
[ 16.014789] ret_from_fork+0x10/0x20
[ 16.025201]
[] other info that might help us debug this:
[ 16.047769] Possible unsafe locking scenario:
[ 16.063942] CPU0 CPU1
[ 16.075166] ---- ----
[ 16.086376] lock(&mtk_subdev->channels_lock);
[ 16.097592] lock(&scp->ipi_desc[i].lock);
[ 16.113188] lock(&mtk_subdev->channels_lock);
[ 16.129482] lock(&scp->ipi_desc[i].lock);
[ 16.140020]
[] *** DEADLOCK ***
[ 16.158282] 4 locks held by kworker/0:3/155:
[ 16.168978] #0: ffff5b4d00008748 ((wq_completion)events){+.+.}-{0:0}, at: process_one_work+0x1fc/0x664
[ 16.190017] #1: ffff80000953bdc8 ((work_completion)(&mtk_subdev->register_work)){+.+.}-{0:0}, at: process_one_work+0x1fc/0x664
[ 16.215269] #2: ffff5b4d0e8f1918 (&mtk_subdev->channels_lock){+.+.}-{4:4}, at: mtk_register_device_work_function+0x50/0x1cc [mtk_rpmsg]
[ 16.242131] #3: ffff5b4d05964190 (&dev->mutex){....}-{4:4}, at: __device_attach+0x44/0x190
To solve this, simply unlock the channels_lock mutex before calling
mtk_rpmsg_register_device() and relock it right after, as safety is
still ensured by the locking mechanism that happens right after
through SCP.
Fixes: 7017996951fd ("rpmsg: add rpmsg support for mt8183 SCP.")
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Link: https://lore.kernel.org/r/20220525091201.14210-1-angelogioacchino.delregno@collabora.com
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Upstream commit abe13e9a561d6b3e82b21362c0d6dd3ecd8a5b13 ]
There is no mutex protection for rpmsg_eptdev_open(),
especially for eptdev->ept read and write operation.
It may cause issues when multiple instances call
rpmsg_eptdev_open() in parallel,the return state
may be success or EBUSY.
Fixes: 964e8bedd5a1 ("rpmsg: char: Return an error if device already open")
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Link: https://lore.kernel.org/r/1653104105-16779-1-git-send-email-shengjiu.wang@nxp.com
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Pull driver core updates from Greg KH:
"Here is the set of driver core changes for 5.19-rc1.
Lots of tiny driver core changes and cleanups happened this cycle, but
the two major things are:
- firmware_loader reorganization and additions including the ability
to have XZ compressed firmware images and the ability for userspace
to initiate the firmware load when it needs to, instead of being
always initiated by the kernel. FPGA devices specifically want this
ability to have their firmware changed over the lifetime of the
system boot, and this allows them to work without having to come up
with yet-another-custom-uapi interface for loading firmware for
them.
- physical location support added to sysfs so that devices that know
this information, can tell userspace where they are located in a
common way. Some ACPI devices already support this today, and more
bus types should support this in the future.
Smaller changes include:
- driver_override api cleanups and fixes
- error path cleanups and fixes
- get_abi script fixes
- deferred probe timeout changes.
It's that last change that I'm the most worried about. It has been
reported to cause boot problems for a number of systems, and I have a
tested patch series that resolves this issue. But I didn't get it
merged into my tree before 5.18-final came out, so it has not gotten
any linux-next testing.
I'll send the fixup patches (there are 2) as a follow-on series to this
pull request.
All have been tested in linux-next for weeks, with no reported issues
other than the above-mentioned boot time-outs"
* tag 'driver-core-5.19-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core: (55 commits)
driver core: fix deadlock in __device_attach
kernfs: Separate kernfs_pr_cont_buf and rename_lock.
topology: Remove unused cpu_cluster_mask()
driver core: Extend deferred probe timeout on driver registration
MAINTAINERS: add Russ Weight as a firmware loader maintainer
driver: base: fix UAF when driver_attach failed
test_firmware: fix end of loop test in upload_read_show()
driver core: location: Add "back" as a possible output for panel
driver core: location: Free struct acpi_pld_info *pld
driver core: Add "*" wildcard support to driver_async_probe cmdline param
driver core: location: Check for allocations failure
arch_topology: Trace the update thermal pressure
kernfs: Rename kernfs_put_open_node to kernfs_unlink_open_file.
export: fix string handling of namespace in EXPORT_SYMBOL_NS
rpmsg: use local 'dev' variable
rpmsg: Fix calling device_lock() on non-initialized device
firmware_loader: describe 'module' parameter of firmware_upload_register()
firmware_loader: Move definitions from sysfs_upload.h to sysfs.h
firmware_loader: Fix configs for sysfs split
selftests: firmware: Add firmware upload selftests
...
'&rpdev->dev' is already cached as local variable, so use it to simplify the code. Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Link: https://lore.kernel.org/r/20220429195946.1061725-3-krzysztof.kozlowski@linaro.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
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>
irq_of_parse_and_map() returns 0 on failure, so this should not be
passed further as error return code.
Fixes: 1a358d350664 ("rpmsg: qcom_smd: Fix irq_of_parse_and_map() return value")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Link: https://lore.kernel.org/r/20220423093932.32136-1-krzysztof.kozlowski@linaro.org
Unregister the rpmsg_ctrl device instead of just freeing the
the virtio_rpmsg_channel structure.
This will properly unregister the device and call
virtio_rpmsg_release_device() that frees the structure.
Fixes: c486682ae1e2 ("rpmsg: virtio: Register the rpmsg_char device")
Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Reviewed-by: Hangyu Hua <hbh25y@gmail.com>
Link: https://lore.kernel.org/r/20220426060536.15594-4-hbh25y@gmail.com
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
vch will be free in virtio_rpmsg_release_device() when
rpmsg_ctrldev_register_device() fails. There is no need to call
kfree() again.
Fixes: c486682ae1e2 ("rpmsg: virtio: Register the rpmsg_char device")
Signed-off-by: Hangyu Hua <hbh25y@gmail.com>
Tested-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Link: https://lore.kernel.org/r/20220426060536.15594-3-hbh25y@gmail.com
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>