Repository navigation
Fix lio ua14e - #393
Closed
bmeagherix wants to merge 21 commits into
Closed
Fix lio ua14e#393bmeagherix wants to merge 21 commits into
bmeagherix wants to merge 21 commits into
Conversation
…ion holder When a LUN is unmapped from a node ACL, core_scsi3_free_pr_reg_from_nacl() releases the reservation if its holder belongs to that ACL, then frees every registration of the ACL. For an all registrants reservation, __core_scsi3_complete_pro_release() does not drop the reservation when called with unreg set. It makes the next registration on the list the holder instead. If that registration belongs to the same ACL, e.g. a second I_T nexus of the same initiator with another ISID, it is freed by the loop that follows. dev_pr_res_holder is left pointing at freed memory. The next use of the holder dereferences it. Here that was removing the backstore after a WRITE EXCLUSIVE - ALL REGISTRANTS reservation held by two I_T nexuses of one initiator: BUG: kernel NULL pointer dereference, address: 0000000000000148 RIP: 0010:__core_scsi3_complete_pro_release+0x1c/0x270 [target_core_mod] Call Trace: core_scsi3_free_all_registrations+0x44/0x170 [target_core_mod] target_free_device+0xb9/0x110 [target_core_mod] config_item_cleanup+0x52/0xa0 [configfs] configfs_rmdir+0x319/0x3a0 [configfs] Repeat the release and free for as long as the holder belongs to the ACL being removed. The reservation then passes to a registration of another ACL, or is released once none is left, as SPC-4 requires when the last registrant of an all registrants reservation goes away. Fixes: 6c3c9ba ("target: Avoid dropping AllRegistrants reservation during unregister") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-5-5
…with a zero key A PERSISTENT RESERVE OUT PREEMPT with a zero SERVICE ACTION RESERVATION KEY against an all registrants reservation takes the first branch of core_scsi3_pro_preempt(), because the holder's key is not zero. There every registration except the caller's is passed to __core_scsi3_free_registration(). The reservation holder is among them unless the caller holds the reservation. For PREEMPT the free list is NULL, so the holder is freed at once, and dev_pr_res_holder still points at it. __core_scsi3_complete_pro_preempt() then releases the old reservation through dev_pr_res_holder, reading and writing the freed registration, before it makes the caller the new holder. Without KASAN this goes unnoticed: the freed object still holds its old contents, and the command completes as expected. To reproduce, have I_T nexus A register and reserve WRITE EXCLUSIVE - ALL REGISTRANTS, have nexus B register, then send PREEMPT from B with SERVICE ACTION RESERVATION KEY 0. Release the reservation before the loop frees the holder. __core_scsi3_complete_pro_preempt() then finds no holder to release and establishes the new reservation as before. PREEMPT AND ABORT, which defers the free, sees the same release, only earlier. Fixes: c66ac9d ("[SCSI] target: Add LIO target core v4.0.0-rc6") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-5-5
…s the new holder An ALL_TG_PT=1 REGISTER puts the registration on the issuing target port on the registration list, followed by the initiator port's registrations on the other target ports. An UNREGISTER from it frees all of them. If the issuing registration holds an all registrants reservation, core_scsi3_check_implicit_release() releases it first. __core_scsi3_complete_pro_release() does not drop an all registrants reservation when called with unreg set. It makes the next registration on the list the holder instead, and that is one of the other target ports' registrations unless another I_T nexus registered in between. core_scsi3_emulate_pro_register() then frees it, and dev_pr_res_holder is left pointing at freed memory. The next command that checks the reservation dereferences it. To reproduce, register one initiator port with ALL_TG_PT=1 through a target with two target ports, reserve EXCLUSIVE ACCESS - ALL REGISTRANTS, then send REGISTER with SERVICE ACTION RESERVATION KEY 0. After the release, if the new holder is one of the registrations about to be freed, pass the reservation on to the first registration that stays, or release it if none does. SPC-4 requires the release when the last registrant of an all registrants reservation goes away. The test for "about to be freed" moves into core_scsi3_is_atp_sibling(), shared with the loop that frees them, so the two cannot disagree. Fixes: 6c3c9ba ("target: Avoid dropping AllRegistrants reservation during unregister") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-5-5
__core_scsi3_alloc_registration() is called with a NULL deve for a REGISTER from the I_T nexus the command came in on, plain or with SPEC_I_PT=1. __core_scsi3_do_alloc_registration() then looks up the nexus's se_dev_entry and takes a deve->pr_kref reference, which __core_scsi3_add_registration() drops once the registration is added. If allocating the ALL_TG_PT=1 registrations fails, the out: path releases those and frees pr_reg, but never drops that reference. Callers that pass in a deve own its reference and drop it themselves on failure, so only the NULL deve case leaks. Removing a mapping (core_disable_device_list_for_node()) or replacing a dynamic one with an explicit one (core_enable_device_list_for_node()) drops the mapping's own reference and then waits for pr_comp, which is completed only when pr_kref reaches zero. With the leaked reference, that wait never ends: unmapping the LUN, removing the node ACL or the LUN, or making the mapping explicit hangs. Drop the reference on the out: path when it was taken there. Fixes: 3ccd6e8 ("target: Fix PR registration + APTPL RCU conversion regression") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-5-5
…tup fails For a REGISTER with SPEC_I_PT=1, core_scsi3_decode_spec_i_port() first allocates the registration of the local I_T nexus. With ALL_TG_PT=1, __core_scsi3_alloc_registration() also allocates one registration per other target port the initiator is mapped through and queues them on local_pr_reg->pr_reg_atp_list. Each holds a configfs dependency on its mapped LUN and a deve->pr_kref reference. If core_scsi3_lunacl_depend_item() then fails for the local nexus, for instance because its mapped LUN is being removed, local_pr_reg is freed without releasing that list. The registrations leak, and so do their dependencies and references: configfs refuses to remove the other mapped LUNs, and removing one of those LUNs from its TPG, which drops the mapping, hangs waiting for pr_comp. Release the list the same way the out: path below does. Fixes: f32ba61 ("scsi: target: Fix crash during SPEC_I_PT handling") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-5-5
…ncies core_scsi3_decode_spec_i_port() takes a configfs dependency on the mapped LUN (the initiator's lun_N directory) of the issuing I_T nexus, and of each I_T nexus named by a TransportID. Since commit f32ba61 ("scsi: target: Fix crash during SPEC_I_PT handling") it expects __core_scsi3_add_registration() to release them. That function does so only for the extra ALL_TG_PT=1 registrations; for the registration it is given it only drops deve->pr_kref. So after a successful REGISTER with SPEC_I_PT=1, every one of those lun_N directories stays pinned and rmdir fails with -EBUSY until reboot. The ACL and the target cannot be removed either. The error path leaks too. It releases the dependency through tidh->dest_se_deve, which is never set for the issuing I_T nexus. Since commit d8ab68b ("scsi: target: Fix NULL pointer dereference in core_scsi3_decode_spec_i_port()") the NULL is skipped, which leaks that dependency and the deve->pr_kref reference taken by __core_scsi3_alloc_registration(). Set tidh->dest_se_deve for the issuing I_T nexus. In the success loop, drop the configfs dependency after __core_scsi3_add_registration(). The se_lun_acl is read beforehand, while deve->pr_kref still pins the deve; the dependency keeps the se_lun_acl itself alive until it is dropped. Print the mapped LUN from the registration instead of from the deve, which is no longer pinned at that point. Demo-mode ACLs have no se_lun_acl, so they never took the dependency. That is why this has gone unnoticed. Fixes: f32ba61 ("scsi: target: Fix crash during SPEC_I_PT handling") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-5-5
When the destination I_T nexus of REGISTER AND MOVE is not registered, core_scsi3_emulate_pro_register_and_move() registers it with core_scsi3_alloc_registration(), passing dest_se_deve. That deve carries a single deve->pr_kref reference, taken by core_get_se_deve_from_rtpi(). __core_scsi3_add_registration() drops it when the registration completes. core_scsi3_lunacl_undepend_item() then drops it again on the success path, together with the configfs dependency. pr_kref thus reaches zero while the LUN is still mapped, completing pr_comp early. When the LUN is later unmapped, core_disable_device_list_for_node() puts it once more: refcount_t: underflow; use-after-free. WARNING: CPU: 1 PID: 1016 at lib/refcount.c:28 refcount_warn_saturate+0xbe/0x110 Call Trace: core_disable_device_list_for_node+0x117/0x120 [target_core_mod] core_clear_lun_from_tpg+0x8a/0xd0 [target_core_mod] core_tpg_remove_lun+0x2c/0x110 [target_core_mod] target_fabric_port_unlink+0x4c/0x80 [target_core_mod] configfs_unlink+0xed/0x1a0 [configfs] Take a reference for the new registration to consume, and drop it if the registration cannot be allocated. The reference from core_get_se_deve_from_rtpi() is left for core_scsi3_lunacl_undepend_item(), as it already is when the destination I_T nexus is registered. Fixes: 3ccd6e8 ("target: Fix PR registration + APTPL RCU conversion regression") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-5-5
… MOVE REGISTER AND MOVE ignores the SCOPE and TYPE fields of the CDB and moves the reservation with the holder's type and scope. It reads that scope from pr_res_holder->pr_res_type, though, and then stores it in pr_reg, the source I_T nexus's registration, which no longer holds the reservation, instead of in dest_pr_reg. The two mistakes cancel out today. RESERVE and PREEMPT accept only LU_SCOPE, which is 0, so every registration's scope is 0, whether dest_pr_reg was just allocated or already existed, and the new holder ends up with the right scope. Fixing only the second would give the destination a scope equal to the type code, and a RELEASE from it with the scope it reserved with would fail with RESERVATION CONFLICT. Read the scope from pr_res_scope and set it on dest_pr_reg. Assisted-by: Claude:claude-opus-5-5
When a session's Time2Retain timer expires, iscsit_handle_time2retain_timeout() calls iscsit_close_session() from the timer, in softirq context. When no one else is using the session, that goes on to transport_deregister_session() and transport_free_session(), which takes se_tpg->acl_node_mutex, and puts the node ACL, which for a dynamic ACL takes it again. Sleeping in a timer callback is a bug: with CONFIG_DEBUG_ATOMIC_SLEEP it warns, and a contended mutex schedules in softirq. It happens whenever an initiator drops a connection without logging out and does not log in again within DefaultTime2Retain (20 seconds by default), for example after a cable pull or a host crash. Queue the close on a workqueue instead, under session_lock where the timer marks the session expired, and close it there with can_sleep set. Every path that looks for sessions to close skips an expired one, as before, leaving it to the work. iscsit_release_sessions_for_tpg() flushes the workqueue after its loop, so that a TPG being disabled or deleted is not freed while such a close is pending; the timer callback could run concurrently with that before, too. With can_sleep set, iscsit_close_session() waits for other users of the session instead of restarting the timer. No caller passes false any more; the parameter is left for a separate cleanup. Fixes: 01d4d67 ("target: Fix multi-session dynamic se_node_acl double free OOPs") Cc: stable@vger.kernel.org Assisted-by: Claude:claude-opus-5-5
…tate SPC requires each I_T nexus (session) to maintain its own independent Unit Attention queue. LIO instead keys UA state on struct se_dev_entry by (se_node_acl, lun) -- i.e. by initiator identity, not by nexus. Two sessions sharing one identity, the normal shape of a multipath (MPIO) client, therefore share a single UA queue. Whichever session's command drains the queue first silently hides the condition from every other session on that identity. This affects resize UAs, ALUA state-change UAs, and PR-related UAs alike. Introduce struct se_session_deve, a join object binding one active session to one of its mapped LUNs, holding that nexus's own UA queue. Add the struct, the new se_session->deve_list and se_dev_entry->sess_list back-pointer lists, and their lifecycle. A set-up session has exactly one entry per LUN mapped to its node ACL, whichever of the two came first: - A session gets one entry per LUN mapped to its node ACL when it is set up (target_setup_session_deve_entries()), and is added to the new se_node_acl->ua_sess_list. - A LUN mapped to the node ACL later gets one entry per session on that list (target_setup_deve_sess_entries(), from core_enable_device_list_for_node(), before the mapping is published). If that allocation fails, the mapping fails with -ENOMEM. - A dynamic node ACL mapping converted to an explicit one is replaced by a new se_dev_entry; its entries, with any queued UAs, move to the new one (target_move_deve_sess_entries()) instead of being left pointing at the old se_dev_entry, which is freed. - A LUN unmap frees the LUN's entries (core_scsi3_ua_release_all()), and session teardown frees the session's (target_free_session_deve_entries(), from transport_free_session()). All of these run under the node ACL's lun_entry_mutex, which the mapping paths already hold, so they never race with each other: no entry can be freed by both sides, or reached through a se_dev_entry or se_session that the other side is freeing. The mutex is also the only lock writers of the two new lists take: both are RCU lists and readers walk them under rcu_read_lock(). So looking up a session's entry, which the following patch does for every command, takes no lock. Teardown does not wait for a grace period. It takes an entry off both lists, marks it dead under the entry's ua_lock so that no reader can queue another UA on it, drains its queue and frees it with kfree_rcu(). Removing a LUN from a TPG unmaps it from every node ACL in turn, holding the TPG's acl_node_mutex; a synchronize_rcu() per node ACL there would stall logins to the TPG for all of them. Because the session can be freed once its entries are dead, a reader that found an entry through the se_dev_entry may dereference its session only under ua_lock, and only if the entry is not dead. Setting up a session takes that mutex and allocates with GFP_KERNEL, so it cannot happen in __transport_register_session(), which runs under se_portal_group->session_lock. Add transport_setup_session_ua() for it. transport_register_session() calls it before taking the lock, which covers every fabric using target_setup_session(); iSCSI, which calls __transport_register_session() directly, calls it before taking the lock in iscsi_post_login_handler(). No command can arrive on the session yet at either point. On -ENOMEM the failure is logged and the session still works for ordinary I/O; it just gets no per-nexus UA delivery until it reconnects. transport_free_session() now takes a mutex; annotate it with might_sleep(). This patch is additive only: struct se_dev_entry's existing ua_lock/ ua_list fields are untouched, and every current UA producer/consumer keeps reading and writing them directly. The new lists are populated and torn down correctly, but nothing consults them yet, so this patch changes no observable behavior. The following patch migrates every producer/consumer onto se_session_deve and removes the old fields, which is where the bug described above actually gets fixed. Assisted-by: Claude:claude-opus-5-5
Complete the per-I_T-nexus Unit Attention migration started by the previous patch. Migrate every UA producer and consumer onto struct se_session_deve, and remove the now unused ua_lock/ua_list fields from struct se_dev_entry (including their init site in core_enable_device_list_for_node()): - core_scsi3_ua_allocate() becomes core_scsi3_ua_allocate_all(), gaining an exclude_sess parameter so a UA can be raised on every session mapped to a LUN except the one that caused it -- mirroring the existing core_scsi3_ua_release_all()'s naming, since both now operate across every session mapped to a struct se_dev_entry rather than a single queue. A failed allocation for one session does not stop the others being notified; the function returns -ENOMEM if any session missed the UA. - The legacy allocator's queue ordering is replaced. It had three bugs: it looked only at the first queued UA instead of walking the whole list; it compared a 2Ah ASCQ against the queued UA's ASC; and it put the higher 29h ASCQ first, which reverses SAM-5 5.14.1 Table 53 (e.g. I_T NEXUS LOSS OCCURRED ahead of BUS DEVICE RESET FUNCTION OCCURRED). __core_scsi3_ua_insert_sorted() now deduplicates and keeps the queue in reporting order using the Table 53 precedence levels (target_ua_precedence()). Within the lowest level it orders the same ASC by ascending ASCQ, which puts ASCQ 00h first as 5.14.1 requires, and keeps different ASCs in the order they were established. The legacy code also put every 2Ah UA ahead of other lowest-level UAs; SAM-5 ranks those equally, so that is dropped. Every UA is queued through it, so it is also where a UA is refused on an entry that teardown has marked dead (see the previous patch). - target_scsi3_ua_check(), core_scsi3_ua_for_check_condition(), and core_scsi3_ua_clear_for_request_sense() now resolve the calling session's own se_session_deve instead of a nacl-wide deve, by walking se_session->deve_list under RCU. target_scsi3_ua_check(), which runs for every command, still takes no lock: as before it tests the queue with list_empty_careful(). The other two take the entry's ua_lock only to dequeue, and treat an entry that teardown has marked dead as not found, so they never hold ua_lock on an entry that is about to be freed. - core_scsi3_ua_for_check_condition()'s NO_CLEAR (peek) branch now returns true instead of false. Its caller, translate_sense_reason() in target_core_transport.c, discards the populated sense data and substitutes SAM_STAT_BUSY whenever this function returns false -- meaning every NO_CLEAR-mode UA report was silently replaced with BUSY, defeating emulate_ua_intlck_ctrl whenever an admin sets it to a non-default value. This restores the documented behavior. - target_ua_allocate_lun() gains the same exclude_sess parameter as core_scsi3_ua_allocate_all(), for its PR and PR-reservation-conflict callers that need to notify every nexus but the one that caused the condition. All 7 real call sites (6 in target_core_pr.c, 1 in target_core_transport.c's UA-interlock-control reservation-conflict handling) are updated to the new signature; each currently passes NULL, matching existing behavior exactly -- assigning the right exclude_sess value at each call site is a separate, later patch in this series. - core_allocate_nexus_loss_ua() (target_core_tpg.c) gains an exclude_sess parameter for the same reason; its one real caller in tcm_loop.c is updated to pass NULL, since it derives nacl from the departing session itself and has no other session to exclude. - target_dev_ua_allocate() and target_luns_data_has_changed() (target_core_device.c), and __target_attach_tg_pt_gp() (target_core_alua.c), all call the renamed allocator with exclude_sess = NULL: each raises a UA from an administrative or configuration action with no originating command to exclude. - core_alua_queue_state_change_ua() (target_core_alua.c) is updated to call the renamed allocator, currently passing NULL. This is a mechanical, behavior-preserving conversion only -- its own exclusion-granularity bugs (two separate over-exclusion issues, unrelated to this migration) are fixed in the next patch in this series, which changes what this call site passes here. Unchanged here: reporting a UA (CHECK CONDITION with the default UA_INTLCK_CTRL setting, or REQUEST SENSE) still frees every queued UA, not just the one reported. SAM-5 requires clearing only the reported one; a later patch in this series fixes that on its own, since the change is visible to initiators. Fixes: c66ac9d ("[SCSI] target: Add LIO target core v4.0.0-rc6") Fixes: 325c1e8 ("scsi: target: Fix handling of removed LUNs") Assisted-by: Claude:claude-sonnet-5-5
…e UAs core_alua_queue_state_change_ua() has two exclusion-granularity bugs when raising ASYMMETRIC_ACCESS_STATE_CHANGED unit attentions after an explicit STPG (SET TARGET PORT GROUPS): - Whole-LUN skip: comparing tg_pt_gp_alua_lun against the LUN being iterated excludes every session on that LUN, not just the one that issued the STPG. - Whole-nacl skip: comparing tg_pt_gp_alua_nacl against the deve's node ACL excludes every session sharing the issuing initiator's identity -- the same multipath collateral-exclusion bug the rest of this series fixes for UA storage, occurring here on the ALUA path instead. Fix this by replacing tg_pt_gp_alua_nacl (struct se_node_acl *) with tg_pt_gp_alua_sess (struct se_session *), and deleting both `continue` skips in core_alua_queue_state_change_ua() in favor of a single, correctly-scoped exclude_sess argument to core_scsi3_ua_allocate_all(). core_alua_do_port_transition() previously only received a pre-resolved node ACL, with no originating session available at all. Its signature now takes the real session instead, so it can populate tg_pt_gp_alua_sess. target_emulate_set_target_port_groups(), which handles the explicit STPG case, now resolves and passes cmd->se_sess directly. Its other caller, the implicit-transition configfs store function, passes NULL for the excluded session, since an administrative transition has no originating command. Implicit ALUA transitions (not triggered by an explicit STPG) never populate tg_pt_gp_alua_sess, so it is NULL there and nobody is excluded -- matching SPC intent. With the whole-LUN skip gone, tg_pt_gp_alua_lun is never read. Remove it, and the l_lun parameter of core_alua_do_port_transition() that only set it. Fixes: c51c8e7 ("target: use 'se_dev_entry' when allocating UAs") Assisted-by: Claude:claude-sonnet-5-5
…our bytes sess_get_initiator_sid() returns the ISID as the ASCII hexadecimal digits of its 6-byte value (iSCSI uses "%6phN", 12 characters). The core derived sess_bin_isid and pr_reg_bin_isid from that text with get_unaligned_be64(), which reads the first eight characters, that is only ISID bytes 0 to 3. ISIDs that differ only in the trailing qualifier compared equal, so the SCSI initiator ports of one initiator name were merged. Add target_isid_to_bin(), which converts the whole value, and use it where the binary ISID is derived. An initiator port is the initiator name plus the complete ISID (SPC-5 7.6.4.5, RFC 7143 4.4.2 c, 4.4.3 and 11.12.5), and a different ISID is a different port. The binary ISID is also used by the SCSI-2 reservation checks, which now distinguish ISIDs they previously merged. A string that is not a hexadecimal number is converted as before. Fixes: c66ac9d ("[SCSI] target: Add LIO target core v4.0.0-rc6") Assisted-by: Claude:claude-sonnet-5-5
… ACL Persistent reservation notifications (RESERVATIONS RELEASED from REGISTER and RELEASE, RESERVATIONS PREEMPTED from CLEAR, REGISTRATIONS PREEMPTED and RESERVATIONS RELEASED from PREEMPT) are raised with target_ua_allocate_lun(), keyed by the registration's se_node_acl and mapped LUN, and with exclude_sess always NULL. That reaches every session of the node ACL, so when one initiator has several I_T nexuses through the same target port (several ISIDs), a notification meant for one registration also reaches the sibling sessions, including the one that issued the command. A registration already records its I_T nexus: the node ACL (one initiator name on one target port) plus, when the fabric has one, the ISID. Add core_scsi3_pr_match_nexus() to compare a session with that, and core_scsi3_pr_ua_allocate() to raise a unit attention on the registration's LUN for each session that is its nexus, other than the issuing session. Because it matches the recorded nexus rather than a session pointer, a registration needs no live session attached to it: one created for an ALL_TG_PT=1 sibling port, a SPEC_I_PT=1 or REGISTER AND MOVE destination, or one restored from APTPL metadata, is notified through whichever session is its nexus at the time, as is a nexus that has reconnected since it registered. Use it for REGISTER's implicit release, RELEASE and CLEAR. CLEAR now raises the unit attention before freeing the registration, while registration_lock still pins it. PREEMPT also has to tell removed registrations from surviving ones under the same node ACL and is converted later in this series; for now pass the issuing session as exclude_sess at its three sites. The one other target_ua_allocate_lun() caller, the UA interlock in transport_generic_request_failure(), raises the unit attention for the nexus that has just been sent RESERVATION CONFLICT, for that nexus's next command to see. Add target_ua_allocate_sess(), which queues it on that one session's se_session_deve, and use it there. core_scsi3_ua_allocate_all(), target_ua_allocate_sess() and the new target_ua_allocate_sess_deve_locked() share __core_scsi3_ua_queue() to allocate and queue the se_ua. core_scsi3_pr_ua_allocate() finds sessions through the se_dev_entry's sess_list under RCU, and a session can be freed as soon as teardown has marked its entry dead. So it takes the entry's ua_lock and checks that the entry is not dead before core_scsi3_pr_match_nexus() looks at the session. Assisted-by: Claude:claude-opus-5-5
… deve core_scsi3_pr_seq_non_holder() decides whether a command from a non-holder session is allowed against an active persistent reservation by computing a registered_nexus flag. For CLEAR, PREEMPT, and PREEMPT_AND_ABORT, an unregistered nexus gets RESERVATION CONFLICT; a registered one does not. registered_nexus came from two different, both wrong, sources depending on the caller's isid_mismatch parameter -- set by target_scsi3_pr_reservation_check() when the reservation holder shares the caller's se_node_acl but not its ISID (a real multipath session: one initiator, two I_T nexuses): - isid_mismatch true: registered_nexus was hardcoded to 0, without ever checking whether the calling session itself has an active registration. Any nexus sharing an identity with the current holder, but connected via a different ISID, was unconditionally denied CLEAR/PREEMPT even when it held its own valid registration. - isid_mismatch false: registered_nexus came from DEF_PR_REG_ACTIVE, a single bit on struct se_dev_entry -- keyed by (se_node_acl, LUN), shared across every session under one identity, not per I_T nexus. This answers "is *some* session under my nacl+lun registered", not "am I registered" -- a latent false positive for any identity with more than one session; isid_mismatch's hardcoded 0 happened to mask it for the case that matters most. DEF_PR_REG_ACTIVE also can't be cleared correctly for that same multi-session case: __core_scsi3_free_registration() cleared it on any single registration's teardown, with no check for surviving sibling registrations under the same nacl+lun, so unregistering (or being preempted) on one nexus could wrongly clear it while a sibling nexus is still actively registered. Fix both by adding core_scsi3_sess_is_registered(), which answers the question the check actually needs -- does this session's I_T nexus hold a registration on this device -- by walking the device's registration list under registration_lock and matching each registration with core_scsi3_pr_match_nexus() (earlier in this series). Use it unconditionally in core_scsi3_pr_seq_non_holder(), removing the isid_mismatch parameter and the branch it gated, and use core_scsi3_pr_match_nexus() for the holder check in target_scsi3_pr_reservation_check() too. Matching the registration's recorded nexus, rather than looking for a registration bound to the session, also counts registrations created without the session that is now their nexus: ALL_TG_PT=1 siblings, SPEC_I_PT=1 and REGISTER AND MOVE destinations, and APTPL registrations activated while the nexus is logged in. Cost: under an all-registrants or registrants-only reservation, every registrant other than the holder runs this check on every command, so each such command now takes registration_lock and walks the registration list once. With registered_nexus now computed correctly and consistently, DEF_PR_REG_ACTIVE has no remaining reader. Remove it, its two setters in __core_scsi3_add_registration(), the now-unnecessary clearer in __core_scsi3_free_registration(), the clearer in core_disable_device_list_for_node() (full deve teardown, unrelated to the bug above but the last remaining reference), and struct se_dev_entry->deve_flags itself, which had no other bit ever defined on it. Assisted-by: Claude:claude-opus-5-5
…e-nacl nexuses core_scsi3_pro_preempt() has three places that notify a nexus about a PR change (REGISTRATIONS PREEMPTED when its registration is removed, or RESERVATIONS RELEASED for surviving registrants after a type/scope change) via target_ua_allocate_lun(nacl, lun, asc, ascq, exclude_sess). That function fans out to every live session sharing that nacl, not just the one nexus each notification is actually about. PREEMPT, unlike RELEASE/CLEAR, always splits nexuses into a removed group and a surviving group within one call -- and removing a PR registration never removes the underlying live session. So whenever a removed nexus and a surviving nexus share an nacl (a real multipath session: one identity, several I_T nexuses), a broadcast meant for one of them still reaches the other: - The "SA key matches holder" branch removes only the registration(s) matching sa_res_key, then broadcasts REGISTRATIONS PREEMPTED via that registration's nacl -- reaching a same-nacl sibling whose own registration was never touched. - The "no existing/non-matching holder" branch's !all_reg case has the identical shape: removal scoped to sa_res_key, notification scoped to the whole nacl. - The post-preempt "type or scope changed" step broadcasts RESERVATIONS RELEASED via each surviving registrant's nacl to notify every survivor -- correct in intent, but the broadcast also reaches a same-nacl nexus this same PREEMPT call already removed and separately notified moments earlier, which then sees the wrong status win priority sorting over its correct one. Fix by notifying through core_scsi3_pr_ua_allocate() (added earlier in this series for REGISTER, RELEASE and CLEAR), which raises the unit attention only for the sessions that are the registration's own I_T nexus. Each loop already iterates one specific struct t10_pr_registration at a time, so that is exactly the nexus the notification is about. Raise REGISTRATIONS PREEMPTED before __core_scsi3_free_registration(), while registration_lock still pins the registration, as CLEAR already does. target_ua_allocate_lun() has no callers left; remove it. Assisted-by: Claude:claude-opus-5-5
When a command completes with CHECK CONDITION for a unit attention, or a REQUEST SENSE reports one, the unit attention queue of that I_T nexus is emptied: the highest priority entry is reported and every other pending entry is freed without ever being reported. SAM-5 requires the device server to clear the unit attention condition from the queue after reporting it, and only that one. The other pending conditions remain until a later command reports them. The loss is visible to an initiator. If REPORTED LUNS DATA HAS CHANGED is queued behind another pending unit attention, it is dropped when that one is reported, and the initiator never learns that a LUN was added or removed. Add __target_ua_pop_head(), which removes and frees only the head entry, and use it in core_scsi3_ua_for_check_condition() for the default UA_INTLCK_CTRL setting and in core_scsi3_ua_clear_for_request_sense(). The other UA_INTLCK_CTRL settings already report the head without clearing it and are unchanged. __target_ua_drain_queue() is still used where a struct se_session_deve is released, on LUN unmap and on session logout, which must free every queued entry. An I_T nexus with several distinct unit attentions pending now sees one CHECK CONDITION per command, highest priority first, until the queue is empty. Duplicates are still suppressed when a unit attention is queued, so the number is bounded by the number of distinct ASC/ASCQ values. Fixes: c66ac9d ("[SCSI] target: Add LIO target core v4.0.0-rc6") Assisted-by: Claude:claude-sonnet-5-5
…registered I_T nexus SPC-5 5.14.7 requires any PERSISTENT RESERVE OUT service action other than REGISTER and REGISTER AND IGNORE EXISTING KEY, received from an unregistered I_T nexus, to be completed with RESERVATION CONFLICT. Table 71 gives the same result for REGISTER AND MOVE. PREEMPT, PREEMPT AND ABORT and the two REGISTER service actions already behave this way. RELEASE, RESERVE, CLEAR and REGISTER AND MOVE instead treat a failed lookup of the issuing nexus's registration as an internal error and complete with NOT READY, LOGICAL UNIT COMMUNICATION FAILURE. A nexus that holds no registration is a normal state, for example another ISID of an initiator that is registered through a different ISID, so return RESERVATION CONFLICT for it. For the same reason, log it with pr_debug() rather than pr_err(): any initiator can cause it, and could otherwise fill the kernel log. This is visible to initiators: an unregistered nexus that issues one of these four service actions now gets RESERVATION CONFLICT, which tells it that it must register first, rather than a check condition reporting that the logical unit is not ready. Fixes: c66ac9d ("[SCSI] target: Add LIO target core v4.0.0-rc6") Assisted-by: Claude:claude-sonnet-5-5
Export target_dev_ua_allocate() and wire a real rescan=1 token into iblock/fileio's control attribute, raising CAPACITY DATA HAS CHANGED on every attached I_T nexus. Assisted-by: Claude:claude-sonnet-5-5
On the STANDBY node, a global atomic flag (lio_ha_forward_active) redirects all I/O and TMR away from local backends and over a TCP channel to ACTIVE. lio_ha.ko registers its forwarding callbacks with lio_ha_register_ops() and sets the flag to activate interception. The ops pointer is read under SRCU, and lio_ha_unregister_ops() waits for every reader, so the module can go away while commands are being submitted. With the flag set and no ops registered, a command fails with LOGICAL UNIT COMMUNICATION FAILURE and a TMR is rejected. The session hooks run in process context and may sleep. A nacl_release hook is called from core_tpg_del_initiator_node_acl() once the ACL's fabric sessions are shut down and before LIO core waits for its last reference. Sessions lio_ha.ko builds for initiators logged in to the peer hold the ACL without being on its session list; the hook has them removed, so deleting the ACL does not wait for the initiator to log out of the peer. Backend devices are created at STANDBY startup but their block devices are not opened until the pool is imported at failover. Furthermore, a per-device opened configfs attribute allows individual LUN recovery after partial failover failures, such as when a dataset or zvol remains locked or unavailable. Until a backend is open, target_cmd_parse_cdb() fails every command to it, so neither the backend nor PR emulation runs against a device with no storage, and fd_get_blocks() tolerates a fileio device with no file. PR changes on ACTIVE are reported to lio_ha.ko through the pr_change() hook of struct lio_ha_pr_notifier, called after each successful PR OUT with the device whose state may have changed. lio_ha.ko then sends the device's whole PR state, which target_ha_pr_export() copies under dev_reservation_lock and registration_lock and formats after dropping them, using the field names and values of core_scsi3_update_aptpl_buf() plus the initiator_sid=, tpgt= and res_all_tg_pt= fields of each registration. Sending the whole state rather than the operation keeps every service action, including SPEC_I_PT, PREEMPT, RELEASE and REGISTER AND MOVE, consistent with the core by construction. The PR OUT completes to the initiator without waiting for the peer. At failover, lio_ha.ko restores replicated PR state by resolving the TPG each registration was made on with target_ha_get_tpg(), which pins it against deregistration until target_ha_put_tpg(), then calling target_ha_foreach_tpg_nacl_dev() to find the live nacl+lun mappings there and target_ha_pr_add_reg() to inject the registration, with its discriminator and ALL_TG_PT flag, into registration_list. For ALL_TG_PT registrations, target_ha_foreach_nacl_dev() returns the mappings on which LIO creates the sibling registrations. NULL and empty discriminators are treated alike, so a fabric without one does not produce a registration that no session can match. For a nacl and discriminator the device already has, target_ha_pr_add_reg() updates the key under both PR locks, and installing a reservation releases a different existing holder first. A registration belongs to a physical target port, so struct t10_pr_registration records the port's tag and the matching and lookup paths compare it with the nexus's. The tags are global: node B's TPG tags are node A's plus 32000, so a tag names one port on either node. A session forwarded from the peer carries the tag and relative target port identifier of the peer's port, and target_ha_pr_add_reg() takes both. Two nexuses with the same initiator name and ISID through different ports are then separate registrations. For ALL_TG_PT, a registration is also made on the paired port of the other node, found through the notifier's peer_port_tag() callback and target_ha_tpg_rtpi(), so the initiator is registered on every physical port. No registration is made for a peer port whose TPG does not exist. Sessions forwarded from the peer are registered on the ha_recv TPG while their node ACL belongs to the real one. Under CONFIG_TRUENAS, core_scsi3_locate_pr_reg() takes the TPG from the session, so the discriminator is read through the ops the session is registered with, and neither it nor core_scsi3_emulate_pro_register() treats an empty discriminator as present. After PR state is restored, target_ha_reopen_backend() calls configure_device() directly (skipping the already-configured guard) to open each deferred backend and refresh its block geometry attributes. (This commit covers the LIO core side only; the lio_ha.ko module that drives the hooks will follow in a separate commit.) Assisted-by: Claude:claude-sonnet-5-5 Assisted-by: Claude:claude-opus-5-5
Introduces lio_ha.ko, a kernel module that implements active/standby high-availability for the LIO SCSI target subsystem on TrueNAS. On the STANDBY node, the module intercepts every SCSI command, TMR, and PR OUT before it reaches the local backend and forwards it to the ACTIVE node over a private TCP channel (port 999). On failover a single write to a configfs attribute clears the forwarding flag; lio_ha restores replicated Persistent Reservation state directly into LIO core, opens the local iblock/fileio backends, and resumes local execution -- without removing or reinserting any LUN. Forwarding is set only when the module is loaded, with default_forward_active=1; once cleared it cannot be set again, and a node that is to be STANDBY again reloads the module. On the ACTIVE node, a private fabric driver (ha_recv) accepts the TCP connection from STANDBY, creates a synthetic se_session for each initiator with the correct node_acl, and submits forwarded commands to LIO core for local execution under the right I_T nexus context so that PR per-I_T nexus checking remains correct across the HA pair. Where the fabric has a nexus discriminator (the iSCSI/iSER ISID), it travels in SESSION_CONNECT and is reported through an ha_recv_ops callback, so registrations made through STANDBY record the same discriminator as those made directly. Fabrics without one, such as FC, send it empty. The module is loaded on both nodes. A single persistent TCP connection carries session lifecycle, I/O, TMR and PR sync (LUN_SYNC) traffic. Every message starts with a 12-byte header holding a magic number, the protocol version, the channel and the payload length; a node closes the connection on a header it does not accept, or on a CTL message whose reserved field is set, and every message is checked against its payload length before it is used. Only the node with the lower address dials, the listener retries on failure, and both sockets have a 10 second send and user timeout; a failed send closes the connection. Session and command identifiers are pointers on the sending node, so nothing sent on one connection may be applied on the next. When the link drops, ha_recv stops each synthetic session, waits for its commands and removes it, and frees every buffered WRITE payload; the STANDBY completes every forwarded command; and no new connection is accepted until that teardown has finished. The STANDBY then replays SESSION_CONNECT for its live sessions, serialised with new connects and disconnects. Forwarded READ data that was not delivered completes with BUSY rather than GOOD. On ACTIVE, a PR change marks the device dirty and one work item sends the device's whole PR state, from target_ha_pr_export(), as a LUN_SYNC; the same mutex covers the snapshot, the send and the full sync at link-up, so the STANDBY receives each device's states in order. The STANDBY replaces the device's entries in one step. Its replicated PR table is keyed on the full I_T nexus: initiator, nexus discriminator, target name and the physical port's tag, so registrations that differ only in discriminator or target port stay separate, and an ALL_TG_PT entry also stands for the sibling registrations LIO creates on the initiator's other ports. The tags are the global physical-port tags and are stored as sent, with no translation by the receiver. ha_recv builds the session on the real TPG's node ACL and records the physical port in the session, so registrations made through STANDBY are attributed to STANDBY's port. At failover each registration is restored on the port it was made on, with its discriminator. The node ACL comes from whichever of the entry's TPG and its paired TPG has a static ACL. An entry with no such ACL is skipped with a warning. ALL_TG_PT entries also recreate the sibling registrations on the other ports, including the paired port on this node. The reservation goes on the registration for the port it was taken through, which for an ALL_TG_PT registrant need not be the port it registered through. Deleting a node ACL on ACTIVE first removes the synthetic sessions built on it, waiting for their commands (the core's nacl_release hook). Enabled by CONFIG_LIO_HA (tristate, depends on CONFIG_TRUENAS). Assisted-by: Claude:claude-sonnet-5-5 Assisted-by: Claude:claude-opus-5-5
Contributor
Author
|
Opened by mistake |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.