A Bhyve guest can cause e82545_transmit() to follow a path in which iovb[] and thus *iov are not initialized, but the code executes memcpy(hdrp, iov->iov_base, now); The guest can do this by creating a transmit descriptor with the following content: buffer_addr = (anything) lower.data = 0x21f00000 = CMD_DEXT | TXD_MASK | CMD_EOP upper.data = 0x00000100 = POPTS_IXSM This causes the descriptor type to be 0x20f00000 (i.e. not TYP_L) and the data length to be zero. As a result of the zero length, iov->* are never assigned to. Because of the EOP and IXSM, ckinfo[0].ck* are set, causing hdrlen to be 2, which causes e82545_transmit() to execute the code to prepend a header, which causes the memcpy() shown above to execute. This sequence also depends on what's on the stack in iovb[] (i.e. *iov), but often when I run it, iov->iov_len is a huge number. Sometimes iov->iov_base is an illegal pointer, sometimes a valid pointer to somewhere.
I have written a patch that might fix this at https://reviews.freebsd.org/D36182 I have not written my own PoC however to verify it. Are you easily able to test this with your reproducer?
(In reply to John Baldwin from comment #1) Patch D36182 does indeed make the problem go away for me.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=fa46f3704b7618f9d9493c126df781faf59040a8 commit fa46f3704b7618f9d9493c126df781faf59040a8 Author: John Baldwin <jhb@FreeBSD.org> AuthorDate: 2022-08-17 17:01:16 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2022-08-17 17:01:16 +0000 bhyve e1000: Skip packets with a small header. Certain operations such as checksum insertion and VLAN insertion require the device model to rewrite the packet header. The first step in rewriting the packet header is to copy the existing packet header from the source packet. This copy is done by copying data from an iovec array that corresponds to the S/G entries described by transmit descriptors. However, if the total packet length is smaller than the headers that need to be copied as the initial template, this copy can overflow the iovec array and use garbage values as the source pointer to memcpy. The PR used a single descriptor with a length of 0 in its PoC. To fix, track the total packet length and drop requests to transmit packets whose payload is smaller than the required header length. While here, fix another issue where the final descriptor could have an invalid length (too short) that could underflow 'len' when stripping the checksum. Skip those requests instead, too. PR: 264372 Reported by: Robert Morris <rtm@lcs.mit.edu> Reviewed by: grehan, markj MFC after: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D36182 usr.sbin/bhyve/pci_e82545.c | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=ff5d46d7f9a1042acdc0abf6a8f47e0d3fc9d446 commit ff5d46d7f9a1042acdc0abf6a8f47e0d3fc9d446 Author: John Baldwin <jhb@FreeBSD.org> AuthorDate: 2022-08-17 17:01:16 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2022-08-25 16:37:38 +0000 bhyve e1000: Skip packets with a small header. Certain operations such as checksum insertion and VLAN insertion require the device model to rewrite the packet header. The first step in rewriting the packet header is to copy the existing packet header from the source packet. This copy is done by copying data from an iovec array that corresponds to the S/G entries described by transmit descriptors. However, if the total packet length is smaller than the headers that need to be copied as the initial template, this copy can overflow the iovec array and use garbage values as the source pointer to memcpy. The PR used a single descriptor with a length of 0 in its PoC. To fix, track the total packet length and drop requests to transmit packets whose payload is smaller than the required header length. While here, fix another issue where the final descriptor could have an invalid length (too short) that could underflow 'len' when stripping the checksum. Skip those requests instead, too. PR: 264372 Reported by: Robert Morris <rtm@lcs.mit.edu> Reviewed by: grehan, markj MFC after: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D36182 (cherry picked from commit fa46f3704b7618f9d9493c126df781faf59040a8) usr.sbin/bhyve/pci_e82545.c | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-)
A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=2d349fd22c97e34e5cf4e8e2ac9454d062fb7a61 commit 2d349fd22c97e34e5cf4e8e2ac9454d062fb7a61 Author: John Baldwin <jhb@FreeBSD.org> AuthorDate: 2022-08-17 17:01:16 +0000 Commit: John Baldwin <jhb@FreeBSD.org> CommitDate: 2022-08-25 16:39:59 +0000 bhyve e1000: Skip packets with a small header. Certain operations such as checksum insertion and VLAN insertion require the device model to rewrite the packet header. The first step in rewriting the packet header is to copy the existing packet header from the source packet. This copy is done by copying data from an iovec array that corresponds to the S/G entries described by transmit descriptors. However, if the total packet length is smaller than the headers that need to be copied as the initial template, this copy can overflow the iovec array and use garbage values as the source pointer to memcpy. The PR used a single descriptor with a length of 0 in its PoC. To fix, track the total packet length and drop requests to transmit packets whose payload is smaller than the required header length. While here, fix another issue where the final descriptor could have an invalid length (too short) that could underflow 'len' when stripping the checksum. Skip those requests instead, too. PR: 264372 Reported by: Robert Morris <rtm@lcs.mit.edu> Reviewed by: grehan, markj MFC after: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D36182 (cherry picked from commit fa46f3704b7618f9d9493c126df781faf59040a8) usr.sbin/bhyve/pci_e82545.c | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-)