[PATCH 0/2] nvmet: fix Reservation Acquire preempt sent by the holder

[Daejun Park]

nvmet_pr_preempt() does not follow NVMe Base Specification 2.1, section
8.1.22.7 when a Reservation Acquire command with the Preempt or Preempt
and Abort action is sent by the reservation holder and the reservation
is not of an All Registrants type.

Patch 1 fixes a use-after-free read in the path that handles a preempt
sent by the holder.  It does not depend on patch 2 and comes first so
that it can be backported alone.

Patch 2: a holder that preempts with the key of another registrant gets
a successful completion, but that registrant stays registered and the
reservation type is changed to RTYPE.  The pNFS SCSI layout server
fences clients with this command (RFC 9561 section 2.2.3), so on an
nvmet namespace nfsd logs a client as fenced while the client can still
write to the device.  The patch lists what changes for a host that
holds the reservation.

Not changed by this series: when the holder preempts itself,
registrants that share its key stay registered.  Case a) of section
8.1.22.7 unregisters them, but the paragraph about a holder preempting
itself lists three effects and does not mention other registrants, so I
left it alone.  The SCSI target removes them in
core_scsi3_emulate_pro_preempt().  I can send a patch if that is the
intended reading.

The series is based on the block-7.3 branch of the block tree at
684b413b5483, which has the current nvme fixes, and was tested there
with KASAN, PROVE_LOCKING and PROVE_RCU enabled, in QEMU guests, with
nvme-cli 2.8.  drivers/nvme/target/pr.c is the
same file there, in nvme-7.3, in nvme-7.4 and in mainline.  "before" is
that commit, "after" is that commit with both patches.  Each case was
run once unless noted.  The runs with the patches applied produced no
KASAN or lockdep report.  Mainline ce1e0223d8ad, which does not have
"nvmet: copy the hostid into the ctrl before creating PR pc_refs" yet,
gave the same results before and after.

1. nvme-loop, hosts A, B and C registered on one namespace with
   different keys, A holds the reservation.  "regs" is the number of
   registrants after the command.  EA-RO is Exclusive Access -
   Registrants Only, WE Write Exclusive, WE-RO Write Exclusive -
   Registrants Only, EA-AR Exclusive Access - All Registrants.

   reservation and command                     before          after
   EA-RO, A preempts key of B, same RTYPE      regs 3          regs 2
   EA-RO, A preempts key of B, other RTYPE     regs 3, type    regs 2, type
                                               changed         unchanged
   EA-RO, A preempts and aborts key of B       regs 3          regs 2
   WE,    A preempts key of B                  regs 3          regs 2
   WE-RO, A preempts and aborts key of B       regs 3          regs 2
   EA-RO, A preempts with PRKEY 0h,            success, type   Invalid Field
          other RTYPE                          changed         in Command
   EA-RO, A preempts with an unregistered      success         Reservation
          key, same RTYPE                                      Conflict
   EA-RO, A preempts itself with another RTYPE regs 3, type    same
                                               changed
   EA-RO, C preempts key of B                  regs 2          same
   EA-AR, A preempts key of B                  regs 2          same
   EA-AR, A preempts with PRKEY 0h             regs 1          same

   With EA-RO and WE-RO, writes from B succeed while B is registered and
   fail with Reservation Conflict once it is unregistered.

2. blktests nvme/054 passes before and after, with nvme-loop and
   nvme-tcp.  A test for the multi host cases is posted separately as
   "[PATCH blktests] nvme/071: test reservation preempt with multiple
   hosts".  It fails before and passes after (nvme-loop three times,
   nvme-tcp once, and both transports in one run).

3. pNFS SCSI layout.  An nfsd server exports XFS on an nvmet-tcp
   namespace to two clients, all nodes run the same kernel.  The server
   and both clients are registered, the server holds an EA-RO
   reservation.  Client 1 holds a layout and writes 1 MiB with O_DIRECT
   every 0.2 seconds.  Its network link to the server is then blocked
   while its link to the NVMe target stays up, and a local write to the
   file on the server recalls the layout.  The server write waits for
   the lease break time (45 seconds by default), then nfsd fences
   client 1 and logs "FENCED client[...]" in both cases.  The link is
   restored about 15 seconds after the fence, and the RPC counter is
   read when client 1 stops writing.

                                               before          after
   seconds the server write waited             49              49
   registrants after the fence                 3               2
   bytes client 1 wrote to the device in the
   10 seconds after the fence                  45088768        0
   WRITE RPCs sent by client 1 by the end      0               172

   After the fence client 1 logs "reservation conflict error, dev
   nvme0c0n1" and, once the link is back, sends its writes to the
   server.

   A run with the patches in which client 1 was cut off from both the
   server and the NVMe target gave 45 seconds, 2 registrants and 0
   bytes; the fence did not block on the unreachable client.

Daejun Park (2):
  nvmet: fix use-after-free of the old registrant in nvmet_pr_preempt()
  nvmet: fix preempt of another registrant by the reservation holder

 drivers/nvme/target/pr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


base-commit: 684b413b5483f57c890c171b9400076a0143b918
-- 
2.43.0