io: bounds-check the LBA range and validate PRPs in __do_perform_io#75
Open
jongukc wants to merge 1 commit into
Open
io: bounds-check the LBA range and validate PRPs in __do_perform_io#75jongukc wants to merge 1 commit into
jongukc wants to merge 1 commit into
Conversation
__do_perform_io() builds the backing-store address directly from the
host-supplied SLBA / transfer length and from the PRP entries, with no
validation:
offset = (cmd->slba) << LBA_BITS; // host SLBA
length = (cmd->length + 1) << LBA_BITS; // host transfer length
...
memcpy(nvmev_vdev->ns[nsid].mapped + offset, vaddr + mem_offs, io_size);
A normal block-layer read/write is bounded by the disk capacity, so this
never fires in ordinary use. But a passthrough I/O command lets user space
set slba/nblocks (and nsid) directly, and a malformed/forged SQ entry can
do the same. Two memory-safety bugs result:
1. A large SLBA/length makes ns->mapped + offset point outside the
mapped backing store -> wild read/write (GPF, or silent corruption
of unrelated kernel memory):
KASAN: wild-memory-access Read in nvmev_io_worker
general protection fault in nvmev_io_worker
2. A PRP whose physical address is not mapped makes memremap() return
NULL (after a WARNING in __ioremap_caller); the following memcpy then
dereferences NULL:
WARNING in __ioremap_caller
KASAN: null-ptr-deref Write in nvmev_io_worker
Fix both:
- Reject an I/O whose [offset, offset+length) range falls outside the
namespace size (and whose nsid is out of range) before any transfer.
- NULL-check every resolved PRP -- both the PRP-list page and each data
page -- and abort the transfer instead of dereferencing NULL.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Summary
__do_perform_io()builds the backing-store address from host command fields without checking them.It takes the namespace index from
cmd->nsid, the offset fromcmd->slba, and the length fromcmd->length.It then runs a
memcpyto or fromnvmev_vdev->ns[nsid].mapped + offset.A normal block read/write is bounded by the disk size, so this is safe in normal use.
But a passthrough I/O command lets user space set
slba,nblocks, andnsiddirectly.A malformed or forged SQ entry can do the same.
This leads to two memory bugs:
slbamakesns->mapped + offsetpoint outside the backing store.memcpyis then a wild read or write.0also makesnsid - 1a huge index intonvmev_vdev->ns[].memremap()returns NULL.memcpythen dereferences NULL.Description
With KASAN enabled, the out-of-bounds access faults in the I/O worker.
This was reproduced on unpatched Linux 7.1.0 by reading far past the namespace size.
The namespace is 1023 MiB, and the read starts at LBA 2^30 (offset 512 GiB).
The bad-PRP case instead shows
KASAN: null-ptr-deref Write in nvmev_io_worker.This bug was found by Syzkaller during fuzzing.
Linux 7.1.0 x86_64, CONFIG_KASAN=y, panic_on_warn=1,
kasan.fault=panic, running under QEMU/KVM.Reproduction
On the test machine, with NVMeVirt probed as
/dev/nvme0n1.Pick a start block well past the namespace size.
See io-range-repro.sh:
For the bad-PRP case, send an I/O passthrough whose transfer length is larger than the data buffer.
bug-C-repro.shhas an example.Fix
In
__do_perform_io():[offset, offset+length)range is outside the namespace size (and whosensidis out of range) before any transfer.A fully spec-faithful version would also return
NVME_SC_LBA_RANGEto the host.This patch focuses on the memory-safety hole.
Validated on Linux 7.1.0.