lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 3 Jan 2021 19:49:39 +0100
From:   Arnd Bergmann <>
To:     "James E.J. Bottomley" <>
Cc:     Phil Oester <>, Arnd Bergmann <>,
        Kashyap Desai <>,
        Sumit Saxena <>,
        Shivasharan S <>,
        "Martin K. Petersen" <>,
        Christoph Hellwig <>,
        "# 3.4.x" <>,
        Anand Lodnoor <>,
        Chandrakanth Patil <>,
        Hannes Reinecke <>,,
        linux-scsi <>,
        "" <>
Subject: Re: [PATCH 2/3] scsi: megaraid_sas: check user-provided offsets

On Sun, Jan 3, 2021 at 6:00 PM James Bottomley <> wrote:
> On Sun, 2021-01-03 at 17:26 +0100, Arnd Bergmann wrote:
> [...]
> > @@ -8209,7 +8208,7 @@ megasas_mgmt_fw_ioctl(struct megasas_instance
> > *instance,
> >                 if (instance->consistent_mask_64bit)
> >                         put_unaligned_le64(sense_handle, sense_ptr);
> >                 else
> > -                       put_unaligned_le32(sense_handle, sense_ptr);
> > +                       put_unaligned_le64(sense_handle, sense_ptr);
> >         }
> This hunk can't be right.  It effectively means removing the if.

I'm just trying to restore the state before the regression introduced
in my 381d34e376e3 ("scsi: megaraid_sas: Check user-provided offsets").

The old code always stored 'sizeof(long)' bytes into sense_ptr,
regardless of instance->consistent_mask_64bit, but it would truncate
the address to 32 bit if that was cleared. This was clearly bogus
and I tried to make it do something more meaningful, only storing
8 bytes into the structure if it was configured for 64-bit DMA, regardless
of the capabilities of the kernel.

> However, the if is needed because sense_handle is a dma_addr_t which
> can be either 32 or 64 bit.  What about changing the if to
> if (sizeof(dma_addr_t) == 8)
> instead?

That would not be useful either, the device surely does not care
if the kernel supports 64-bit DMA. What we'd really need here is
someone with access to the interface specifications to see how
many bytes should be stored in the structure. I suspect always
storing 64 bits (as my patch does) is correct, and would send a
proper patch to remove the if() if Phil confirms that my test
patch fixes the regression.


Powered by blists - more mailing lists