From: Mark Syms Subject: Bound nr_segments by seg[] capacity and right-size buffer Two constants were in play for the number of segments in a request: - BLKIF_MAX_SEGMENTS_PER_REQUEST (11), the Xen ABI limit and the size of the ring descriptor's blkif_request.seg[] array; and - BLKIF_MAX_BUFFER_SEGMENTS_PER_REQUEST (32), a blktap-local value introduced when multi-page rings were enabled (dd6095c). Multi-page rings enlarge the ring (more request slots), not the number of segments per request, so sizing the per-request structures by 32 was both unnecessary and unsafe: - tapdisk_xenblkif_make_vbd_request() validated nr_segments against 32, but nr_segments is a guest-controlled uint8_t and msg.seg[] only holds 11 entries. A request with nr_segments in (11, 32] passed validation and then drove out-of-bounds reads of msg.seg[] in tapdisk_xenblkif_parse_request() and guest_copy2(), whose stale bytes were used as gref/first_sect/last_sect. - The per-request buffer and the iov[]/gref[] arrays were over-allocated to 32 pages/entries where only 11 are reachable (gcopy_segs[] was already 11). Bound nr_segments by BLKIF_MAX_SEGMENTS_PER_REQUEST and size the per-request buffer (TD_REQ_BUFFER_SIZE) and the iov[]/gref[] arrays by the same constant, so the validation bound, the segment arrays, and the ring descriptor's seg[] all agree. block-lcache.c inherits the corrected buffer size via TD_REQ_BUFFER_SIZE. Also derive the bufcache munmap size from TD_REQ_BUFFER_SIZE so the map and unmap sizes share one definition. The legacy blktap2 kernel-mmap macros in blktaplib.h (MMAP_PAGES / MMAP_VADDR) are a separate, kernel-shared layout and are intentionally left untouched. This is CVE-2026-79606, part of XSA-513. Signed-off-by: Mark Syms Co-Authored-By: Claude Opus 4.8 Reviewed-by: Tim Smith diff --git a/drivers/td-req.c b/drivers/td-req.c index 5b9b33f15c5d..f4679af11c39 100644 --- a/drivers/td-req.c +++ b/drivers/td-req.c @@ -121,7 +121,7 @@ td_xenblkif_bufcache_free(struct td_xenblkif * const blkif) while (blkif->n_reqs_bufcache_free > TD_REQS_BUFCACHE_MIN){ munmap(blkif->reqs_bufcache[--blkif->n_reqs_bufcache_free], - (size_t)BLKIF_MAX_BUFFER_SEGMENTS_PER_REQUEST << PAGE_SHIFT); + (size_t)TD_REQ_BUFFER_SIZE); } } @@ -786,11 +786,14 @@ tapdisk_xenblkif_make_vbd_request(struct td_xenblkif * const blkif, gettimeofday(&tapreq->ts, NULL); /* - * Check that the number of segments is sane. + * Check that the number of segments is sane. nr_segments is guest- + * controlled; the blkif protocol permits at most + * BLKIF_MAX_SEGMENTS_PER_REQUEST segments per request, which is how the + * ring descriptor and our per-request buffers are sized. */ if (unlikely((tapreq->msg.nr_segments == 0 && tapreq->msg.operation != BLKIF_OP_WRITE_BARRIER) || - tapreq->msg.nr_segments > BLKIF_MAX_BUFFER_SEGMENTS_PER_REQUEST)) { + tapreq->msg.nr_segments > BLKIF_MAX_SEGMENTS_PER_REQUEST)) { RING_ERR(blkif, "req %lu: bad number of segments in request (%d)\n", tapreq->msg.id, tapreq->msg.nr_segments); err = EINVAL; diff --git a/drivers/td-req.h b/drivers/td-req.h index dad40f294628..7279a935a6b5 100644 --- a/drivers/td-req.h +++ b/drivers/td-req.h @@ -38,7 +38,14 @@ #include #include "td-blkif.h" -#define TD_REQ_BUFFER_SIZE (BLKIF_MAX_BUFFER_SEGMENTS_PER_REQUEST << PAGE_SHIFT) +/* + * A ring request descriptor (blkif_request_t) carries at most + * BLKIF_MAX_SEGMENTS_PER_REQUEST segments, each mapping a single page, so the + * per-request data buffer and the vectorised segment arrays below only ever + * need that many entries. (This backend does not implement BLKIF_OP_INDIRECT, + * which is the only mechanism that would raise the per-request segment count.) + */ +#define TD_REQ_BUFFER_SIZE (BLKIF_MAX_SEGMENTS_PER_REQUEST << PAGE_SHIFT) /** * Representation of the intermediate request used to retrieve a request from @@ -80,9 +87,9 @@ struct td_xenblkif_req { /** * The scatter/gather list td_vbd_request_t.iov points to. */ - struct td_iovec iov[BLKIF_MAX_BUFFER_SEGMENTS_PER_REQUEST]; + struct td_iovec iov[BLKIF_MAX_SEGMENTS_PER_REQUEST]; - grant_ref_t gref[BLKIF_MAX_BUFFER_SEGMENTS_PER_REQUEST]; + grant_ref_t gref[BLKIF_MAX_SEGMENTS_PER_REQUEST]; int prot; struct gntdev_grant_copy_segment