Commit Graph

316 Commits

Author SHA1 Message Date
Ricardo B. Marliere
b03aa6d4e9 rpmsg: core: Make rpmsg_bus const
Now that the driver core can properly handle constant struct bus_type,
move the rpmsg_bus variable to be a constant structure as well,
placing it into read-only memory which can not be modified at runtime.

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Link: https://lore.kernel.org/r/20240204-bus_cleanup-rpmsg-v1-1-1703508c23b7@marliere.net
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
2024-02-05 13:43:22 -07:00
Christophe JAILLET
acc48fee5e rpmsg: Remove usage of the deprecated ida_simple_xx() API
ida_alloc() and ida_free() should be preferred to the deprecated
ida_simple_get() and ida_simple_remove().

Note that the upper limit of ida_simple_get() is exclusive, but the one of
ida_alloc_max() is inclusive. So a -1 has been added when needed.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Link: https://lore.kernel.org/r/c09ee5b66d451bf97d14c167048549aa0824ee06.1705225049.git.christophe.jaillet@wanadoo.fr
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
2024-01-22 18:32:23 -07:00
Xiaolei Wang
d5362c37e1 rpmsg: virtio: Free driver_override when rpmsg_remove()
Free driver_override when rpmsg_remove(), otherwise
the following memory leak will occur:

unreferenced object 0xffff0000d55d7080 (size 128):
  comm "kworker/u8:2", pid 56, jiffies 4294893188 (age 214.272s)
  hex dump (first 32 bytes):
    72 70 6d 73 67 5f 6e 73 00 00 00 00 00 00 00 00  rpmsg_ns........
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<000000009c94c9c1>] __kmem_cache_alloc_node+0x1f8/0x320
    [<000000002300d89b>] __kmalloc_node_track_caller+0x44/0x70
    [<00000000228a60c3>] kstrndup+0x4c/0x90
    [<0000000077158695>] driver_set_override+0xd0/0x164
    [<000000003e9c4ea5>] rpmsg_register_device_override+0x98/0x170
    [<000000001c0c89a8>] rpmsg_ns_register_device+0x24/0x30
    [<000000008bbf8fa2>] rpmsg_probe+0x2e0/0x3ec
    [<00000000e65a68df>] virtio_dev_probe+0x1c0/0x280
    [<00000000443331cc>] really_probe+0xbc/0x2dc
    [<00000000391064b1>] __driver_probe_device+0x78/0xe0
    [<00000000a41c9a5b>] driver_probe_device+0xd8/0x160
    [<000000009c3bd5df>] __device_attach_driver+0xb8/0x140
    [<0000000043cd7614>] bus_for_each_drv+0x7c/0xd4
    [<000000003b929a36>] __device_attach+0x9c/0x19c
    [<00000000a94e0ba8>] device_initial_probe+0x14/0x20
    [<000000003c999637>] bus_probe_device+0xa0/0xac

Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
Fixes: b0b03b8119 ("rpmsg: Release rpmsg devices in backends")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/r/20231215020049.78750-1-xiaolei.wang@windriver.com
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
2023-12-18 10:56:03 -07:00
Justin Stitt
2a6e483ad0 rpmsg: virtio: Replace deprecated strncpy with strscpy/_pad
strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.

This patch replaces 3 callsites of strncpy().

The first two populate the destination buffer `nsm.name` -- which we
expect to be NUL-terminated based on their use with format strings.

Firstly, as I understand it, virtio_rpmsg_announce_create() creates an
rpmsg_ns_msg and sends via:

virtio_rpmsg_bus.c:
336: err = rpmsg_sendto(rpdev->ept, &nsm, sizeof(nsm), RPMSG_NS_ADDR);

... which uses:
virtio_rpmsg_sendto() -> rpmsg_send_offchannel_raw()

... which copies its data into an rpmsg_hdr `msg` in virtio_rpmsg_bus.c
618: memcpy(msg->data, data, len);

This callback is invoked when a message is received from the remote
processor:

rpmsg_ns.c:
30: /* invoked when a name service announcement arrives */
31: static int rpmsg_ns_cb(struct rpmsg_device *rpdev, void *data, int len,
32: 		       void *priv, u32 src)
33: {
34:         struct rpmsg_ns_msg *msg = data;
...
50:         /* don't trust the remote processor for null terminating the name */
51:         msg->name[RPMSG_NAME_SIZE - 1] = '\0';

... which leads into the use of `name` within a format string:
rpmsg_ns.c:
57: dev_info(dev, "%sing channel %s addr 0x%x\n",
58:          rpmsg32_to_cpu(rpdev, msg->flags) & RPMSG_NS_DESTROY ?
59:          "destroy" : "creat", msg->name, chinfo.dst);

We can also observe that `nsm` is not zero-initialized and as such we
should maintain the NUL-padding behavior that strncpy() provides:

virtio_rpmsg_bus.c:
330: struct rpmsg_ns_msg nsm;

Considering the above, a suitable replacement is `strscpy_pad` due to
the fact that it guarantees both NUL-termination and NUL-padding on the
destination buffer.

Now, for the third and final destination buffer rpdev->id.name we can
just go for strscpy() (not _pad()) as rpdev points to &vch->rpdev:
|       rpdev = &vch->rpdev;

... and vch is zero-allocated:
|       vch = kzalloc(sizeof(*vch), GFP_KERNEL);

... this renders any additional NUL-byte assignments (like the ones
strncpy() or strscpy_pad() does) redundant.

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
Link: https://lore.kernel.org/r/20231023-strncpy-drivers-rpmsg-virtio_rpmsg_bus-c-v2-1-dc591c36f5ed@google.com
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
2023-10-23 13:11:07 -06:00
Justin Stitt
ec189da923 rpmsg: Replace deprecated strncpy with strscpy_pad
strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.

We expect chinfo.name to be NUL-terminated based on its use with format
strings:
|       dev_err(&ctrldev->dev, "failed to create %s channel\n", chinfo.name);

Since chinfo is not default initialized, we should NUL-pad the `name`
field so that the behavior is consistent with what strncpy() provides:
|       struct rpmsg_channel_info chinfo;

Considering the above, a suitable replacement is `strscpy_pad` due to
the fact that it guarantees both NUL-termination and NUL-padding on the
destination buffer.

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
Link: https://lore.kernel.org/r/20231020-strncpy-drivers-rpmsg-rpmsg_ns-c-v1-1-99b16b00c36c@google.com
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
2023-10-23 11:57:57 -06:00
Justin Stitt
a9580b9b36 rpmsg: core: Replace deprecated strncpy with strscpy
strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.

We expect chinfo.name to be NUL-terminated based on its usage with
strncmp():

rpmsg_core.c:
389: if (strncmp(chinfo->name, rpdev->id.name, RPMSG_NAME_SIZE))

Moreover, NUL-padding is not required as chinfo has stack default
initialized all fields to zero:

rpmsg_core.c:
539: struct rpmsg_channel_info chinfo = {};

Considering the above, a suitable replacement is `strscpy` [2] due to
the fact that it guarantees NUL-termination on the destination buffer
without unnecessarily NUL-padding.

Also, favor the more idiomatic strscpy() usage of:
(dest, src, sizeof(dest)).

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
Link: https://lore.kernel.org/r/20231020-strncpy-drivers-rpmsg-rpmsg_core-c-v1-1-a86b7930c1cf@google.com
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
2023-10-23 11:57:05 -06:00
Linus Torvalds
3d904704c8 rpmsg updates for v6.6
Add support for the GLINK flow control signals, and expose this to the
 user through the rpmsg_char interface. Add missing kstrdup() failure
 handling during allocation of GLINK channel objects.
 -----BEGIN PGP SIGNATURE-----
 
 iQJJBAABCAAzFiEEBd4DzF816k8JZtUlCx85Pw2ZrcUFAmT1/ygVHGFuZGVyc3Nv
 bkBrZXJuZWwub3JnAAoJEAsfOT8Nma3FGZIQAKT+VxWzqLDqUxqjgbvvHOloGwfy
 NfINvbS//OTC7wVInf8PbjXRUflwxK8j5gcOhnZgfohJM2jOdzO1+hh+A0JPTplj
 VKxNs1U+vbV/bviRDQoJ5RvBDPzfghMFA18L6QmGiRDeTjc7bAW/ZcEu6qTvvBAF
 wYr1zi/F7Dg1me6JkZWbjFg8P9dhYM9MsnAHB6sBsWvvOA9LRsfkheu/ihC4hZwg
 S6elviM/rIJ0D35LBdnSDqCULzXdz/kBgI1lpYlJLeuBJTzOEn51efkbyuRN9xfW
 llEMBZqelAEl+qy8qCTA523HxFRzg6FtLvBbOhbgxnm2Z6J9NEs+elJ4Oe+oDieQ
 5elfISrCJ2gSwV/QmaRr/HTXbh9nNAvEpyn2Z1szRGny1kZJGXx0eQj27SlGJSrX
 GJkbJlzjmOrQUrSl3MOOr2NJCgIMIIDRQQuiz6fg10oMlcYDp7k+vH0wfR90aypO
 187Ze1mSGPsdm19kVObbcGCYlXjVYnqGSpNu/0BSwrdib6skT8qVrzAzRrjC4f1I
 XsOLe56/VTvRmGWVn1GCbkgA39k7KwVIzUqETbty7mKZHHeJH1mzkzP+XkMQQrTc
 3ImGT9gOTSz5ht4XA6+KMicoS5HWxhi7XLGdlYxlzKTcVo6sTElcVpMdtdimCjyB
 MsiKKNa+ns1NUtO+
 =liDV
 -----END PGP SIGNATURE-----

Merge tag 'rpmsg-v6.6' of git://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux

Pull rpmsg updates from Bjorn Andersson:
 "Add support for the GLINK flow control signals, and expose this to the
  user through the rpmsg_char interface. Add missing kstrdup() failure
  handling during allocation of GLINK channel objects"

* tag 'rpmsg-v6.6' of git://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux:
  rpmsg: glink: Avoid dereferencing NULL channel
  rpmsg: glink: Add check for kstrdup
  rpmsg: char: Add RPMSG GET/SET FLOWCONTROL IOCTL support
  rpmsg: glink: Add support to handle signals command
  rpmsg: core: Add signal API support
2023-09-04 15:08:52 -07:00
Bjorn Andersson
d629e5bcdf rpmsg: glink: Avoid dereferencing NULL channel
The newly introduced signal command handler checks for non-existing
channel and print an error message, but then continues on to dereference
that same channel.

Instead abort the handler when no channel is found.

Fixes: a2b73aa512 ("rpmsg: glink: Add support to handle signals command")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/202307160800.sb7gMnL6-lkp@intel.com/
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Link: https://lore.kernel.org/r/20230717165538.1542034-1-quic_bjorande@quicinc.com
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
2023-07-18 10:12:26 -06:00
Jiasheng Jiang
b5c9ee8296 rpmsg: glink: Add check for kstrdup
Add check for the return value of kstrdup() and return the error
if it fails in order to avoid NULL pointer dereference.

Fixes: b4f8e52b89 ("rpmsg: Introduce Qualcomm RPM glink driver")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
Link: https://lore.kernel.org/r/20230619030631.12361-1-jiasheng@iscas.ac.cn
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
2023-07-15 14:59:30 -07:00
Chris Lew
5550201c0f rpmsg: char: Add RPMSG GET/SET FLOWCONTROL IOCTL support
Add RPMSG_GET_OUTGOING_FLOWCONTROL and RPMSG_SET_INCOMING_FLOWCONTROL
IOCTL support for rpmsg char device nodes to get/set the low level
transport signals.

Signed-off-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Deepak Kumar Singh <quic_deesin@quicinc.com>
Signed-off-by: Sarannya S <quic_sarannya@quicinc.com>
Acked-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Link: https://lore.kernel.org/r/1688679698-31274-4-git-send-email-quic_sarannya@quicinc.com
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
2023-07-15 11:35:02 -07:00
Chris Lew
a2b73aa512 rpmsg: glink: Add support to handle signals command
Remote peripherals send signal notifications over glink with commandID 15.

Add support to send and receive the signal command and based signals
enable or disable flow control with remote host.

Signed-off-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Deepak Kumar Singh <quic_deesin@quicinc.com>
Signed-off-by: Sarannya S <quic_sarannya@quicinc.com>
Acked-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Link: https://lore.kernel.org/r/1688679698-31274-3-git-send-email-quic_sarannya@quicinc.com
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
2023-07-15 11:34:56 -07:00
Deepak Kumar Singh
8ce49c2a2a rpmsg: core: Add signal API support
Some transports like Glink support the state notifications between
clients using flow control signals similar to serial protocol signals.
Local glink client drivers can send and receive flow control status
to glink clients running on remote processors.

Add APIs to support sending and receiving of flow control status by
rpmsg clients.

Signed-off-by: Deepak Kumar Singh <quic_deesin@quicinc.com>
Signed-off-by: Sarannya S <quic_sarannya@quicinc.com>
Acked-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Link: https://lore.kernel.org/r/1688679698-31274-2-git-send-email-quic_sarannya@quicinc.com
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
2023-07-15 11:34:49 -07:00
Stephan Gerhold
181563be43 rpmsg: qcom_smd: Use qcom_smem_is_available()
Rather than looking up a dummy item from SMEM, use the new
qcom_smem_is_available() function to make the code more clear
(and reduce the overhead slightly).

Add the same check to qcom_smd_register_edge() as well to ensure that
it only succeeds if SMEM is already available - if a driver calls the
function and SMEM is not available yet then the initial state will be
read incorrectly and the RPMSG devices might never become available.

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
Link: https://lore.kernel.org/r/20230531-rpm-rproc-v3-8-a07dcdefd918@gerhold.net
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
2023-07-13 22:18:56 -07:00
Linus Torvalds
556eb8b791 Driver core changes for 6.4-rc1
Here is the large set of driver core changes for 6.4-rc1.
 
 Once again, a busy development cycle, with lots of changes happening in
 the driver core in the quest to be able to move "struct bus" and "struct
 class" into read-only memory, a task now complete with these changes.
 
 This will make the future rust interactions with the driver core more
 "provably correct" as well as providing more obvious lifetime rules for
 all busses and classes in the kernel.
 
 The changes required for this did touch many individual classes and
 busses as many callbacks were changed to take const * parameters
 instead.  All of these changes have been submitted to the various
 subsystem maintainers, giving them plenty of time to review, and most of
 them actually did so.
 
 Other than those changes, included in here are a small set of other
 things:
   - kobject logging improvements
   - cacheinfo improvements and updates
   - obligatory fw_devlink updates and fixes
   - documentation updates
   - device property cleanups and const * changes
   - firwmare loader dependency fixes.
 
 All of these have been in linux-next for a while with no reported
 problems.
 
 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
 -----BEGIN PGP SIGNATURE-----
 
 iG0EABECAC0WIQT0tgzFv3jCIUoxPcsxR9QN2y37KQUCZEp7Sw8cZ3JlZ0Brcm9h
 aC5jb20ACgkQMUfUDdst+ykitQCfamUHpxGcKOAGuLXMotXNakTEsxgAoIquENm5
 LEGadNS38k5fs+73UaxV
 =7K4B
 -----END PGP SIGNATURE-----

Merge tag 'driver-core-6.4-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core

Pull driver core updates from Greg KH:
 "Here is the large set of driver core changes for 6.4-rc1.

  Once again, a busy development cycle, with lots of changes happening
  in the driver core in the quest to be able to move "struct bus" and
  "struct class" into read-only memory, a task now complete with these
  changes.

  This will make the future rust interactions with the driver core more
  "provably correct" as well as providing more obvious lifetime rules
  for all busses and classes in the kernel.

  The changes required for this did touch many individual classes and
  busses as many callbacks were changed to take const * parameters
  instead. All of these changes have been submitted to the various
  subsystem maintainers, giving them plenty of time to review, and most
  of them actually did so.

  Other than those changes, included in here are a small set of other
  things:

   - kobject logging improvements

   - cacheinfo improvements and updates

   - obligatory fw_devlink updates and fixes

   - documentation updates

   - device property cleanups and const * changes

   - firwmare loader dependency fixes.

  All of these have been in linux-next for a while with no reported
  problems"

* tag 'driver-core-6.4-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core: (120 commits)
  device property: make device_property functions take const device *
  driver core: update comments in device_rename()
  driver core: Don't require dynamic_debug for initcall_debug probe timing
  firmware_loader: rework crypto dependencies
  firmware_loader: Strip off \n from customized path
  zram: fix up permission for the hot_add sysfs file
  cacheinfo: Add use_arch[|_cache]_info field/function
  arch_topology: Remove early cacheinfo error message if -ENOENT
  cacheinfo: Check cache properties are present in DT
  cacheinfo: Check sib_leaf in cache_leaves_are_shared()
  cacheinfo: Allow early level detection when DT/ACPI info is missing/broken
  cacheinfo: Add arm64 early level initializer implementation
  cacheinfo: Add arch specific early level initializer
  tty: make tty_class a static const structure
  driver core: class: remove struct class_interface * from callbacks
  driver core: class: mark the struct class in struct class_interface constant
  driver core: class: make class_register() take a const *
  driver core: class: mark class_release() as taking a const *
  driver core: remove incorrect comment for device_create*
  MIPS: vpe-cmp: remove module owner pointer from struct class usage.
  ...
2023-04-27 11:53:57 -07:00
Bjorn Andersson
ba7a4754da rpmsg: glink: Consolidate TX_DATA and TX_DATA_CONT
Rather than duplicating most of the code for constructing the initial
TX_DATA and subsequent TX_DATA_CONT packets, roll them into a single
loop.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Reviewed-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230418163018.785524-3-quic_bjorande@quicinc.com
2023-04-19 12:43:19 -07:00
Bjorn Andersson
7a68f9fa97 rpmsg: glink: Propagate TX failures in intentless mode as well
As support for splitting transmission over several messages using
TX_DATA_CONT was introduced it does not immediately return the return
value of qcom_glink_tx().

The result is that in the intentless case (i.e. intent == NULL), the
code will continue to send all additional chunks. This is wasteful, and
it's possible that the send operation could incorrectly indicate
success, if the last chunk fits in the TX fifo.

Fix the condition.

Fixes: 8956927fae ("rpmsg: glink: Add TX_DATA_CONT command while sending")
Reviewed-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230418163018.785524-2-quic_bjorande@quicinc.com
2023-04-19 12:43:19 -07:00
Bjorn Andersson
c05dfce0b8 rpmsg: glink: Wait for intent, not just request ack
In some implementations of the remote firmware, an intent request
acknowledgment is sent when it's determined if the intent allocation
will be fulfilled, but then the intent is queued after the
acknowledgment.

The result is that upon receiving a granted allocation request, the
search for the newly allocated intent will fail and an additional
request will be made. This will at best waste memory, but if the second
request is rejected the transaction will be incorrectly rejected.

Take the incoming intent into account in the wait to mitigate this
problem.

The above scenario can still happen, in the case that, on that same
channel, an unrelated intent is delivered prior to the request
acknowledgment and a separate process enters the send path and picks up
the intent. Given that there's no relationship between the
acknowledgment and the delivered (or to be delivered intent), there
doesn't seem to be a solution to this problem.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Reviewed-by: Chris Lew <quic_clew@quicinc.com>
[bjorn: Fixed spelling issues pointed out by checkpatch in commit message]
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230327144617.3134175-3-quic_bjorande@quicinc.com
2023-04-19 12:43:19 -07:00
Bjorn Andersson
0a7eee89e7 rpmsg: glink: Transition intent request signaling to wait queue
Transition the intent request acknowledgement to use a wait queue so
that it's possible, in the next commit, to extend the wait to also wait
for an incoming intent.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Reviewed-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230327144617.3134175-2-quic_bjorande@quicinc.com
2023-04-19 12:43:17 -07:00
Uwe Kleine-König
38be89514b rpmsg: qcom_smd: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.

qcom_smd_remove() always returned zero, though that isn't completely
trivial to see. So explain that in a comment and convert to
.remove_new().

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230321154039.355098-4-u.kleine-koenig@pengutronix.de
2023-04-05 20:58:32 -07:00
Uwe Kleine-König
49446e573b rpmsg: qcom_glink_rpm: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.

Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230321154039.355098-3-u.kleine-koenig@pengutronix.de
2023-04-05 20:58:32 -07:00
Uwe Kleine-König
810c03d9d7 rpmsg: qcom_smd: Make qcom_smd_unregister_edge() return void
This function returned zero unconditionally. Convert it to return no
value instead. This makes it more obvious what happens in the callers.

One caller is converted to return zero explicitly. The only other caller
(smd_subdev_stop() in drivers/remoteproc/qcom_common.c) already ignored
the return value before.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230321154039.355098-2-u.kleine-koenig@pengutronix.de
2023-04-05 20:58:32 -07:00
Greg Kroah-Hartman
1aaba11da9 driver core: class: remove module * from class_create()
The module pointer in class_create() never actually did anything, and it
shouldn't have been requred to be set as a parameter even if it did
something.  So just remove it and fix up all callers of the function in
the kernel tree at the same time.

Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Link: https://lore.kernel.org/r/20230313181843.1207845-4-gregkh@linuxfoundation.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-03-17 15:16:33 +01:00
Linus Torvalds
cc38a46de7 rpmsg updates for v6.3
rpmsg ctrl and char driver locking is ensure ordering in cases where the
 communication link is being torn down in parallel with calls to open(2)
 or poll(2).
 
 The glink driver is refactored, to move rpm/smem-specifics out of the
 common logic and better suite further improvements, such as transports
 without a mailbox controller. The handling of remoteproc shutdown is
 improved, to fail clients immediately instead of having them to wait for
 timeouts. A driver_override memory leak is corrected and a few spelling
 improvements are introduced.
 
 glink_ssr is transitioned off strlcpy() and "gpr" is added as a valid
 child node of the glink-edge DT binding.
 -----BEGIN PGP SIGNATURE-----
 
 iQJJBAABCAAzFiEEBd4DzF816k8JZtUlCx85Pw2ZrcUFAmP7jLkVHGFuZGVyc3Nv
 bkBrZXJuZWwub3JnAAoJEAsfOT8Nma3FKy4P/RBK8WFS9FJHgP3KZrN8XawruWco
 W/23uiW5tKmtzXO4RB6ZMnHR6iURgqzN6cQnguH9neq4ze1rBbLUkSeKtYK8FwJg
 XApzFMDCxtnjZwCAMdtT14C6R0mDt21DyeGCwRRaQg4eqiaPpuQBNX9ijgh3bons
 GKK0UJuZ1o/E+/pf18+vls1PRKXDqmFr8OUpIPEdM0EA8Mr+bFGbA9TbBkHWr7uq
 5xo7GSZU8u+mA53HGNctIVWAFEM0v9xHLgbjSXcHSiuS0xAC99CHw6TlA26SY6WS
 gh/ovF31m7tC7lz4nFSZfeLHbKb02dRmjIDxsCMFp5pLq/jbMgbkoJ97qtQlSQhW
 VjQemUzGvaENgwZr5ZcHEJ75SoGNtudHnfnwZeybRGjfiMLocVDcTZ81qIvxr8y7
 jZkrQw2uUpGu8qrV9URuIHb5xfo5ixdZ8llgKWq6/PT8pcYWcwBybby55jJ/vhG0
 7Fbv37VeEyq9xxpNfwuMELA2RztT5zrbPZVSfN348PIgMW3nbGzryw//r6HxmUUr
 +FoSvuSBwrnEHlsdgg3ydzIhzgY1+k3jQcS7xW+nZ3PG7eHBhIVBPyNZ9xydtfaL
 TBtjsCRigoLMxp8Fpdz0xANF5PICXcGfqX4591yZ9Q7fAYYQr/WKtiWS+cNd5t6A
 nXDMGwVYjEA46n/1
 =0w8J
 -----END PGP SIGNATURE-----

Merge tag 'rpmsg-v6.3' of git://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux

Pull rpmsg updates from Bjorn Andersson:

 - rpmsg ctrl and char driver locking is ensure ordering in cases where
   the communication link is being torn down in parallel with calls to
   open(2) or poll(2)

 - The glink driver is refactored, to move rpm/smem-specifics out of the
   common logic and better suite further improvements, such as
   transports without a mailbox controller. The handling of remoteproc
   shutdown is improved, to fail clients immediately instead of having
   them to wait for timeouts. A driver_override memory leak is corrected
   and a few spelling improvements are introduced

 - glink_ssr is transitioned off strlcpy() and "gpr" is added as a valid
   child node of the glink-edge DT binding

* tag 'rpmsg-v6.3' of git://git.kernel.org/pub/scm/linux/kernel/git/remoteproc/linux:
  rpmsg: glink: Release driver_override
  rpmsg: glink: Avoid infinite loop on intent for missing channel
  rpmsg: glink: Fix GLINK command prefix
  rpmsg: glink: Fix spelling of peek
  rpmsg: glink: Cancel pending intent requests at removal
  rpmsg: glink: Fail qcom_glink_tx() once remove has been initiated
  rpmsg: glink: Move irq and mbox handling to transports
  rpmsg: glink: rpm: Wrap driver context
  rpmsg: glink: smem: Wrap driver context
  rpmsg: glink: Extract tx kick operation
  rpmsg: glink: Include types in qcom_glink_native.h
  rpmsg: ctrl: Add lock to rpmsg_ctrldev_remove
  rpmsg: char: Add lock to avoid race when rpmsg device is released
  rpmsg: move from strlcpy with unused retval to strscpy
  dt-bindings: remoteproc: qcom,glink-edge: add GPR node
2023-02-26 12:10:28 -08:00
Bjorn Andersson
fb80ef67e8 rpmsg: glink: Release driver_override
Upon termination of the rpmsg_device, driver_override needs to be freed
to avoid leaking the potentially assigned string.

Fixes: 42cd402b8f ("rpmsg: Fix kfree() of static memory on setting driver_override")
Fixes: 39e47767ec ("rpmsg: Add driver_override device attribute for rpmsg_device")
Reviewed-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230109223931.1706429-1-quic_bjorande@quicinc.com
2023-02-15 07:46:02 -08:00
Bjorn Andersson
3e74ec2f39 rpmsg: glink: Avoid infinite loop on intent for missing channel
In the event that an intent advertisement arrives on an unknown channel
the fifo is not advanced, resulting in the same message being handled
over and over.

Fixes: dacbb35e93 ("rpmsg: glink: Receive and store the remote intent buffers")
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Reviewed-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230214234231.2069751-1-quic_bjorande@quicinc.com
2023-02-14 20:28:55 -08:00
Bjorn Andersson
4e816d0318 rpmsg: glink: Fix GLINK command prefix
The upstream GLINK driver was first introduced to communicate with the
RPM on MSM8996, presumably as an artifact from that era the command
defines was prefixed RPM_CMD, while they actually are GLINK_CMDs.

Let's rename these, to keep things tidy. No functional change.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Reviewed-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230214225933.2025595-1-quic_bjorande@quicinc.com
2023-02-14 20:28:51 -08:00
Bjorn Andersson
a8f500c686 rpmsg: glink: Fix spelling of peek
The code is peeking into the buffers, not peaking. Fix this throughout
the glink drivers.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Reviewed-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230214224746.1996130-1-quic_bjorande@quicinc.com
2023-02-14 20:28:48 -08:00
Bjorn Andersson
fb23b97346 rpmsg: glink: Cancel pending intent requests at removal
During removal of the glink edge interrupts are disabled and no more
incoming messages are being serviced. In addition to the remote endpoint
being defunct that means that any outstanding requests for intents will
not be serviced, and qcom_glink_request_intent() will blindly wait for
up to 10 seconds.

Mark the intent request as not granted and complete the intent request
completion to fail the waiting client immediately.

Once the current intent request is failed, any potential clients waiting
for the intent request mutex will not enter the same wait, as the
qcom_glink_tx() call will fail fast.

Reviewed-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230213155215.1237059-7-quic_bjorande@quicinc.com
2023-02-14 08:19:38 -08:00
Bjorn Andersson
9c96bacf1a rpmsg: glink: Fail qcom_glink_tx() once remove has been initiated
Upon removing the glink edge, communication is at best one-way. This
means that the very common scenario of glink requesting intents will not
be possible to serve.

Typically a successful transmission results in the client waiting for a
response, with some timeout and a mechanism for aborting that timeout.

Because of this, once the glink edge is defunct once removal is
commenced it's better to fail transmissions fast.

Reviewed-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230213155215.1237059-6-quic_bjorande@quicinc.com
2023-02-14 08:19:38 -08:00
Bjorn Andersson
f424d1cbe8 rpmsg: glink: Move irq and mbox handling to transports
Not all GLINK transports uses an interrupt and a mailbox instance. The
interrupt for RPM needs to be IRQF_NOSUSPEND, while it seems reasonable
for the SMEM interrupt to use irq_set_wake. The glink struct device is
constructed in the SMEM and RPM drivers but torn down in the core
driver.

Move the interrupt and kick handling into the SMEM and RPM driver, to
improve this and facilitate further improvements.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Reviewed-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230213155215.1237059-5-quic_bjorande@quicinc.com
2023-02-14 08:19:38 -08:00
Bjorn Andersson
178c3af447 rpmsg: glink: rpm: Wrap driver context
As with the SMEM driver update, wrap the RPM context in a struct to
facilitate the upcoming changes of moving IRQ and mailbox registration
to the driver.

Reviewed-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230213155215.1237059-4-quic_bjorande@quicinc.com
2023-02-14 08:19:38 -08:00
Bjorn Andersson
ab9fdd41d9 rpmsg: glink: smem: Wrap driver context
The Glink SMEM driver allocates a struct device and hangs two
devres-allocated pipe objects thereon. To facilitate the move of
interrupt and mailbox handling to the driver, introduce a wrapper object
capturing the device, glink reference and remote processor id.

The type of the remoteproc reference is updated, as these are
specifically targeting the SMEM implementation.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Reviewed-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230213155215.1237059-3-quic_bjorande@quicinc.com
2023-02-14 08:19:38 -08:00
Bjorn Andersson
8278fd3144 rpmsg: glink: Extract tx kick operation
Refactor out the tx kick operations to its own function, in preparation
for pushing the details to the individual transports.

Reviewed-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230213155215.1237059-2-quic_bjorande@quicinc.com
2023-02-14 08:19:38 -08:00
Greg Kroah-Hartman
2a81ada32f driver core: make struct bus_type.uevent() take a const *
The uevent() callback in struct bus_type should not be modifying the
device that is passed into it, so mark it as a const * and propagate the
function signature changes out into all relevant subsystems that use
this callback.

Acked-by: Rafael J. Wysocki <rafael@kernel.org>
Acked-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20230111113018.459199-16-gregkh@linuxfoundation.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
2023-01-27 13:45:52 +01:00
Bjorn Andersson
f014eda5d5 rpmsg: glink: Include types in qcom_glink_native.h
Ensure that the used data types are available in qcom_glink_native.h, to
silence LSP warnings.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
Reviewed-by: Chris Lew <quic_clew@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20230109223745.1706152-1-quic_bjorande@quicinc.com
2023-01-18 11:48:37 -06:00
Deepak Kumar Singh
c23965b7f7 rpmsg: ctrl: Add lock to rpmsg_ctrldev_remove
Call to rpmsg_ctrldev_ioctl() and rpmsg_ctrldev_remove() must be synchronized.
In present code rpmsg_ctrldev_remove() is not protected with lock, therefore
new char device creation can succeed through rpmsg_ctrldev_ioctl() call. At the
same time call to rpmsg_ctrldev_remove() function for ctrl device removal will
free associated rpdev device. As char device creation already succeeded, user
space is free to issue open() call which maps to rpmsg_create_ept() in kernel.
rpmsg_create_ept() function tries to reference rpdev which has already been
freed through rpmsg_ctrldev_remove(). Issue is predominantly seen in aggressive
reboot tests where rpmsg_ctrldev_ioctl() and rpmsg_ctrldev_remove() can race with
each other.

Adding lock in rpmsg_ctrldev_remove() avoids any new char device creation
through rpmsg_ctrldev_ioctl() while remove call is already in progress.

Signed-off-by: Deepak Kumar Singh <quic_deesin@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/1663584840-15762-3-git-send-email-quic_deesin@quicinc.com
2022-12-28 09:54:03 -06:00
Deepak Kumar Singh
17b88a2050 rpmsg: char: Add lock to avoid race when rpmsg device is released
When remote host goes down glink char device channel is freed and
associated rpdev is destroyed through rpmsg_chrdev_eptdev_destroy(),
At the same time user space apps can still try to open/poll rpmsg
char device which will result in calling rpmsg_create_ept()/rpmsg_poll().
These functions will try to reference rpdev which has already been freed
through rpmsg_chrdev_eptdev_destroy().

File operation functions and device removal function must be protected
with lock. This patch adds existing ept lock in remove function as well.

Signed-off-by: Deepak Kumar Singh <quic_deesin@quicinc.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/1663584840-15762-2-git-send-email-quic_deesin@quicinc.com
2022-12-28 09:54:03 -06:00
Wolfram Sang
d2ff0f84c1 rpmsg: move from strlcpy with unused retval to strscpy
Follow the advice of the below link and prefer 'strscpy' in this
subsystem. Conversion is 1:1 because the return value is not used.
Generated by a coccinelle script.

Link: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
Link: https://lore.kernel.org/r/20220818210100.7277-1-wsa+renesas@sang-engineering.com
2022-12-28 09:47:41 -06:00
Shengjiu Wang
467233a4ac rpmsg: char: Avoid double destroy of default endpoint
The rpmsg_dev_remove() in rpmsg_core is the place for releasing
this default endpoint.

So need to avoid destroying the default endpoint in
rpmsg_chrdev_eptdev_destroy(), this should be the same as
rpmsg_eptdev_release(). Otherwise there will be double destroy
issue that ept->refcount report warning:

refcount_t: underflow; use-after-free.

Call trace:
 refcount_warn_saturate+0xf8/0x150
 virtio_rpmsg_destroy_ept+0xd4/0xec
 rpmsg_dev_remove+0x60/0x70

The issue can be reproduced by stopping remoteproc before
closing the /dev/rpmsgX.

Fixes: bea9b79c2d ("rpmsg: char: Add possibility to use default endpoint of the rpmsg device")
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Reviewed-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Cc: stable <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/1663725523-6514-1-git-send-email-shengjiu.wang@nxp.com
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
2022-09-21 11:21:33 -06:00
ye xingchen
06564be4c0 rpmsg: char: Remove the unneeded result variable
Return the value rpmsg_chrdev_eptdev_add() directly instead of storing it
in another redundant variable.

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: ye xingchen <ye.xingchen@zte.com.cn>
Link: https://lore.kernel.org/r/20220826071954.252485-1-ye.xingchen@zte.com.cn
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
2022-09-02 13:25:47 -06:00
Xuezhi Zhang
7113ac8253 rpmsg: convert sysfs snprintf to sysfs_emit
Fix the following coccicheck warning:
drivers/rpmsg/qcom_glink_native.c:1677:8-16:
WARNING: use scnprintf or sprintf

Signed-off-by: Xuezhi Zhang <zhangxuezhi1@coolpad.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Link: https://lore.kernel.org/r/20220607120649.78436-1-zhangxuezhi1@coolpad.com
2022-07-16 23:08:47 -05:00
Miaoqian Lin
65382585f0 rpmsg: qcom_smd: Fix refcount leak in qcom_smd_parse_edge
of_parse_phandle() returns a node pointer with refcount
incremented, we should use of_node_put() on it when done.

Fixes: 53e2822e56 ("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
2022-07-16 22:15:40 -05:00
Krzysztof Kozlowski
101042f4c0 rpmsg: qcom: correct kerneldoc
Correct kerneldoc warnings like:

  drivers/rpmsg/qcom_glink_ssr.c:45:
    warning: expecting prototype for G(). Prototype was for GLINK_SSR_DO_CLEANUP() instead

Also fix meaning of 'flag' argument.

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Link: https://lore.kernel.org/r/20220519073330.7187-3-krzysztof.kozlowski@linaro.org
2022-07-16 22:13:32 -05:00
Krzysztof Kozlowski
6c3ebc96ff rpmsg: qcom: glink: remove unused name
The qcom_glink.name is read from DTS but never used further, never
referenced, so drop it.  This also fixes kerneldoc warning:

  drivers/rpmsg/qcom_glink_native.c:125:
    warning: Function parameter or member 'name' not described in 'qcom_glink'

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Link: https://lore.kernel.org/r/20220519073330.7187-2-krzysztof.kozlowski@linaro.org
2022-07-16 22:13:32 -05:00
Krzysztof Kozlowski
766279a8f8 rpmsg: qcom: glink: replace strncpy() with strscpy_pad()
The use of strncpy() is considered deprecated for NUL-terminated
strings[1]. Replace strncpy() with strscpy_pad(), to keep existing
pad-behavior of strncpy, similarly to commit 08de420a80 ("rpmsg:
glink: Replace strncpy() with strscpy_pad()").  This fixes W=1 warning:

  In function ‘qcom_glink_rx_close’,
    inlined from ‘qcom_glink_work’ at ../drivers/rpmsg/qcom_glink_native.c:1638:4:
  drivers/rpmsg/qcom_glink_native.c:1549:17: warning: ‘strncpy’ specified bound 32 equals destination size [-Wstringop-truncation]
   1549 |                 strncpy(chinfo.name, channel->name, sizeof(chinfo.name));

[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Link: https://lore.kernel.org/r/20220519073330.7187-1-krzysztof.kozlowski@linaro.org
2022-07-16 22:13:32 -05:00
Saud Farooqui
99de6509c4 rpmsg: Strcpy is not safe, use strscpy_pad() instead
Replace strcpy() with strscpy_pad() for copying the rpmsg
device name in rpmsg_register_device_override().

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Saud Farooqui <farooqui_saud@hotmail.com>
Link: https://lore.kernel.org/r/PA4P189MB14210AA95DCA3715AFA7F4A68BB59@PA4P189MB1421.EURP189.PROD.OUTLOOK.COM
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
2022-06-24 11:37:00 -06:00
Hangyu Hua
d7bd416d35 rpmsg: Fix possible refcount leak in rpmsg_register_device_override()
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: bb17d110cb ("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>
2022-06-24 11:34:00 -06:00
Arnaud Pouliquen
416b992b05 rpmsg: Fix parameter naming for announce_create/destroy ops
The parameter associated to the announce_create and
announce_destroy ops functions is not an endpoint but a rpmsg device.

Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
Link: https://lore.kernel.org/r/20220425071723.774050-1-arnaud.pouliquen@foss.st.com
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
2022-06-24 10:58:12 -06:00
AngeloGioacchino Del Regno
353d921468 rpmsg: mtk_rpmsg: Fix circular locking dependency
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: 7017996951 ("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>
2022-06-14 16:41:10 -06:00
Shengjiu Wang
abe13e9a56 rpmsg: char: Add mutex protection for rpmsg_eptdev_open()
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: 964e8bedd5 ("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>
2022-06-14 14:58:10 -06:00