https://bugs.dpdk.org/show_bug.cgi?id=2012
Bug ID: 2012
Summary: memif: add_region accepts claimed region larger than
file
Product: DPDK
Version: 26.11
Hardware: All
OS: All
Status: UNCONFIRMED
Severity: normal
Priority: Normal
Component: ethdev
Assignee: [email protected]
Reporter: [email protected]
Target Milestone: ---
Group: security
Another AI discovered bug.
memif_msg_receive_add_region() in drivers/net/memif/memif_socket.c
validates the region index but takes the region size from the client
unchecked. It does not require ar->size to be non-zero, and it does not
confirm that the file descriptor passed with the message refers to a
file at least that large.
mmap() beyond the end of a file succeeds; the fault happens on access.
So a client that advertises a region larger than the file it actually
passes gets a mapping the server believes is fully backed, and any
access past the real end of the file faults the server with SIGBUS.
This also undermines the fix for the data-path descriptor validation
filed separately. That fix bounds each descriptor with
offset + length <= region_size, but region_size is the value the client
claimed. A descriptor that satisfies the bound against the claimed size
and lies past the real file size passes validation and still faults.
Fixing the data path without fixing this leaves the check looking sound
while resting on a peer-supplied number.
Suggested fix
-------------
Reject ar->size == 0. fstat() the passed file descriptor and reject the
region if st_size is less than the claimed size.
Caveat that should be recorded rather than papered over: an fstat check
does not fully close this. A client can truncate the file after the
check, or after the mapping is established, and the access still faults.
Only F_SEAL_SHRINK prevents that, and sealing is not available on all fd
types in legitimate use. libmemif seals region 0 with F_SEAL_SHRINK, but
hugepage-backed and zero-copy buffer regions use MFD_HUGETLB or
hugetlbfs and are not sealed, so a server that required sealing would
disconnect conforming clients, including VPP zero-copy clients.
The proposed fix therefore checks the size with fstat and only logs when
the fd is not sealed. Whether the residual post-check shrink exposure is
accepted, or handled some other way, is an open design question that
spans DPDK, VPP and libmemif and should be settled jointly rather than
decided unilaterally in one stack.
A candidate patch exists privately and has not been posted. It can be
shared with the security team on request.
Reported by Arthur Chan <[email protected]> (Ada Logics), via
fuzzing.
--
You are receiving this mail because:
You are the assignee for the bug.
