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 <michael.bommarito@gmail.com>
Reviewed-by: Richard Weinberger <richard@nod.at>
Link: https://patch.msgid.link/20260622124823.1695944-1-michael.bommarito@gmail.com
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
This commit is contained in:
Michael Bommarito 2026-06-22 08:48:23 -04:00 committed by Johannes Berg
parent e6d11b4286
commit 3d1b041d41

View File

@ -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) {