Unverified Commit a3168773 authored by Arnd Bergmann's avatar Arnd Bergmann
Browse files

Merge tag 'scmi-fixes-6.2' of... 

Merge tag 'scmi-fixes-6.2' of git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux into arm/fixes

Arm SCMI fixes for v6.2

Few fixes addressing:
1. Possible compromise with the shorter message size from a misbheaving
   SCMI platform firmware. The shmem accesses are now hardened to handle
   the same in fetch_notification and fetch_response.

2. Possible unsafe locking scenario which is solved by calling
   virtio_break_device() before getting hold of vioch->lock.

3. Possible stale error status reported from a previous message being
   used again as it is not cleared.

* tag 'scmi-fixes-6.2' of git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux:
  firmware: arm_scmi: Fix virtio channels cleanup on shutdown
  firmware: arm_scmi: Harden shared memory access in fetch_notification
  firmware: arm_scmi: Harden shared memory access in fetch_response
  firmware: arm_scmi: Clear stale xfer->hdr.status

Link: https://lore.kernel.org/r/20230106093909.652657-1-sudeep.holla@arm.com


Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
parents dc43354c e325285d

drivers/firmware/arm_scmi/driver.c

+2 −0
Original line number Diff line number Diff line
@@ -910,6 +910,8 @@ static int do_xfer(const struct scmi_protocol_handle *ph, 
			      xfer->hdr.protocol_id, xfer->hdr.seq,

			      xfer->hdr.poll_completion);



	/* Clear any stale status */

	xfer->hdr.status = SCMI_SUCCESS;

	xfer->state = SCMI_XFER_SENT_OK;

	/*

	 * Even though spinlocking is not needed here since no race is possible

drivers/firmware/arm_scmi/shmem.c

+6 −3
Original line number Diff line number Diff line
@@ -81,10 +81,11 @@ u32 shmem_read_header(struct scmi_shared_mem __iomem *shmem) 
void shmem_fetch_response(struct scmi_shared_mem __iomem *shmem,

			  struct scmi_xfer *xfer)

{

	size_t len = ioread32(&shmem->length);



	xfer->hdr.status = ioread32(shmem->msg_payload);

	/* Skip the length of header and status in shmem area i.e 8 bytes */

	xfer->rx.len = min_t(size_t, xfer->rx.len,

			     ioread32(&shmem->length) - 8);

	xfer->rx.len = min_t(size_t, xfer->rx.len, len > 8 ? len - 8 : 0);



	/* Take a copy to the rx buffer.. */

	memcpy_fromio(xfer->rx.buf, shmem->msg_payload + 4, xfer->rx.len);
@@ -93,8 +94,10 @@ void shmem_fetch_response(struct scmi_shared_mem __iomem *shmem, 
void shmem_fetch_notification(struct scmi_shared_mem __iomem *shmem,

			      size_t max_len, struct scmi_xfer *xfer)

{

	size_t len = ioread32(&shmem->length);



	/* Skip only the length of header in shmem area i.e 4 bytes */

	xfer->rx.len = min_t(size_t, max_len, ioread32(&shmem->length) - 4);

	xfer->rx.len = min_t(size_t, max_len, len > 4 ? len - 4 : 0);



	/* Take a copy to the rx buffer.. */

	memcpy_fromio(xfer->rx.buf, shmem->msg_payload, xfer->rx.len);

drivers/firmware/arm_scmi/virtio.c

+6 −1
Original line number Diff line number Diff line
@@ -160,7 +160,6 @@ static void scmi_vio_channel_cleanup_sync(struct scmi_vio_channel *vioch) 
	}



	vioch->shutdown_done = &vioch_shutdown_done;

	virtio_break_device(vioch->vqueue->vdev);

	if (!vioch->is_rx && vioch->deferred_tx_wq)

		/* Cannot be kicked anymore after this...*/

		vioch->deferred_tx_wq = NULL;
@@ -482,6 +481,12 @@ static int virtio_chan_free(int id, void *p, void *data) 
	struct scmi_chan_info *cinfo = p;

	struct scmi_vio_channel *vioch = cinfo->transport_info;



	/*

	 * Break device to inhibit further traffic flowing while shutting down

	 * the channels: doing it later holding vioch->lock creates unsafe

	 * locking dependency chains as reported by LOCKDEP.

	 */

	virtio_break_device(vioch->vqueue->vdev);

	scmi_vio_channel_cleanup_sync(vioch);



	scmi_free_channel(cinfo, data, id);