Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] net: vector: integer underflow in pskb_trim leads to info leak - [High] um: vector: OOB read and packet corruption in vector_legacy_rx --
commit 7f2cd5c3a1fb41cfaea6c03500daa85a7433a386 Author: Michael S. Tsirkin <[email protected]> um: vector: trim skbs before verifying the header This commit modifies the vector receive path to trim the SKB before verifying the transport header. This is done to prepare for computing the packet checksum in software, which requires the packet to be trimmed correctly. > diff --git a/arch/um/drivers/vector_kern.c b/arch/um/drivers/vector_kern.c > index 8a70b3a625537..1afbef78df725 100644 > --- a/arch/um/drivers/vector_kern.c > +++ b/arch/um/drivers/vector_kern.c > @@ -865,6 +865,7 @@ static int vector_legacy_rx(struct vector_private *vp) > > if (skb != NULL) { > if (pkt_len > vp->header_size) { > + pskb_trim(skb, pkt_len - vp->rx_header_size); [Severity: High] Can this subtraction underflow if pkt_len is smaller than vp->rx_header_size? For GRE and L2TPv3 over IPv4, vp->rx_header_size (e.g., 24) is strictly greater than vp->header_size (e.g., 4). If a packet is received with a length between these two values (e.g., 20 bytes), the check pkt_len > vp->header_size passes, but pkt_len - vp->rx_header_size underflows to a very large unsigned value. Because the length would exceed the pre-allocated skb->len, pskb_trim() fails silently. The SKB retains its maximum allocated size containing uninitialized memory. If vp->verify_header() randomly succeeds on the undersized IOV buffer, could this leak uninitialized kernel memory up the network stack via napi_gro_receive()? [Severity: High] Does this sequence cause an out-of-bounds read and inner packet corruption? Earlier in vector_legacy_rx(), iov[0].iov_len is restricted to vp->header_size: if (vp->header_size > 0) { iov[0].iov_base = vp->header_rxbuffer; iov[0].iov_len = vp->header_size; } For raw IPv4 GRE/L2TPv3 tunnels where vp->header_size (4) differs from vp->rx_header_size (24), recvmsg() reads only the first 4 bytes of the IP header into vp->header_rxbuffer. The remaining IP header, GRE header, and inner packet spill into iov[1] (skb->data). When gre_verify_header() runs, it advances its pointer by sizeof(struct iphdr) and reads uninitialized memory from the 4-byte header_rxbuffer to verify the header. Additionally, because the headers spilled into the SKB, does trimming by the full vp->rx_header_size here incorrectly truncate the payload from the tail instead of the head, silently destroying the inner packet before it reaches eth_type_trans()? [ ... ] > @@ -981,6 +981,8 @@ static int vector_mmsg_rx(struct vector_private *vp, int > budget) > for (i = 0; i < packet_count; i++) { > skb = (*skbuff_vector); > if (mmsg_vector->msg_len > vp->header_size) { > + pskb_trim(skb, > + mmsg_vector->msg_len - vp->rx_header_size); [Severity: High] Like the vector_legacy_rx() path, does this subtraction also underflow if msg_len is between vp->header_size and vp->rx_header_size? > if (vp->header_size > 0) { > header_check = vp->verify_header( > mmsg_vector->msg_hdr.msg_iov->iov_base, -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
