Page MenuHomeFreeBSD

[iscsi] Fix initiator kernel panic if target ctl(4) port is disabled
ClosedPublic

Authored by afedorov on Apr 27 2020, 11:13 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 18, 9:34 PM
Unknown Object (File)
Thu, Apr 18, 3:43 PM
Unknown Object (File)
Thu, Apr 18, 1:18 PM
Unknown Object (File)
Mar 19 2024, 9:58 PM
Unknown Object (File)
Mar 19 2024, 2:18 PM
Unknown Object (File)
Dec 23 2023, 12:56 AM
Unknown Object (File)
Jul 10 2023, 9:24 AM
Unknown Object (File)
Jul 10 2023, 8:46 AM
Subscribers

Details

Summary

If the target ctl(4) port is disabled using ctladm -p X -o off, the initiator creates incomplete sessions that lead to panic when calling 'ctladm islist'.

See detailed description: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=244792
And debug output: https://bz-attachments.freebsd.org/attachment.cgi?id=212383

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

I think this looks right, but I think there are other places that also need the lock perhaps? I think there's at least one other place that does these two statements without holding the session lock when I looked.

+1
There are more places where the cv_signal(cs->xxx) is not called with the cs lock held. We could take the chance and fix them all.

mav requested changes to this revision.EditedApr 30 2020, 8:17 PM

I think it is correct, except the same should be added also 48 lines lower.

But also looking on cfiscsi_ioctl_list() I am curios why (cs->cs_target == NULL) there is under #ifdef ICL_KERNEL_PROXY? Without one defined there is still a chance to crash before maintenance thread call cfiscsi_session_delete(). I think the ifdef should be removed.

This revision now requires changes to proceed.Apr 30 2020, 8:17 PM

I will try to fix all the places.

This revision was not accepted when it landed; it landed in state Needs Revision.May 3 2020, 4:15 PM
This revision was automatically updated to reflect the committed changes.