diff mbox series

[v2,2/3] scsi: megaraid_sas: check user-provided offsets

Message ID 20200918121522.1466028-1-arnd@arndb.de
State Accepted
Commit 381d34e376e3d9d27730fda8a0e870600e6c8196
Headers show
Series None | expand

Commit Message

Arnd Bergmann Sept. 18, 2020, 12:15 p.m. UTC
It sounds unwise to let user space pass an unchecked 32-bit
offset into a kernel structure in an ioctl. This is an unsigned
variable, so checking the upper bound for the size of the structure
it points into is sufficient to avoid data corruption, but as
the pointer might also be unaligned, it has to be written carefully
as well.

While I stumbled over this problem by reading the code, I did not
continue checking the function for further problems like it.

Cc: <stable@vger.kernel.org> # v2.6.15+
Fixes: c4a3e0a529ab ("[SCSI] MegaRAID SAS RAID: new driver")
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

Comments

Sasha Levin Sept. 21, 2020, 12:54 p.m. UTC | #1
Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: c4a3e0a529ab ("[SCSI] MegaRAID SAS RAID: new driver").

The bot has tested the following trees: v5.8.10, v5.4.66, v4.19.146, v4.14.198, v4.9.236, v4.4.236.

v5.8.10: Build OK!
v5.4.66: Build OK!
v4.19.146: Build OK!
v4.14.198: Failed to apply! Possible dependencies:
    107a60dd71b5 ("scsi: megaraid_sas: Add support for 64bit consistent DMA")
    1b4bed206159 ("scsi: megaraid_sas: Create separate functions for allocating and freeing controller DMA buffers")
    201a810cc188 ("scsi: megaraid_sas: Re-Define enum DCMD_RETURN_STATUS")
    2ce435087902 ("scsi: megaraid_sas: Enhance internal DCMD timeout prints")
    7535f27d1f14 ("scsi: megaraid_sas: Move initialization of instance parameters inside newly created function megasas_init_ctrl_params")
    82add4e1b354 ("scsi: megaraid_sas: Incorrect processing of IOCTL frames for SMP/STP commands")
    e5d65b4b81af ("scsi: megaraid_sas: Move controller memory allocations and DMA mask settings from probe to megasas_init_fw")
    e97e673ca63b ("scsi: megaraid_sas: Retry with reduced queue depth when alloc fails for higher QD")

v4.9.236: Failed to apply! Possible dependencies:
    201a810cc188 ("scsi: megaraid_sas: Re-Define enum DCMD_RETURN_STATUS")
    2493c67e518c ("scsi: megaraid_sas: 128 MSIX Support")
    3e5eadb1a881 ("scsi: megaraid_sas: Enable or Disable Fast path based on the PCI Threshold Bandwidth")
    45b8a35eed7b ("scsi: megaraid_sas: 32 bit descriptor fire cmd optimization")
    45f4f2eb3da3 ("scsi: megaraid_sas: Add new pci device Ids for SAS3.5 Generic Megaraid Controllers")
    82add4e1b354 ("scsi: megaraid_sas: Incorrect processing of IOCTL frames for SMP/STP commands")
    8823abeddbbc ("scsi: megaraid_sas: Fix endianness issues in DCMD handling")
    95c060869e68 ("scsi: megaraid_sas: latest controller OCR capability from FW before sending shutdown DCMD")
    d0fc91d67c59 ("scsi: megaraid_sas: Send SYNCHRONIZE_CACHE for VD to firmware")
    f4fc209326c7 ("scsi: megaraid_sas: change issue_dcmd to return void from int")
    fad119b707f8 ("scsi: megaraid_sas: switch to pci_alloc_irq_vectors")

v4.4.236: Failed to apply! Possible dependencies:
    201a810cc188 ("scsi: megaraid_sas: Re-Define enum DCMD_RETURN_STATUS")
    6d40afbc7d13 ("megaraid_sas: MFI IO timeout handling")
    82add4e1b354 ("scsi: megaraid_sas: Incorrect processing of IOCTL frames for SMP/STP commands")
    8823abeddbbc ("scsi: megaraid_sas: Fix endianness issues in DCMD handling")
    8a01a41d8647 ("megaraid_sas: Make adprecovery variable atomic")
    95c060869e68 ("scsi: megaraid_sas: latest controller OCR capability from FW before sending shutdown DCMD")
    f4fc209326c7 ("scsi: megaraid_sas: change issue_dcmd to return void from int")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?
diff mbox series

Patch

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index 861f7140f52e..c3de69f3bee8 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -8095,7 +8095,7 @@  megasas_mgmt_fw_ioctl(struct megasas_instance *instance,
 	int error = 0, i;
 	void *sense = NULL;
 	dma_addr_t sense_handle;
-	unsigned long *sense_ptr;
+	void *sense_ptr;
 	u32 opcode = 0;
 	int ret = DCMD_SUCCESS;
 
@@ -8218,6 +8218,12 @@  megasas_mgmt_fw_ioctl(struct megasas_instance *instance,
 	}
 
 	if (ioc->sense_len) {
+		/* make sure the pointer is part of the frame */
+		if (ioc->sense_off > (sizeof(union megasas_frame) - sizeof(__le64))) {
+			error = -EINVAL;
+			goto out;
+		}
+
 		sense = dma_alloc_coherent(&instance->pdev->dev, ioc->sense_len,
 					     &sense_handle, GFP_KERNEL);
 		if (!sense) {
@@ -8225,12 +8231,11 @@  megasas_mgmt_fw_ioctl(struct megasas_instance *instance,
 			goto out;
 		}
 
-		sense_ptr =
-		(unsigned long *) ((unsigned long)cmd->frame + ioc->sense_off);
+		sense_ptr = (void *)cmd->frame + ioc->sense_off;
 		if (instance->consistent_mask_64bit)
-			*sense_ptr = cpu_to_le64(sense_handle);
+			put_unaligned_le64(sense_handle, sense_ptr);
 		else
-			*sense_ptr = cpu_to_le32(sense_handle);
+			put_unaligned_le32(sense_handle, sense_ptr);
 	}
 
 	/*