| Age | Commit message (Collapse) | Author |
|---|
|
nvme updates for Linux 7.4
- Fix regressions from the parallel nvme-tcp I/O queue setup: request
PDU sizing with header digest, a NULL controller dereference when
reconnecting with more queues, and early lock initialization
- Handle TLS control records in nvme-tcp: skip records arriving ahead
of the ICResp, and start error recovery when the socket read fails
- Fix an nvme-tcp io_work priority inversion stalling I/O
- Fix nvme-tcp address attribute race with queue teardown
- Validate the TLS handshake timeout parameters for host and target
- Fix fabrics connects with isolated CPUs and queue mapping bugs
- Fix early boot hang and buffer size for the nvme.quirks parameter
- Fix nvme-rdma NULL metadata SGL dereference with T10-PI
- Fix ANA underflow check and an nvme-fc discovery reference leak
- Fix nvme-loop BUG() when a namespace scan races a reset
- Target port and configfs locking fixes, and port removal races
- Target auth fixes for CHALLENGE failure and negotiation handling
- Target fc fixes for a connection use-after-free, read data on failed
commands, and admin queue allocation failure teardown
- Target rdma fix for short connection requests
- Target rdma fix for inline SGL out-of-bounds write
- Target rdma refcount, locking, and module unload fixes
- Target pci-epf SGL length and error status fixes
- Target NULL dereference fixes for pci-epf and debugfs
- Target ZNS NUMD overflow check
- Target Connect fix for a controller leak with a zero queue size
- Target fixes for a metadata SGL double-free and a leaked blk plug
- Target fix for truncated large discards
- Misc cleanups
* tag 'nvme-7.4-2026-10-28' of git://git.infradead.org/nvme: (44 commits)
nvmet: don't clear sq->ctrl for a Connect with sqsize 0
nvmet-rdma: fix double puts after a failed RDMA WRITE
nvmet-rdma: disable interrupts under rsp_wr_wait_lock
nvmet-rdma: flush nvmet-wq on module exit
nvme-loop: don't BUG() if a scan runs beside a reset
nvmet-tcp: don't read a removed port at Connect time
nvmet: fix NULL dereference in debugfs host_traddr
nvmet-tcp: stop listening when a port is removed
nvmet: fix NULL dereference of PCI endpoint targets
nvmet-auth: take only AUTH_Negotiate for a negotiation
nvmet: don't truncate the length of big discards
nvme-tcp: validate the TLS handshake timeout
nvmet-tcp: validate the TLS handshake timeout
nvmet: Free the old negotiation state when restarting authentication
nvmet: Finish the blk plug when integrity allocation fails
nvme-tcp: fix NULL queue->ctrl dereference on reconnect
nvme-tcp: Validate the queue state under the queue lock in the address attribute
nvme-tcp: fix io_work priority inversion when send_mutex is held
nvme-tcp: fix request PDU size when header digest is enabled
nvme-tcp: skip TLS control records while waiting for the ICResp
...
|
|
nvmet_install_queue() checks sqsize first and jumps to err on zero,
where sq->ctrl is set to NULL. Since commit b71df12605ca the cmpxchg()
that sets sq->ctrl comes after the checks, so err now undoes a bind
that this call never made. Over RDMA a host can send two Connects
together and the second is parsed before the first executes, so a
second Connect with sqsize 0 clears the sq->ctrl the first one set.
The reference it held is never dropped and the controller leaks, with
its cntlid, until reboot. The discovery subsystem lets any host do
that, 20 connections leak around 10 controllers. A pending AER on such
a queue hangs nvmet_sq_destroy() forever, and a queue can keep its
"authenticated" mark across a rebind.
Return the status directly, as the two checks after it do, so err is
only reached after this call's cmpxchg() succeeded.
Fixes: b71df12605ca ("nvmet: avoid duplicate qid in connect cmd")
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
For a read with T10-PI the RDMA WRITE is posted alone and
nvmet_rdma_write_data_done() posts the SEND once the metadata check
result is known. By then the request has been completed and its
references put: __nvmet_req_complete() put ns->ref and the pc_ref, and
nvmet_req_complete() sq->ref, right after queuing the response. The
error branch of nvmet_rdma_write_data_done() copies the one from
nvmet_rdma_read_data_done() and calls nvmet_req_uninit(), which puts
all three again. A WRITE fails whenever the connection ends with reads
in flight, which is up to the host. Depending on timing the sq->ref
count goes to zero early and the queue is freed under the remaining
completions, or never reaches zero and nvmet_sq_destroy() sleeps
forever, or percpu_ref_put() dereferences NULL after percpu_ref_exit().
ns->ref stays too low for good, so disabling the namespace hangs too.
Reproduced with KASAN over rxe with a test patch forcing the PI path.
Drop the nvmet_req_uninit(). The branch still releases the RDMA context
and the response, which is all the request owns at that point.
Fixes: b09160c3996c ("nvmet-rdma: add metadata/T10-PI support")
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
rsp_wr_wait_lock protects the list of commands waiting for room in the
send queue, and it's taken with plain spin_lock(). But
nvmet_rdma_queue_response() calls nvmet_rdma_release_rsp() directly if
ib_post_send() fails, and that processes the wait list. The call chain
starts in nvmet_req_complete(), which for a block device namespace runs
from the bio end_io callback, which can be in hard interrupt context.
An interrupt that completes a command on a CPU holding the lock for the
same queue spins forever. Lockdep over siw with a null_blk namespace:
inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage
Use spin_lock_irqsave() for both takers. The processing loop still
drops the lock, and restores the caller's interrupt state, around
nvmet_rdma_execute_command().
Fixes: 8f000cac6e7a ("nvmet-rdma: add a NVMe over Fabrics RDMA target driver")
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_rdma_exit() relies on ib_unregister_client() to flush nvmet-wq
through nvmet_rdma_remove_one(), but since commit a3dd7d0022c3 that
returns at once for a device no longer on device_list. The release work
of the last queue takes the device off the list in nvmet_rdma_free_dev()
and still has to drop the mutex, free the struct and return through
nvmet_rdma_release_queue_work(), so an rmmod in that window frees the
module code it's running.
Flush nvmet-wq after ib_unregister_client(). No release work can be
queued after that point.
Fixes: a3dd7d0022c3 ("nvmet-rdma: Don't flush system_wq by default during remove_one")
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
A reset sets queue_count to 1 while it destroys the I/O queues and
counts it back up when the new ones are made, and it doesn't stop the
scan work. A new namespace found in that window trips the check in
nvme_loop_init_hctx() for every hctx but the first:
kernel BUG at drivers/nvme/target/loop.c:224!
nvme_loop_init_hctx+0x7d/0x80
__blk_mq_realloc_hw_ctxs+0x3af/0xa80
nvme_alloc_ns+0x1a2/0x22f0
The check protects nothing. ctrl->queues has nr_io_queues + 1 entries
for the life of the controller, the tag set never has more hardware
queues than that, and nvme_loop_queue_rq() rejects requests until the
controller is live again.
Remove it.
Fixes: 3a85a5de29ea ("nvme-loop: add a NVMe loopback host driver")
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_tcp_alloc_cmd() takes the nvmet port of a command from
queue->port, and the commands of a queue are allocated from the io work
when the Connect arrives. nvmet_tcp_remove_port() shuts down the queues
that have no controller yet and frees the port without waiting for
them, they're released later from a work. An io work handling a Connect
at that moment reads the freed port for every command of the queue and
hands the result to the core.
Note the nvmet port in the queue at allocation. It outlives the
transport port.
Fixes: 872d26a391da ("nvmet-tcp: add NVMe over TCP target driver")
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
The host_traddr callbacks of tcp, rdma and fc take the host address from
the transport queue around ctrl->sqs[0], which nvmet_sq_destroy() clears
when the admin queue goes away while the controller lives on with its
I/O queues. A host that closes the admin connection and keeps an I/O
queue leaves such a controller behind, and reading its host_traddr in
debugfs dereferences a near-NULL pointer:
BUG: kernel NULL pointer dereference, address: 00000000000002e0
RIP: 0010:nvmet_tcp_host_port_addr+0x19/0x50 [nvmet_tcp]
Don't ask the transport if there's no admin queue. Nothing holds the
queue for debugfs, so a reader that's past the check when the queue
goes away is still on its own.
Fixes: 7e5c3de3f2f2 ("nvmet: add 'host_traddr' callback for debugfs")
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_tcp_accept_work() accepts until there's nothing left. A
connection that is already dead when accepted is torn down right there,
which takes a few ms, and more can arrive meanwhile.
nvmet_tcp_remove_port() waits for the work with cancel_work_sync(), so
someone opening and closing connections in a loop keeps the removal from
finishing. It holds nvmet_config_sem for writing, so nothing can be
configured and no port answers a Connect until the flood stops. No
secret needed, just an admin removing the port at the wrong time.
Shut the listening socket down first. Pending connections are dropped,
as they were on release, and the next kernel_accept() fails and ends the
work.
Fixes: 872d26a391da ("nvmet-tcp: add NVMe over TCP target driver")
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_pci_epf_create_ctrl() calls nvmet_alloc_ctrl() without a queue,
the admin queue comes later from the host. nvmet_alloc_ctrl() passes
args->sq to nvmet_queue_tls_keyid(), which with
CONFIG_NVME_TARGET_TCP_TLS dereferences it, so every PCI endpoint
controller creation crashes on such a kernel. The debugfs tls_key file
passes ctrl->sqs[0] to the same function, which is NULL until the admin
queue exists and again once a fabrics host has closed it while keeping
an I/O queue.
A queue that isn't there has no TLS key.
Fixes: fa2e0f8bbc68 ("nvmet-tcp: support secure channel concatenation")
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_execute_auth_send() hands a common-type message to
nvmet_auth_negotiate() if its ID is AUTH_Negotiate or matches the step
the queue is in. The second case is wrong: the only other common-type
message a host sends is AUTH_Failure2, and no queue waits in that step.
A host can name the step on purpose though, and its message is then
treated as a new AUTH_Negotiate without the work that goes with a new
transaction, like reloading the keys on the admin queue.
Fail every other common-type message, as AUTH_Failure2 already is.
Fixes: db1312dd9548 ("nvmet: implement basic In-Band Authentication")
Cc: Hannes Reinecke <hare@suse.de>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_bdev_execute_discard() shifts the 32-bit nlb of a DSM range into
sectors before widening it to sector_t, so on a 4k namespace the top
bits are lost. A 3 TiB range discards 1 TiB, a 2 TiB range discards
nothing, and both complete with success. The target announces no DMRSL,
and hosts other than Linux (and nvme dsm passthrough) do send ranges
that big.
Widen before the shift.
Fixes: a07b4970f464 ("nvmet: add a generic NVMe target")
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
tls_handshake_timeout is writable but stored as a signed int. Multiplying
the value by MSEC_PER_SEC and HZ therefore uses signed arithmetic.
With HZ=1000, setting it to 2147484 overflows both multiplications. If
the handshake request is accepted and remains pending, the timer wheel
treats the jiffies delay as already expired and queues the timeout work
on its next advance, while the handshake agent was given about 24.9
days. Negative values such as -1 cause the same immediate kernel-side
timeout.
Store the parameter as an unsigned int and reject values that cannot be
represented both by tls_handshake_args::ta_timeout_ms and by a safe
jiffies delay. Take one snapshot so a concurrent parameter update cannot
give the handshake agent and delayed work different timeouts. Document
the accepted range for built-in use.
Negative and oversized values now fail with -EINVAL. Keep zero valid,
preserving the existing immediate-timeout behavior.
Fixes: 675b453e0241 ("nvmet-tcp: enable TLS handshake upcall")
Cc: stable@vger.kernel.org
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Yingjie Wang <1075151112@qq.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
Restarting a DH-HMAC-CHAP negotiation with SEND(NEGOTIATE) resets
sq->dhchap_step but does not release the previous c1/c2/skey, which
are only freed in nvmet_auth_sq_free() at teardown. The next
CHALLENGE/REPLY exchange then overwrites those pointers with fresh
allocations.
An unauthenticated host can repeat the negotiate message endlessly,
leaking a few dozen bytes per round and slowly exhausting target
memory.
Clear the old negotiation state with nvmet_auth_sq_free() before
setting up the new one. Also reset dhchap_skey_len to zero: if a DH
session key was already established, a following round with dhvlen=0
would otherwise pass a NULL skey with a stale non-zero length to the
HMAC code and dereference it.
Fixes: db1312dd95488 ("nvmet: implement basic In-Band Authentication")
Cc: stable@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_bdev_execute_rw() starts a plug before building the bios, but
the two error exits taken when nvmet_bdev_alloc_bip() fails return
without calling blk_finish_plug(). current->plug is left pointing
at the stack frame of the returned function, so the next
blk_flush_plug() walks a stale list.
Route both failures through a common exit that finishes the plug.
Fixes: c6e3f13398123 ("nvmet: add metadata support for block devices")
Cc: stable@vger.kernel.org
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
When the initial admin queue allocation fails after a new association
was created, nvmet_fc_ls_create_association() only drops the
association reference, which frees the association. By then the
association is already on the target port's assoc_list, though, and
removal from that list only happens in nvmet_fc_delete_target_assoc(),
which this error path never calls. The freed association stays on the
list, where later lookups and the port teardown can access it, and the
target port reference taken at allocation time leaks.
A remote host can trigger this reliably with a Create Association LS
requesting a queue too large to allocate.
Schedule the regular association teardown instead of dropping the
reference directly. The teardown removes the association from the
list and releases the target port reference before the final reference
drop frees it.
Fixes: 0cab4404874f ("nvme-fc: fix a missing queue put in nvmet_fc_ls_create_association")
Cc: stable@vger.kernel.org
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
In nvmet_execute_auth_receive(), when nvmet_auth_challenge() fails (for
example when the initiator-supplied allocation length is shorter than the
DH-HMAC-CHAP challenge response, or the challenge nonce allocation fails),
the error path sets status = NVME_SC_INTERNAL and breaks out of the
switch. The unconditional
status = nvmet_copy_to_sgl(req, 0, d, al);
after the switch then overwrites the error status (nvmet_copy_to_sgl()
returns 0 on success), so the AUTH_RECEIVE command incorrectly completes
with SUCCESS and the initiator proceeds with a truncated/empty challenge.
Fix this by freeing the buffer and jumping to the common done: label on
CHALLENGE failure, skipping the copy_to_sgl() entirely and preserving the
NVME_SC_INTERNAL status.
Fixes: db1312dd9548 ("nvmet: implement basic In-Band Authentication")
Cc: stable@vger.kernel.org
Signed-off-by: Shao Mingyin <shao.mingyin@zte.com.cn>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_disable_port() looks up the transport from the current
disc_addr.trtype, while nvmet_enable_port() records the transport in
port->tr_ops. If trtype is changed while a port is being enabled, the
port can be removed through a different transport, passing the private
data of the original transport to the wrong ->remove_port().
The same mismatch is reachable with loop and RDMA: loop add_port()
leaves port->priv NULL, and RDMA remove_port() dereferences it.
Use port->tr_ops for removal and serialize trtype changes with
nvmet_config_sem in nvmet_addr_trtype_store(), so the transport that
added the port is also the one that removes it.
Fixes: 8f000cac6e7a ("nvmet-rdma: add a NVMe over Fabrics RDMA target driver")
Reported-by: syzbot+c98b21427617af7f547a@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=c98b21427617af7f547a
Link: https://lore.kernel.org/all/6acbc835.a150a118.27e199.003c.GAE@google.com/T/
Signed-off-by: Rongzheng Wang <testwrzwrz@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
Readers of nvmet_ports_list (e.g. nvmet_subsys_disc_changed()) hold
nvmet_config_sem and assert it via lockdep. The writers --
nvmet_ports_make() and nvmet_port_release() -- do not take the lock;
they rely on the parent directory's VFS inode lock for serialization,
which is an implementation detail of configfs and not a guaranteed
contract. Align the writers with the lock the readers already require.
Also move list_add() in nvmet_ports_make() to after the port is fully
initialized: the old position published the port on the global list
before INIT_LIST_HEAD(&port->subsystems), so a concurrent reader
could traverse uninitialized list heads.
flush_workqueue() in nvmet_port_release() stays outside the semaphore
to avoid a lock dependency on nvmet_wq.
Fixes: b662a078576e ("nvmet: enable Discovery Controller AENs")
Reviewed-by: Maurizio Lombardi <mlombard@redhat.com>
Signed-off-by: Qing Zhang <diasyzhang@tencent.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
If metadata SGL allocation fails, nvmet_req_alloc_sgls() frees
req->sg without clearing it. The RDMA error completion path later
frees the same scatterlist again.
Use nvmet_req_free_sgls() instead so that the scatterlist pointers
and counts are cleared before response cleanup.
Fixes: c6e3f13398123 ("nvmet: add metadata support for block devices")
Signed-off-by: Avi Weiss <thnkslprpt@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
The PCI endpoint target sends the host every byte a read command's SGL
describes, even when the SGL describes more than the command transfers.
The excess is kmalloc() memory the command never wrote, so the host
receives stale target memory. Any successful read can do this,
including Identify.
nvmet_pci_epf_alloc_iod_data_buf() allocates each segment at the length
of its SGL data descriptor, but sets req->transfer_len from
iod->data_len, the length the command implies. Once the command
completes, nvmet_pci_epf_transfer_iod_data() sends every segment in
full. Nothing compares the descriptor lengths with iod->data_len, and
the MDTS check only bounds iod->data_len.
Fail the command with Data SGL Length Invalid when the data descriptors
do not add up to iod->data_len, before any segment is allocated. The
tcp and rdma transports take the data length from the SGL itself, so
there most commands already reject a mismatch in
nvmet_check_transfer_len(). PRP lists are built from iod->data_len and
are unaffected.
Found by code inspection.
Fixes: 0faa0fe6f90e ("nvmet: New NVMe PCI endpoint function target driver")
Cc: stable@vger.kernel.org
Tested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
When the PCI endpoint target fails a command itself, the host receives a
successful completion. This happens for a data length above MDTS,
malformed PRPs or SGLs, a buffer allocation failure, an SQ that is not
live, and a failed DMA of the command data. A write whose data cannot be
fetched from the host is thus reported as written, though nothing was
executed.
Each of these paths sets iod->status and calls
nvmet_pci_epf_complete_iod(), which then reloads iod->status from
iod->cqe.status. The nvmet core only fills that field when it completes
the request, which these paths never do, so it is still zero and
nvmet_pci_epf_cq_work() posts a success status to the host.
Drop the reload. Every path already sets iod->status before completion:
nvmet_pci_epf_queue_response() copies it from the request's CQE, and
the transport error paths set it themselves.
Found by code inspection.
Fixes: 0faa0fe6f90e ("nvmet: New NVMe PCI endpoint function target driver")
Cc: stable@vger.kernel.org
Tested-by: Niklas Cassel <cassel@kernel.org>
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
Zone Management Receive computes its buffer size as (numd + 1) << 2 in
u32, in nvmet_bdev_validate_zone_mgmt_recv(),
nvmet_bdev_zone_zmgmt_recv_work() and
nvmet_bdev_execute_zone_mgmt_recv(). NUMD is a 32-bit dword count, so
any NUMD from 0x3fffffff up wraps.
The data length check in nvmet_bdev_execute_zone_mgmt_recv() sees the
same wrapped value, so such a command can pass it. A data length that
does not match the wrapped size is already rejected by that check, with
Data SGL Length Invalid.
Every site agrees on the wrapped size, so the target zeroes and fills
only that many bytes and nothing leaks, but it answers a command whose
NUMD asks for 4 GiB or more.
Reject a NUMD from U32_MAX / 4 up with Invalid Field at the start of
nvmet_bdev_execute_zone_mgmt_recv(), before this handler or its work
item computes the byte count. A size_t computation would still wrap on
32-bit kernels, where nvmet_check_transfer_len() and req->transfer_len
are 32 bits wide.
nvmet_io_cmd_transfer_len() in core.c, which the pci-epf transport uses
to size its buffer, computes the same expression before this handler
runs. Return 0 there for such a NUMD, so pci-epf allocates no buffer and
the handler then rejects the command.
Tested over the loop, tcp, rdma and fc transports with a zoned
null_blk namespace: a wrapping NUMD, sent with a data length that
matches the wrapped size, was accepted before this patch and now fails
with Invalid Field, while ordinary NUMD values still succeed. The core.c
change only affects pci-epf and is build-tested only.
Fixes: aaf2e048af27 ("nvmet: add ZBD over ZNS backend support")
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
When a read command completes with an error,
__nvmet_fc_fcp_nvme_cmd_done() still pushes the whole data buffer to
the host before the response. nvmet_fc_alloc_tgt_pgs() allocates that
buffer with sgl_alloc() and no __GFP_ZERO, and a command rejected by
nvmet_check_transfer_len() leaves it untouched, so the host receives
stale target memory.
The buffer is as large as the data length in the command IU, so the
amount of stale memory returned is up to the host.
The tcp and rdma targets send data only when cqe->status is zero (see
nvmet_tcp_need_data_out() and nvmet_rdma_need_data_out()), so no nvmet
command can rely on returning data with an error status. Do the same
for fc. The ERSP then reports a transferred length of fod->offset,
which is 0, matching what nvme_fc_fcpio_done() checks on the host.
Tested over nvme-fcloop: a failed read command no longer transfers
data and still completes with its error status, and successful reads
are unaffected.
Fixes: c53432030d86 ("nvme-fabrics: Add target support for FC transport")
Cc: stable@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_rdma_map_sgl_inline() bounds the host-supplied in-capsule SGL
offset with "off + len > inline_data_size", evaluated in 64-bit
arithmetic. off is taken straight from the host SGL descriptor
(le64_to_cpu(sgl->addr)) and len is promoted to 64-bit, so the sum
wraps: an off within len of 2^64 makes it small and the check passes.
off is then used as page_idx = off / PAGE_SIZE to index the fixed-size
rsp->cmd->inline_sg[] array (NVMET_RDMA_MAX_INLINE_SGE entries), so a
crafted offset yields an out-of-bounds scatterlist index that is written
through in nvmet_rdma_use_inline_sg().
The mapping runs while handling a received capsule, before the command
executes, so a remote RDMA peer can reach it (the Connect capsule is a
write and takes this path on a queue that has no controller yet). The
host controls only the low bits of the resulting displacement, so the
expected impact is an out-of-bounds write that faults the target
(remote denial of service).
This wrap was reported, with a KASAN reproduction, on linux-nvme in May,
and the check below was suggested in that thread. The fix that was
applied, 48c0162f647b, reworked how the offset is mapped but kept the
original check, and its page index turns a wrapped offset into an
out-of-bounds index into inline_sg[].
Reject the offset before it can wrap.
Found independently by static analysis with AI assistance, not
reproduced at runtime.
Fixes: 0d5ee2b2ab4f ("nvmet-rdma: support max(16KB, PAGE_SIZE) inline data")
Fixes: 48c0162f647b ("nvmet-rdma: handle inline data with a nonzero offset")
Reported-by: Bryam Vargas <hexlabsecurity@proton.me>
Closes: https://lore.kernel.org/all/LM21QIR-1-qJb7PViyJKCnGBnUzizeiNJVWQ3wb7ZwGezodjgKg3f-iobqOyequ-sT1jFCKJImfqNO_BKU3KO80xFITnaI5GTV_GxLUNDDc=@proton.me/
Suggested-by: Keith Busch <kbusch@kernel.org>
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Andrew Stellman <astellman@stellman-greene.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_fc_ls_create_connection() looks up the association with
nvmet_fc_find_target_assoc(), which returns it with a reference held,
and drops that reference as soon as the target queue has been allocated.
iod->assoc is then dereferenced again by nvmet_fc_makeconnid() while
building the connection id.
The queue only borrows the pointer and takes no reference of its own.
Once the lookup reference is dropped the association is kept alive
solely by the reference taken when it was created, so a disconnect LS or
a remote port unregister running concurrently can free it before
nvmet_fc_makeconnid() reads it:
BUG: KASAN: slab-use-after-free in nvmet_fc_handle_ls_rqst_work (drivers/nvme/target/fc.c:193 drivers/nvme/target/fc.c:1841 drivers/nvme/target/fc.c:1995 drivers/nvme/target/fc.c:2021)
Read of size 8 at addr ffff888179f1b000 by task kworker/u9:2/1167
Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
Workqueue: nvmet-wq nvmet_fc_handle_ls_rqst_work
Call Trace:
<TASK>
dump_stack_lvl (lib/dump_stack.c:94 lib/dump_stack.c:120)
print_report (mm/kasan/report.c:378 mm/kasan/report.c:482)
kasan_report (mm/kasan/report.c:595)
nvmet_fc_handle_ls_rqst_work (drivers/nvme/target/fc.c:193 drivers/nvme/target/fc.c:1841 drivers/nvme/target/fc.c:1995 drivers/nvme/target/fc.c:2021)
process_scheduled_works (kernel/workqueue.c:3322 kernel/workqueue.c:3405)
worker_thread (kernel/workqueue.c:3486)
kthread (kernel/kthread.c:436)
ret_from_fork (archkernel/process.c:158)
ret_from_fork_asm (arch/x86/entry/entry_64.S:245)
</TASK>
Freed by task 1167 on cpu 0 at 405.998150s:
kasan_save_track (mm/kasan/common.c:57 mm/kasan/common.c:78)
kasan_save_free_info (mm/kasan/generic.c:584)
__kasan_slab_free (mm/kasan/common.c:253 mm/kasan/common.c:285)
kfree (include/linux/kasan.h:235 mm/slub.c:2705 mm/slub.c:6405 mm/slub.c:6720)
nvmet_fc_handle_ls_rqst_work (drivers/nvme/target/fc.c:1808 drivers/nvme/target/fc.c:1995 drivers/nvme/target/fc.c:2021)
process_scheduled_works (kernel/workqueue.c:3322 kernel/workqueue.c:3405)
worker_thread (kernel/workqueue.c:3486)
kthread (kernel/kthread.c:436)
ret_from_fork (archkernel/process.c:158)
ret_from_fork_asm (arch/x86/entry/entry_64.S:245)
The buggy address belongs to the object at ffff888179f1b000
which belongs to the cache kmalloc-2k of size 2048
The buggy address is located 0 bytes inside of
freed 2048-byte region [ffff888179f1b000, ffff888179f1b800)
Hold the lookup reference until the response has been built, and drop it
on the error path as well.
Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
Closes: https://lore.kernel.org/linux-nvme/20260731204504.2396434-1-shuangpeng.kernel@gmail.com
Fixes: c53432030d86 ("nvme-fabrics: Add target support for FC transport")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_rdma_parse_cm_connect_req() treats RDMA-CM private data as a
struct nvme_rdma_cm_req but accepts any nonzero private_data_len. The
iWARP path allocates exactly the peer-provided length, so a six-byte MPA
request reaches the parser and the hsqsize load reads two bytes past the
allocation.
Require the complete request structure before reading any of its fields.
The decoded KASAN report is:
BUG: KASAN: slab-out-of-bounds in nvmet_rdma_cm_handler
(drivers/nvme/target/rdma.c:1427 drivers/nvme/target/rdma.c:1475
drivers/nvme/target/rdma.c:1614 drivers/nvme/target/rdma.c:1814)
Read of size 2 at addr ff1100010459d366 by task kworker/u16:1/41
Workqueue: iw_cm_wq cm_work_handler
Call Trace:
<TASK>
dump_stack_lvl (lib/dump_stack.c:94 lib/dump_stack.c:120)
print_report (mm/kasan/report.c:378 mm/kasan/report.c:482)
kasan_report (mm/kasan/report.c:595)
nvmet_rdma_cm_handler
(drivers/nvme/target/rdma.c:1427 drivers/nvme/target/rdma.c:1475
drivers/nvme/target/rdma.c:1614 drivers/nvme/target/rdma.c:1814)
cma_cm_event_handler (drivers/infiniband/core/cma.c:2181)
iw_conn_req_handler (drivers/infiniband/core/cma.c:2640)
cm_work_handler
(drivers/infiniband/core/cm.c:2502
drivers/infiniband/core/cm.c:3858)
process_one_work (kernel/workqueue.c:3396)
worker_thread (kernel/workqueue.c:3479 kernel/workqueue.c:3560)
kthread (kernel/kthread.c:436)
ret_from_fork (arch/x86/kernel/process.c:158)
ret_from_fork_asm (arch/x86/entry/entry_64.S:245)
</TASK>
Allocated by task 12:
__kasan_kmalloc (mm/kasan/common.c:398 mm/kasan/common.c:415)
__kmalloc_node_track_caller_noprof
(mm/slub.c:5414 mm/slub.c:5545)
kmemdup_noprof (mm/util.c:138)
cm_event_handler
(drivers/infiniband/core/iwcm.c:194
drivers/infiniband/core/iwcm.c:1076)
siw_cm_upcall (drivers/infiniband/sw/siw/siw_cm.c:408)
siw_proc_mpareq (drivers/infiniband/sw/siw/siw_cm.c:750)
siw_cm_work_handler (drivers/infiniband/sw/siw/siw_cm.c:1086)
The buggy address is located 0 bytes to the right of
allocated 6-byte region [ff1100010459d360, ff1100010459d366)
Cc: stable@vger.kernel.org
Fixes: 8f000cac6e7a ("nvmet-rdma: add a NVMe over Fabrics RDMA target driver")
Reported-by: <co+e015e82956da641b@bugs.sh>
Assisted-by: LLM
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux
Pull block updates from Jens Axboe:
- NVMe updates via Keith:
- Apple M4 (t8132) ANS2 NVMe support
- nvme-multipath sysfs attributes for path_state, iopolicies, and
fail_on_ctrl_loss, plus path state evaluation fixes
- nvme-tcp parallelized I/O queue allocation and startup, and using
the caller's network namespace
- nvmet passthru UAF fix and rework, PR preempt use-after-free
fixes, auth hardening, and traffic based keep alive support
- DH-HMAC-CHAP secret attributes readable by root only
- Various other fixes and cleanups
- swim driver overhaul and fixes, plus floppy fixes, via a pull from
Denis
- Block device buffered and direct I/O rework from Tal: use
iomap_dirty_folio, take i_rwsem for the write fallback and splice
read, honor IOCB_NOWAIT in the buffered read path, and reject atomic
write fallback to buffered I/O
- Large zone revalidation rework: serialize revalidation, handle
capacity changes by dropping and retrying zone write plugs, propagate
read-only and offline conditions, and treat offline and read-only
zones as dead
- ublk robustness work around canceled FETCH rounds and going live over
canceled io commands, dropping the default I/O scheduler, and a batch
of selftest additions
- blk-iocost flush cost support and zone append accounting
- rust block fixes for Send/Sync bounds on GenDisk and TagSet, and
import style cleanups
- Atomic write fixes capping size by the PI buffer constraints and
failing short atomic pins
- Skip the redundant flush for O_DSYNC direct writes, and only use
REQ_FUA when the device supports it
- Store GPT attributes as a raw value
- Remove support for swapping loop fds
- Various other fixes and cleanups all over
* tag 'for-7.4/block-20261021' of git://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux: (144 commits)
block: restore the iterator when rejecting a misaligned bio
blk-mq: clear stale inactive flag on hctx reuse
block, bfq: initialize active lists for all actuators
zloop: print device that had errors
ublk: pin module while release work is pending
blk-mq: drop the tag_set argument from blk_mq_free_rq_map/tags
blk-mq: share one tags_srcu instance across all tag sets
blk-iocost: fix stale comment in ioc_rqos_throttle()
blk-iocost: account zone append completions in latency stats
blk-iocost: charge zone appends as writes
blk-iocost: add flush cost support with the flushiops model parameter
ublk: use global index for reported zones
selftests: ublk: add test for going live over canceled io commands
selftests: ublk: stop a batch thread when PREP_IO_CMDS fails
selftests: ublk: move the control command helpers into ctrl.c
ublk: refuse to go live after an io command was canceled
ublk: let STOP_DEV cancel the server's commands before its release
ublk: allow a NULL io->cmd in the cancel callback
ublk: reset the FETCH round under ub->mutex
ublk: reset the FETCH round in release also without a disk
...
|
|
nvmet_subsys_attr_qid_max_store()
nvmet_subsys_attr_qid_max_store() sets subsys->max_qid and then walks
subsys->ctrls to force every controller to reconnect, but it holds only
nvmet_config_sem (for write). subsys->ctrls is protected by subsys->lock:
nvmet_alloc_ctrl() adds an entry under subsys->lock, and nvmet_ctrl_free()
removes it under subsys->lock with list_del() + kfree() (not kfree_rcu).
The free path takes no nvmet_config_sem, so the two locks are independent.
A controller teardown (host disconnect, keep-alive timeout or fatal error
-> nvmet_sq_destroy() -> nvmet_ctrl_put() -> nvmet_ctrl_free()) can
therefore list_del() an entry while qid_max_store() is iterating the list,
corrupting the walk.
KASAN, concurrent "echo 128 > .../subsystems/<nqn>/attr_qid_max" racing
nvme-loop controllers connecting and disconnecting, panics in seconds:
BUG: KASAN: null-ptr-deref in nvmet_subsys_attr_qid_max_store+0x10d/0x1b0
Read of size 8 at addr 0000000000000090
Call Trace:
nvmet_subsys_attr_qid_max_store+0x10d/0x1b0
configfs_write_iter+0x1a7/0x250
vfs_write+0x5df/0x940
ksys_write+0xfd/0x1e0
(also seen as a general-protection fault dereferencing a LIST_POISON
pointer at the same instruction.)
Take subsys->lock around the list_for_each_entry() loop, matching every
other subsys->ctrls accessor. ->delete_ctrl() only queues asynchronous
controller teardown, so it does not re-enter subsys->lock under the loop.
Fixes: 3e980f5995e0 ("nvmet: expose max queues to configfs")
Cc: stable@vger.kernel.org
Reviewed-by: Maurizio Lombardi <mlombard@redhat.com>
Signed-off-by: Yu Zhang <yuz08559@gmail.com>
Tested-by: Yu Zhang <yuz08559@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
Cross-merge networking fixes after downstream PR (net-7.3-rc8).
No conflicts. Adjacent changes:
drivers/net/ethernet/microchip/lan865x/lan865x.c
2a3f6d2a9ac3 ("microchip: lan865x: move setting of netops to fix crash")
15358c2c3dca ("net: ethernet: oa_tc6: return ERR_PTR from oa_tc6_init()")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
A Zone Management Receive report returns the full buffer the host asked
for, but nvmet_bdev_zone_zmgmt_recv_work() writes only the 8-byte
Number of Zones field and one 64-byte descriptor for each matching zone
that fits. The 56 reserved header bytes and every descriptor slot past
the last match keep whatever the buffer held before. A 4096-byte report
covering a single zone leaves 4024 bytes unwritten.
The tcp, rdma and fc transports allocate that buffer with sgl_alloc()
and pci-epf with kmalloc(), none with __GFP_ZERO, and on success each
sends the whole buffer to the host (nvmet_setup_c2h_data_pdu() for
tcp), so the host receives old kernel memory from the target.
nvmet_bdev_execute_zone_mgmt_recv() also never checks the transport
data length against NUMD, so on transports where the host sets that
length, the buffer sent back can be much larger than the report NUMD
describes. pci-epf derives its data length from NUMD, so the length
check below does not apply there, but the zeroing still covers the
report.
Reject a data length that does not match NUMD with
nvmet_check_transfer_len(), as nvmet_bdev_execute_zone_append() does,
and zero the report with nvmet_zero_sgl() before filling it. Zeroing
NUMD bytes is only enough because of the length check, so the two go
together.
Allocating with __GFP_ZERO in the transports would also close the leak,
but it adds a memset to every read command to fix one handler, so the
transports are left alone.
Tested over the loop, tcp, rdma and fc transports with a zoned
null_blk namespace: before this patch, zone reports carried stale bytes
and a data length larger than NUMD was accepted; with it, reports are
fully initialized and such a length fails with Data SGL Length Invalid.
Fixes: aaf2e048af27 ("nvmet: add ZBD over ZNS backend support")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_execute_auth_send() allocates and copies the initiator-supplied
transfer length, then reads auth_type and auth_id through a common DHCHAP
message pointer. The existing zero and transfer-length checks do not
ensure that both common-header bytes are present.
An unauthenticated NVMe/TCP initiator can complete ICReq and admin CONNECT,
then send AUTH_SEND with tl equal to one. KASAN reports a one-byte slab
out-of-bounds read when auth_id is loaded.
Reject payloads that do not cover the common auth_type/auth_id header
before allocating or parsing them. The one-byte trigger and
header-minus-one boundary were cleanly rejected in three fixed fresh-boot
runs. Exact-header, valid DHCHAP, wrong-type, mismatch, and zero-length
controls remained clean, and a following valid AUTH_SEND on the same
connection received a response. The touched source is unchanged at
0c2669a9f4a1d607e7591ae50ccf3c432a0aff08, where the patch applies and
builds cleanly.
The demonstrated impact is unauthenticated pre-authentication target-kernel
denial of service. No disclosure, controlled write, RCE, or LPE was
demonstrated. The source reproducer and complete logs are available on
request and are not included in this public report.
OpenAI Codex assisted source inspection, test design, patch drafting, and
evidence organization.
Fixes: db1312dd9548 ("nvmet: implement basic In-Band Authentication")
Assisted-by: LLM
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Sung Byeongchan <tjdqudcks0424@naver.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
syzbot hit a recursive locking report on subsys->lock when a subsystem
is unlinked from an FC port.
nvmet_port_del_ctrls() holds subsys->lock while it calls ->delete_ctrl()
for each controller. nvmet_fc_delete_ctrl() grabs a reference on the
association, queues del_work and then drops the reference. If del_work
gets to run to completion before that put, ours is the last reference.
Freeing the assoc tears down the queues, the last nvmet_cq_put() drops
the last ctrl reference, and nvmet_ctrl_free() goes for subsys->lock
again:
WARNING: possible recursive locking detected
...
nvmet_ctrl_free+0xa6/0x8e0 drivers/nvme/target/core.c:1766
nvmet_cq_put+0x158/0x200 drivers/nvme/target/core.c:848
nvmet_fc_target_assoc_free+0x418/0x2220 drivers/nvme/target/fc.c:1173
nvmet_fc_delete_ctrl+0x717/0x7e0 drivers/nvme/target/fc.c:1581
nvmet_port_del_ctrls+0xfb/0x150 drivers/nvme/target/core.c:311
nvmet_port_subsys_drop_link+0x1e8/0x310 drivers/nvme/target/configfs.c:1119
It's a real hang, not only a lockdep warning. I reproduced it with
fcloop by putting an msleep() before the put: rm on the configfs link
gets stuck in D state for good.
I don't think we need that reference at all. del_work does
list_del_rcu() and synchronize_rcu() before it drops the initial
reference, so an assoc we find on assoc_list can't go away before
rcu_read_unlock(). nvmet_fc_schedule_delete_assoc() doesn't sleep, so
just call it inside the RCU section and get rid of the get/put. If the
assoc is already being deleted, terminating is set and the call is a
no-op, which matches what happened before when the get failed.
With the same msleep() in place the patched kernel unlinks cleanly and
the assoc is freed from the workqueue.
Fixes: 4049dc96b8de ("nvmet-fc: defer cleanup using RCU properly")
Reported-by: syzbot+da94a51633758d13bd63@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=da94a51633758d13bd63
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Palla Raghunath <raghunathpalla.0209@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
I/O queues do not perform independent DH-HMAC-CHAP authentication. They
instead rely on authentication of the controller's admin queue. Fabrics
property commands, however, are dispatched before the general admin command
authentication check.
An unauthenticated peer can consequently set CC.EN, connect an I/O queue,
and access a namespace after admin CONNECT reported that authentication was
required. Reject PROPERTY_SET until the admin queue has authenticated.
Targets without a configured authentication key are unchanged.
This was found by source review with AI assistance. A localhost NVMe/TCP
wire reproducer retrieved a known 4 KiB namespace block in two independent
boots without completing AUTH_SEND or AUTH_RECEIVE. With this change,
PROPERTY_SET returned NVME_SC_AUTH_REQUIRED and the namespace data was not
returned; the no-authentication control continued to work.
Fixes: c91a20129185 ("nvmet-auth: authenticate on admin queue only")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Sung Byeongchan <tjdqudcks0424@naver.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_file_execute_rw() allocates the bvec before
nvmet_file_execute_io() checks whether the request fits in the backing
file. If the request extends beyond the file, nvmet_file_execute_io()
calls nvmet_req_complete() to return an error. This skips
nvmet_file_io_done(), so the allocated memory is never freed. Repeated
requests can exhaust memory.
Use nvmet_file_io_done() to free the bvec before returning the range
error to the client.
Fixes: 50a909db36f2 ("nvmet: use IOCB_NOWAIT for file-ns buffered I/O")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
A REPLY AUTH_SEND command can derive a dhchap_skey key while a
concurrent AUTH_RECEIVE command observing this REPLY state executes
the failure path and frees that key. The REPLY command then continues
and writes into freed memory, causing a use-after-free. KASAN reports
it as:
BUG: KASAN: slab-use-after-free in __sha256_final
Write of size 4 at addr ffff88800359c540
...
nvmet_auth_sq_free (drivers/nvme/target/auth.c:237)
nvmet_execute_auth_receive (drivers/nvme/target/fabrics-cmd-auth.c:621)
Add a mutex to each submission queue to serialize authentication
commands and timeout handling. Unlock the mutex before completing
requests and ignore outdated timeout callbacks. Limit timeout delays
to prevent immediate expiry on 32-bit kernels.
Fixes: 7a277c37d352 ("nvmet-auth: Diffie-Hellman key exchange support")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
Unless the reservation is of an All Registrants type, nvmet_pr_preempt()
treats every Reservation Acquire command with the Preempt or Preempt and
Abort action that is sent by the reservation holder as the holder
preempting itself. It updates the reservation type to RTYPE and
completes the command successfully without looking at PRKEY.
NVMe Base Specification 2.1, section 8.1.22.7 makes the action depend
on PRKEY for these reservation types, no matter which registrant sends
the command:
- PRKEY matches the key of the reservation holder: the reservation is
preempted. This is how a holder preempts itself.
- PRKEY does not match the key of the holder and is not 0h: the
registrants whose key matches PRKEY are unregistered.
- PRKEY does not match the key of the holder and is 0h: the command is
aborted with Invalid Field in Command.
With the current code, a holder that preempts with the key of another
registrant leaves that registrant registered, changes the reservation
type to RTYPE and still gets a successful completion. With hosts A, B
and C registered with the keys 0xa, 0xb and 0xc, and A holding an
Exclusive Access - Registrants Only reservation, A sends:
# nvme resv-acquire /dev/nvme1 -n 1 --crkey=0xa --prkey=0xb \
--rtype=4 --racqa=1
NVME Reservation Acquire success
# nvme resv-report /dev/nvme1 -n 1 --eds | grep '^regctl '
regctl : 3
and B can still write to the namespace. Preempt and Abort (--racqa=2)
gives the same result. When C sends the preempt instead (--crkey=0xc),
B is unregistered and its writes fail with Reservation Conflict.
This defeats fencing in the pNFS SCSI layout server. nfsd holds the
reservation and fences a client that does not return its layout by
calling pr_preempt() with the key of that client and the abort flag,
which is the command RFC 9561 section 2.2.3 asks for. On an nvmet
namespace the preempt succeeds, nfsd logs the client as fenced, and the
client keeps writing to the device. In a test with an nfsd server and
two clients on an nvmet-tcp namespace, the namespace still had three
registrants after the fence and the fenced client went on writing to
it. With this patch two registrants are left and the writes of the
fenced client fail with a reservation conflict.
Take the self preempt path only when PRKEY is the key of the holder.
Any other PRKEY sent by the holder is then handled by the code that
already serves a non-holder registrant, for Preempt and for Preempt and
Abort. For a host that holds the reservation this changes the
following:
- A preempt with the key of another registrant unregisters the
registrants with that key, sends them the registration preempted
notification and leaves the reservation type alone.
- A preempt with PRKEY 0h fails with Invalid Field in Command. A
holder that changed its reservation type this way has to pass its
own key in PRKEY, which works with and without this patch.
- A preempt with a PRKEY that matches no registrant fails with
Reservation Conflict. The specification defines that status only
for the All Registrants types. It is what nvmet already returns to
a non-holder registrant in this case and what the SCSI target
returns for an unregistered key. nfsd treats it as a successful
fence.
Not changed: when the holder preempts itself, registrants that share
its key stay registered.
Fixes: 5a47c2080a73 ("nvmet: support reservation feature")
Cc: stable@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
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
®->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@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
Concurrently disabling a passthru controller while passthru I/Os are
in flight can potentially result in a use-after-free. Introduce a
percpu refcount, an atomic flag, and an nvmet request flag to protect
the passthru controller lifetime.
When enabling the passthru controller, initialize the percpu refcount
and set the enabled flag. Each passthru I/O acquires a reference before
entering the passthru hotpath and sets an nvmet request flag to record
that the reference is held. The reference is released when the I/O
completes.
When disabling the passthru controller, clear the enabled flag, kill
the percpu refcount, and wait for all in-flight I/Os to release their
references before releasing the passthru controller.
Disable operations are serialized by subsys->lock. The enabled flag
ensures that only the first disable operation proceeds and subsequent
attempts observe the flag as cleared and return.
Once disabling starts, new I/Os either fail to be routed to the
passthru path or fail to acquire a live reference, and therefore cannot
dereference the passthru controller and thus avoid use-after-free.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
Currently, passthru parameters are grouped under struct nvmet_subsys.
Since passthru can be configured through configfs and all passthru
parameters are exposed under a separate configfs subdirectory, group
these parameters under a new struct nvmet_passthru.
Grouping passthru-specific parameters in a separate structure makes
the code easier to maintain and reason about.
This also allows the passthru structure to be allocated on demand,
avoiding the memory overhead for subsystems that do not use passthru.
The structure is allocated when passthru configuration is needed and
its configfs attributes are accessed under subsys->lock. The passthru
configfs group is still created when the subsystem is allocated.
There are no functional changes intended.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
Add the ability to turn on and off tbkas (traffic based keep alive
support) via configfs. This is useful for testing and debugging.
Reviewed-by: Mohamed Khalfella <mkhalfella@purestorage.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jesse Taube <jtaubepe@redhat.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
Cross-merge networking fixes after downstream PR (net-7.3-rc7).
Conflicts:
include/net/strparser.h
0984ebc631792 ("strparser: make sure __strp_recv isn't running before tearing down the parser")
02fd0a2111374 ("net: strparser: removed the aborted bit and aborts counter")
https://lore.kernel.org/asYfmLsfy-SCxXjr@sirena.co.uk
Adjacent changes:
Documentation/networking/strparser.rst
5f193cdabf25 ("docs: networking: strparser: remove general mode")
0984ebc63179 ("strparser: make sure __strp_recv isn't running before tearing down the parser")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Use the new helpers instead of open coding the SOCK_NOSPACE
manipulation, so that TCP can later maintain a cheaper private
copy of this bit.
No functional change intended.
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
Link: https://patch.msgid.link/20260929071743.23624-8-edumazet@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
nvmet_req_alloc_p2pmem_sgls() assigns req->p2p_dev only after the data
and metadata SGL allocations succeed. If the data allocation succeeds
but the metadata allocation fails, out_free_sg passes req->p2p_dev to
pci_p2pmem_free_sgl() before that assignment has taken place.
Pass the local p2p_dev argument instead, so cleanup uses the same PCI
device as the successful data allocation.
Fixes: bcd9a0797d73 ("nvmet: fix freeing unallocated p2pmem")
Reported-by: Changyul Lee <lcy8047@gmail.com>
Reviewed-by: Maurizio Lombardi <mlombard@redhat.com>
Signed-off-by: Sang-Hoon Choi <csh0052@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
The Reservation Notification Mask feature (FID 82h) defines its own
status code for namespaces that do not support reservations:
"If a Set Features command or a Get Features command attempts to
access the Reservation Notification Mask on a namespace that does
not support reservations or is invalid, then that command is
aborted with status code of Invalid Field in Command."
-- NVMe Base Specification, section 5.2.26.1.33
The Set Features handler routes the namespace lookup through
nvmet_req_find_ns(), which returns Invalid Namespace or Format for an
unknown NSID. That is the correct status for the generic
namespace-scoped admin commands, but not for this feature: a Set
Features 82h naming a nonexistent NSID is aborted with Invalid
Namespace or Format instead of Invalid Field in Command.
Translate the lookup failure to Invalid Field in Command. The
broadcast NSID, the namespace-without-reservations case and everything
else keep their current behavior.
Fixes: 5a47c2080a73 ("nvmet: support reservation feature")
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_req_find_ns() uses percpu_ref_get() to obtain a reference to
the namespace. However, percpu_ref_get() can acquire a reference even
after the namespace reference has been killed (or marked DEAD).
This is undesirable during namespace disable because nvmet_ns_disable()
kills the namespace reference and then waits for all outstanding
references to drain. Acquiring a new reference after the reference is
killed can therefore extend the namespace drain period.
Replace percpu_ref_get() in nvmet_req_find_ns() with
percpu_ref_tryget_live_rcu(), which only acquires a reference while
the namespace reference is still live. This handles the race where
nvmet_req_find_ns() finds ns is admitting I/O (or it's live) but before
it acquires the reference to ns, its reference is killed in
nvmet_ns_disable(). For instance check this race:
CPU0 CPU1
nvmet_req_find_ns(): nvmet_ns_disable():
xa_load() -> ns
IO_LIVE == set
xa_clear_mark()
percpu_ref_kill() // DEAD
percpu_ref_get() synchronize_rcu()
| wait_for_completion()
+-- succeed
Replacing percpu_ref_get() with percpu_ref_tryget_live_rcu() prevents
the I/O request from acquiring a namespace reference once the
reference has been marked DEAD.
Perform the namespace lookup and reference acquisition in
nvmet_req_find_ns() within an RCU read-side critical section.
nvmet_ns_disable() uses synchronize_rcu() before draining and exiting
the namespace reference, ensuring that RCU readers which may be
acquiring the namespace reference have completed before the reference
is exited.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_ns_disable() currently clears ns->enabled before draining
in-flight I/O references. This allows namespace configuration to be
changed while existing I/O requests can still hold a reference to the
namespace.
This can race with configuration of namespace attributes such as the
device path, UUID, NGUID etc. These attributes can be accessed by I/O
requests without holding subsys->lock and must not be modified while
such requests are still using the namespace.
In nvmet_ns_disable(), keep ns->enabled set while existing namespace
references are being drained, so namespace configuration remains blocked
until all in-flight I/O has completed. Set ns->enabled to false only
after the namespace references have been drained and the namespace
device has been disabled.
Introduce the NVMET_NS_IO_LIVE flag, which is set after the namespace
is successfully enabled in nvmet_ns_enable(). When nvmet_ns_disable()
starts, clear NVMET_NS_IO_LIVE so that the I/O path stops admitting new
I/O once the flag is cleared. Using test_and_clear_bit() in
nvmet_ns_disable() also prevents concurrent callers from starting a
second disable operation.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
Commit 6202783184bf ("nvmet: Improve nvmet_alloc_ctrl() interface and
implementation") added a second uuid_copy() of args->hostid near the end
of nvmet_alloc_ctrl(), and commit 7b658153f1b8 ("nvmet: Remove duplicate
uuid_copy") removed the original copy that sat before
nvmet_ctrl_init_pr() instead of the new one.
Since then nvmet_ctrl_init_pr() snapshots ctrl->hostid into the
per-controller per-namespace reservation refs while the uuid_copy()
from the connect data runs later, after the controller is published.
The ctrl is allocated with kzalloc(), so every pc_ref created on this
path stores the nil UUID.
pc_ref->hostid has a single consumer: nvmet_pr_set_ctrl_to_abort()
matches it against the preempted registrant's hostid to kill and drain
the victim's in-flight I/O for Preempt and Abort. The match can never
hit with the nil UUID, so whenever a namespace with reservations
enabled exists before a host connects, which includes every reconnect,
Preempt and Abort silently degrades into a plain Preempt: the
preempting host sees success while the victim's in-flight I/O is still
in the air.
Copy the hostid where the rest of the connect data is consumed, before
the controller is published and before nvmet_ctrl_init_pr() takes its
snapshot.
Fixes: 7b658153f1b8 ("nvmet: Remove duplicate uuid_copy")
Cc: stable@vger.kernel.org
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
nvmet_auth_challenge() receives its output buffer as a void pointer.
sizeof(*d) therefore evaluates to one with GCC and undercounts the fixed
challenge header by 15 bytes. A short AUTH_RECEIVE buffer can pass the
check before the challenge is copied past the end of its allocation.
Use the typed challenge pointer when calculating the response size.
BUG: KASAN: slab-out-of-bounds in nvmet_execute_auth_receive (drivers/nvme/target/fabrics-cmd-auth.c:442 drivers/nvme/target/fabrics-cmd-auth.c:569)
Write of size 32 by task kworker/1:1H/64
Workqueue: nvmet_tcp_wq nvmet_tcp_io_work
Call Trace:
nvmet_execute_auth_receive (drivers/nvme/target/fabrics-cmd-auth.c:442 drivers/nvme/target/fabrics-cmd-auth.c:569)
nvmet_tcp_try_recv_pdu (drivers/nvme/target/tcp.c:1119 drivers/nvme/target/tcp.c:1243)
nvmet_tcp_io_work (drivers/nvme/target/tcp.c:1350 drivers/nvme/target/tcp.c:1382 drivers/nvme/target/tcp.c:1445)
process_one_work (kernel/workqueue.c:3322)
worker_thread (kernel/workqueue.c:3405 kernel/workqueue.c:3486)
kthread (kernel/kthread.c:436)
ret_from_fork (arch/x86/kernel/process.c:158)
ret_from_fork_asm (arch/x86/entry/entry_64.S:245)
The buggy address is located 16 bytes inside of
allocated 33-byte region [ffff888000554e80, ffff888000554ea1)
Kernel panic - not syncing: KASAN: panic_on_warn set ...
Cc: stable@vger.kernel.org
Fixes: db1312dd9548 ("nvmet: implement basic In-Band Authentication")
Reported-by: co+77553a7fc66ac133@bugs.sh
Closes: https://lore.kernel.org/all/Ah6QavQXwvshtqyUcYD1P1R7XE0ND9fB7lbV@bugs.sh/
Assisted-by: Codex:gpt-5.6
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
|
The host controls an SGL segment's length. A nonzero length smaller than
one SGL descriptor makes nr_descs zero, so
nvmet_pci_epf_get_sgl_segment() reads the type byte of sgls[-1] outside
the allocated buffer. If that byte contains a segment descriptor type,
the function also copies the preceding 16 bytes and returns a negative
descriptor count.
The read was reproduced in 3/3 KASAN boots with only the host transfer
mocked by a KUnit test:
BUG: KASAN: slab-out-of-bounds in nvmet_pci_epf_get_sgl_segment
Read of size 1 by task kunit_try_catch
Call Trace:
nvmet_pci_epf_get_sgl_segment
nvmet_pci_epf_short_sgl_test
Kernel panic - not syncing: KASAN: panic_on_warn set ...
Reject segments that cannot hold one descriptor before allocating or
transferring their contents.
Fixes: 0faa0fe6f90e ("nvmet: New NVMe PCI endpoint function target driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
|
