On Wed, 16 Nov 2022 11:50:37 +0100, Benjamin Block wrote:
We used to use the wrong type of integer in `zfcp_fsf_req_send()` to cache the FSF request ID when sending a new FSF request. This is used in case the sending fails and we need to remove the request from our internal hash table again (so we don't keep an invalid reference and use it when we free the request again).
In `zfcp_fsf_req_send()` we used to cache the ID as `int` (signed and 32 bit wide), but the rest of the zfcp code (and the firmware specification) handles the ID as `unsigned long`/`u64` (unsigned and 64 bit wide [s390x ELF ABI]). For one this has the obvious problem that when the ID grows past 32 bit (this can happen reasonably fast) it is truncated to 32 bit when storing it in the cache variable and so doesn't match the original ID anymore. The second less obvious problem is that even when the original ID has not yet grown past 32 bit, as soon as the 32nd bit is set in the original ID (0x80000000 = 2'147'483'648) we will have a mismatch when we cast it back to `unsigned long`. As the cached variable is of a signed type, the compiler will choose a sign-extending instruction to load the 32 bit variable into a 64 bit register (e.g.: `lgf %r11,188(%r15)`). So once we pass the cached variable into `zfcp_reqlist_find_rm()` to remove the request again all the leading zeros will be flipped to ones to extend the sign and won't match the original ID anymore (this has been observed in practice).
[...]
Applied to 6.1/scsi-fixes, thanks!
[1/1] zfcp: fix double free of fsf request when qdio send fails https://git.kernel.org/mkp/scsi/c/0954256e970e