From 3d1b041d41ebbd7a1e93ec7c2966c1499554c0c5 Mon Sep 17 00:00:00 2001 From: Michael Bommarito Date: Mon, 22 Jun 2026 08:48:23 -0400 Subject: [PATCH] um: ubd: validate COW header fields before use read_cow_header() copies the backing-file path and computes geometry from header fields without validating them, and the historical "XXX Need to sanity-check the values read from the header" comment was never addressed. Three image-controlled fields are unsafe: - backing_file[]: not guaranteed NUL-terminated. cow_strdup() and the later strlen()/printf() walk off the end of the header buffer when a crafted image leaves the array unterminated (heap over-read). - sectorsize: used as a divisor in cow_sizes() (bitmap_len = (size + sectorsize - 1) / (8 * sectorsize)); a zero or negative sectorsize causes a divide-by-zero / bogus geometry. - For the V3/V3_b layouts the existing "align == 0" check printed a warning but fell through and then computed ROUND_UP(sizeof(header), 0), another divide-by-zero. Reject an unterminated backing_file for each header version, turn the align == 0 warnings into hard errors (goto out), and reject a non-positive sectorsize before the geometry math runs. A COW image is parsed from a file the guest opens (ubdN=cow,backing on the command line, or a cow file an unprivileged user can point ubd at), so a crafted header is attacker-reachable. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Michael Bommarito Reviewed-by: Richard Weinberger Link: https://patch.msgid.link/20260622124823.1695944-1-michael.bommarito@gmail.com Signed-off-by: Johannes Berg --- arch/um/drivers/cow_user.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/arch/um/drivers/cow_user.c b/arch/um/drivers/cow_user.c index dc1d1bcd85ec..bb9f05ac70cf 100644 --- a/arch/um/drivers/cow_user.c +++ b/arch/um/drivers/cow_user.c @@ -279,7 +279,7 @@ int file_reader(__u64 offset, char *buf, int len, void *arg) return pread(fd, buf, len, offset); } -/* XXX Need to sanity-check the values read from the header */ +/* Sanity checks are performed after reading each header version below. */ int read_cow_header(int (*reader)(__u64, char *, int, void *), void *arg, __u32 *version_out, char **backing_file_out, @@ -325,6 +325,12 @@ int read_cow_header(int (*reader)(__u64, char *, int, void *), void *arg, *sectorsize_out = header->v1.sectorsize; *bitmap_offset_out = sizeof(header->v1); *align_out = *sectorsize_out; + if (!memchr(header->v1.backing_file, '\0', + sizeof(header->v1.backing_file))) { + cow_printf("%s - V1 backing_file not terminated\n", + __func__); + goto out; + } file = header->v1.backing_file; } else if (version == 2) { @@ -338,6 +344,12 @@ int read_cow_header(int (*reader)(__u64, char *, int, void *), void *arg, *sectorsize_out = be32toh(header->v2.sectorsize); *bitmap_offset_out = sizeof(header->v2); *align_out = *sectorsize_out; + if (!memchr(header->v2.backing_file, '\0', + sizeof(header->v2.backing_file))) { + cow_printf("%s - V2 backing_file not terminated\n", + __func__); + goto out; + } file = header->v2.backing_file; } /* This is very subtle - see above at union cow_header definition */ @@ -354,8 +366,15 @@ int read_cow_header(int (*reader)(__u64, char *, int, void *), void *arg, if (*align_out == 0) { cow_printf("read_cow_header - invalid COW header, " "align == 0\n"); + goto out; } *bitmap_offset_out = ROUND_UP(sizeof(header->v3), *align_out); + if (!memchr(header->v3.backing_file, '\0', + sizeof(header->v3.backing_file))) { + cow_printf("%s - V3 backing_file not terminated\n", + __func__); + goto out; + } file = header->v3.backing_file; } else if (version == 3) { @@ -385,14 +404,27 @@ int read_cow_header(int (*reader)(__u64, char *, int, void *), void *arg, if (*align_out == 0) { cow_printf("read_cow_header - invalid COW header, " "align == 0\n"); + goto out; } *bitmap_offset_out = ROUND_UP(sizeof(header->v3_b), *align_out); + if (!memchr(header->v3_b.backing_file, '\0', + sizeof(header->v3_b.backing_file))) { + cow_printf("%s - V3 backing_file not terminated\n", + __func__); + goto out; + } file = header->v3_b.backing_file; } else { cow_printf("read_cow_header - invalid COW version\n"); goto out; } + + if (*sectorsize_out <= 0) { + cow_printf("%s - invalid sectorsize %d\n", __func__, + *sectorsize_out); + goto out; + } err = -ENOMEM; *backing_file_out = cow_strdup(file); if (*backing_file_out == NULL) {