[PATCH 1/2] nvmet: fix use-after-free of the old registrant in nvmet_pr_preempt()

[Daejun Park]

nvmet_pr_preempt() handles a preempt sent by the reservation holder by
changing the reservation type with nvmet_pr_update_reg_attr(), unless
the reservation is of an All Registrants type.  For the holder that
helper does not modify the registrant in place: it puts a copy on the
registrant list, points pr->holder to the copy and passes the old
registrant to kfree_rcu().

If the reservation type changed, nvmet_pr_preempt() then hands
&reg->hostid of that old registrant to nvmet_pr_resv_released().  Unless
the reservation released notification is masked, that function takes
subsys->lock and compares the host id against every controller of the
subsystem.  The caller only holds pr->pr_sem and is not in an RCU
read-side critical section, so nothing keeps the old registrant alive
while it is read.

The result is a read of 16 freed bytes that decides which controllers
get the notification.  Found by code inspection.  kfree_rcu() defers
the free, so it usually happens after the read; when the pending free
is forced to run before the read, KASAN reports the host id as freed
memory.

Pass the host id of the controller that sent the command instead.  The
registrant was looked up by that host id, and the controller outlives
the request.

Fixes: 5a47c2080a73 ("nvmet: support reservation feature")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Daejun Park <daejun7.park@xxxxxxxxxxx>
---

Notes:
    The KASAN report came from this change on top of the base commit, which
    forces the pending free and then checks the pointer that is handed to
    nvmet_pr_resv_released():
    
     		status = nvmet_pr_update_reg_attr(pr, holder,
     				nvmet_pr_update_holder_rtype, &rtype);
    +		kvfree_rcu_barrier();
    +		kasan_check_read(&reg->hostid, sizeof(reg->hostid));
     		if (!status && original_rtype != rtype)
     			nvmet_pr_resv_released(pr, &reg->hostid);
    
    A holder that preempts itself with another reservation type then gives:
    
      BUG: KASAN: slab-use-after-free in nvmet_pr_preempt.isra.0+0x462/0xa50
      Read of size 16 at addr ff110001113bb088 by task kworker/u32:0/12
      ...
      Allocated by task 12:
       ...
       nvmet_execute_pr_register+0x1f4/0x1130
      ...
      Freed by task 864:
       ...
       __rcu_free_sheaf_prepare+0x61/0x230
       rcu_free_sheaf_nobarn+0x1d/0x50
       rcu_core+0x691/0x1e20
    
    The free in this report is the one forced by the added barrier.  The
    same two lines with &ctrl->hostid on top of this patch give no report.

 drivers/nvme/target/pr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/nvme/target/pr.c b/drivers/nvme/target/pr.c
index 09d8c63f5..bfd5447f6 100644
--- a/drivers/nvme/target/pr.c
+++ b/drivers/nvme/target/pr.c
@@ -589,7 +589,7 @@ static u16 nvmet_pr_preempt(struct nvmet_req *req,
 		status = nvmet_pr_update_reg_attr(pr, holder,
 				nvmet_pr_update_holder_rtype, &rtype);
 		if (!status && original_rtype != rtype)
-			nvmet_pr_resv_released(pr, &reg->hostid);
+			nvmet_pr_resv_released(pr, &ctrl->hostid);
 		return status;
 	}
 
-- 
2.43.0