On Fri, Oct 30, 2020 at 05:44:20PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
> 
> 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: <[email protected]> # v2.6.15+
> Fixes: c4a3e0a529ab ("[SCSI] MegaRAID SAS RAID: new driver")
> Reviewed-by: Christoph Hellwig <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
>  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 41cd66fc7d81..b1b9a8823c8c 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -8134,7 +8134,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;
>  
> @@ -8257,6 +8257,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;

This still has the overly long line.

Reply via email to