iscsi: Always free a cdw before its associated ctl_io.

[jhb]

Summary

cxgbei stores state about a target transfer in the ctl_private[] array
of a ctl_io that is freed when a target transfer (represented by the
cdw) is freed. As such, freeing a ctl_io before a cdw that references
it can result in a use after free in cxgbei. Two of the four places
freed the cdw first, and the other two freed the ctl_io first. Fix
the latter two places to free the cdw first.

Reported by: Jithesh Arakkan @ Chelsio

[jhb]

The panics showed up after Chelsio QA started testing the fix in e894e3adb206815c2acff17a4011becb166c2f66. Specifically, in some cases due to the bug this change fixes, the write to clear the pointer to NULL in ctl_io in the cxgbei driver in that commit would write to ctl_io after it had been freed by ctl_datamove_done().

sys/cam/ctl/ctl_frontend_iscsi.c

2959

I would appreciate some thoughts on my questions here.

[mav]

Looks good to me. Just remove the XXX comment.

sys/cam/ctl/ctl_frontend_iscsi.c

2959

No, cdw_io != io here. io is the abort request, while cdw_io is the original SCSI command being aborted. You've properly fixed CTL_FLAG_DMA_INPROG clear on wrong io.

[imp]

sys/cam/ctl/ctl_frontend_iscsi.c

2967

above we set it to 42, here 43? I'm not super familiar with the code, but is that a problem?

[mav]

sys/cam/ctl/ctl_frontend_iscsi.c

2967

These codes were never formalized. I've done some cleanup, but haven't completed. So anything non-zero is OK, unique values are used just to ease debugging. And since the command was aborted, this code goes nowhere really, but still it is better to indicate that the transfer was not successful.

[jhb]

sys/cam/ctl/ctl_frontend_iscsi.c

2959

Thanks, I've axed the comment.

[jhb]

• Remove comment with questions.