On Fri, Sep 18, 2020 at 02:15:22PM +0200, Arnd Bergmann wrote:
> 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: <sta...@vger.kernel.org> # v2.6.15+
> Fixes: c4a3e0a529ab ("[SCSI] MegaRAID SAS RAID: new driver")
> Reviewed-by: Christoph Hellwig <h...@lst.de>
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> ---
> drivers/scsi/megaraid/megaraid_sas_base.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> 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))) {
Add a line break to avoid the overly long line - also the braces
around the arithmetics aren't actually needed.
