lists.openwall.net | 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
| ||
|
Date: Sun, 3 Jan 2021 19:49:39 +0100 From: Arnd Bergmann <arnd@...nel.org> To: "James E.J. Bottomley" <jejb@...ux.ibm.com> Cc: Phil Oester <kernel@...uxace.com>, Arnd Bergmann <arnd@...db.de>, Kashyap Desai <kashyap.desai@...adcom.com>, Sumit Saxena <sumit.saxena@...adcom.com>, Shivasharan S <shivasharan.srikanteshwara@...adcom.com>, "Martin K. Petersen" <martin.petersen@...cle.com>, Christoph Hellwig <hch@...radead.org>, "# 3.4.x" <stable@...r.kernel.org>, Anand Lodnoor <anand.lodnoor@...adcom.com>, Chandrakanth Patil <chandrakanth.patil@...adcom.com>, Hannes Reinecke <hare@...e.de>, megaraidlinux.pdl@...adcom.com, linux-scsi <linux-scsi@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: Re: [PATCH 2/3] scsi: megaraid_sas: check user-provided offsets On Sun, Jan 3, 2021 at 6:00 PM James Bottomley <jejb@...ux.ibm.com> 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. Arnd
Powered by blists - more mailing lists