[v2] block/iscsi: fix ioctl cancel use-after-free

[Qemu-devel] [PATCH v2 0/3] block/iscsi: fix ioctl cancel use-after-free

Posted by Stefan Hajnoczi 6 years, 2 months ago

v2:
 * It was unnecessary to avoid duplicate iscsi_schedule_bh() calls since this
   function already protects against duplicate calls internally [Stefan]

Patches 1 & 2 are cleanups.

Patch 3 fixes cancellation of ioctls.  Felipe showed me a trace where an acb is
cancelled and then completes twice.  The second time around crashes QEMU.

Compile-tested only.

Felipe: Please let us know if this fixes the issue you are seeing.  Thanks!

Stefan Hajnoczi (3):
  block/iscsi: drop unused IscsiAIOCB->buf field
  block/iscsi: take iscsilun->mutex in iscsi_timed_check_events()
  block/iscsi: fix ioctl cancel use-after-free

 block/iscsi.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

-- 
2.14.3

Re: [Qemu-devel] [PATCH v2 0/3] block/iscsi: fix ioctl cancel use-after-free

Posted by Stefan Hajnoczi 6 years, 2 months ago

On Sat, Feb 03, 2018 at 07:16:18AM +0100, Stefan Hajnoczi wrote:
> v2:
>  * It was unnecessary to avoid duplicate iscsi_schedule_bh() calls since this
>    function already protects against duplicate calls internally [Stefan]
> 
> Patches 1 & 2 are cleanups.
> 
> Patch 3 fixes cancellation of ioctls.  Felipe showed me a trace where an acb is
> cancelled and then completes twice.  The second time around crashes QEMU.
> 
> Compile-tested only.
> 
> Felipe: Please let us know if this fixes the issue you are seeing.  Thanks!
> 
> Stefan Hajnoczi (3):
>   block/iscsi: drop unused IscsiAIOCB->buf field
>   block/iscsi: take iscsilun->mutex in iscsi_timed_check_events()
>   block/iscsi: fix ioctl cancel use-after-free
> 
>  block/iscsi.c | 33 ++++++++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 11 deletions(-)

Thanks for the reviews, Paolo and Felipe.

Paolo: Please merge this, I'll send an additional patch that works
around libiscsi's missing cancellation callback.

Re: [Qemu-devel] [PATCH v2 0/3] block/iscsi: fix ioctl cancel use-after-free

Posted by Paolo Bonzini 5 years, 4 months ago

On 15/02/18 11:37, Stefan Hajnoczi wrote:
> On Sat, Feb 03, 2018 at 07:16:18AM +0100, Stefan Hajnoczi wrote:
>> v2:
>>  * It was unnecessary to avoid duplicate iscsi_schedule_bh() calls since this
>>    function already protects against duplicate calls internally [Stefan]
>>
>> Patches 1 & 2 are cleanups.
>>
>> Patch 3 fixes cancellation of ioctls.  Felipe showed me a trace where an acb is
>> cancelled and then completes twice.  The second time around crashes QEMU.
>>
>> Compile-tested only.
>>
>> Felipe: Please let us know if this fixes the issue you are seeing.  Thanks!
>>
>> Stefan Hajnoczi (3):
>>   block/iscsi: drop unused IscsiAIOCB->buf field
>>   block/iscsi: take iscsilun->mutex in iscsi_timed_check_events()
>>   block/iscsi: fix ioctl cancel use-after-free
>>
>>  block/iscsi.c | 33 ++++++++++++++++++++++-----------
>>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> Thanks for the reviews, Paolo and Felipe.
> 
> Paolo: Please merge this, I'll send an additional patch that works
> around libiscsi's missing cancellation callback.
> 

Queued now for 4.0.  It's only been 9 months...

I also queued "block/iscsi: cancel libiscsi task when ABORT TASK TMF
completes".

Paolo