memif: add_region accepts claimed region larger than file

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.