[PATCH 0/2] ubi: Check two values read from the media at attach time

[PATCH 0/2] ubi: Check two values read from the media at attach time

[Alejandro Pacheco]

Two values that UBI reads from the media when it attaches a device are
used as an index and as a length without being checked against the
device or the volume:

 - the PEB numbers of the free, used, scrub and erase lists of a
   fastmap, which ubi_wl_init() uses to index ubi->lookuptbl;

 - the data size of the last LEB of a static volume, which
   ubi_check_volume() reads into a buffer of vol->usable_leb_size bytes
   when the volume is first opened.

Patch 1 rejects a fastmap with an out-of-range PEB number, so that UBI
attaches by scanning instead; patch 2 makes check_av() reject a static
volume whose last LEB data size does not fit its usable LEB size. Both
need a crafted image: the fastmap and the VID headers carry CRCs.

Testing: UML (ARCH=um) with KASAN and KASAN_VMALLOC, a 64 MiB mtdram
device with 128 KiB eraseblocks, fastmap on, this base with and without
the series. Each boot formats the device, makes a dynamic volume and two
static volumes (the last LEB partly used and exactly full), writes them,
detaches, attaches from the fastmap UBI wrote and reads every volume
back; that passes with and without the series. It then attaches once
more with one value of the fastmap altered:

 - the first PEB number of the free list set to the PEB count (512):
   without the series KASAN reports a slab-out-of-bounds write in
   ubi_wl_init(); with it UBI logs "bad PEB 512 in fastmap free list",
   attaches by scanning, and the volumes read back;

 - the last LEB data size of the static volumes set to the LEB size
   + 1024: without the series the first read of the static volume
   fails the UBI assertions in ubi_eba_read_leb() and ubi_io_read() and
   does not return (the boot was stopped after 15 minutes); with it the
   attach fails with "bad attaching information, error 6".

The altered values were set by a test-only change in
ubi_attach_fastmap(), right after it reads them from the fastmap, not by
a crafted image. Not tested: the used, scrub and erase lists, negative
values, patch 2 on the scan path, and 32-bit kernels. W=1 builds of
drivers/mtd/ubi/ (x86_64) show no warnings with or without the series.

Alejandro Pacheco (2):
  ubi: fastmap: Check the PEB numbers in the fastmap lists
  ubi: Check the data size of the last LEB of static volumes

 drivers/mtd/ubi/fastmap.c | 40 +++++++++++++++++++++++++++++----------
 drivers/mtd/ubi/vtbl.c    |  6 ++++++
 2 files changed, 36 insertions(+), 10 deletions(-)


base-commit: 734bef548a96e8774d6eef5dfa2026c6edc4e80b

[PATCH 1/2] ubi: fastmap: Check the PEB numbers in the fastmap lists

[Alejandro Pacheco]

From: Alejandro Pacheco <ale@alepacheco.dev>

ubi_attach_fastmap() reads the free, used, scrub and erase lists of the
fastmap and adds every PEB number in them to the attach information as
it is on the media. ubi_wl_init() later uses these PEB numbers as
indexes into ubi->lookuptbl, which has ubi->peb_count entries, and the
PEBs of the used and scrub lists become the PEBs of the volumes' LEBs.
None of them is checked against the device, so a fastmap listing a PEB
number that is negative or not below ubi->peb_count makes ubi_wl_init()
write outside ubi->lookuptbl.

The PEB numbers of the two pools are not affected: scan_pool() reads
every pool PEB from the flash before using it, and the MTD layer refuses
an offset outside the device. A PEB number of an EBA table is only
accepted when it is found in the used list.

Check every PEB number of the four lists against ubi->peb_count, next to
the other sanity checks of the fastmap, and treat a fastmap with a bad
one as unusable, so that UBI attaches by scanning instead. Fastmaps
written by UBI are not affected: ubi_write_fastmap() only lists PEBs of
the device.

This needs a crafted fastmap (its data CRC catches random corruption)
on a device that UBI attaches at boot or that a user with
CAP_SYS_RESOURCE attaches.

An LLM coding assistant was used to find this bug and to write this
patch and its changelog.

Fixes: dbb7d2a88d2a ("UBI: Add fastmap core")
Cc: stable@vger.kernel.org
Signed-off-by: Alejandro Pacheco <ale@alepacheco.dev>
---
 drivers/mtd/ubi/fastmap.c | 40 +++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/ubi/fastmap.c b/drivers/mtd/ubi/fastmap.c
index 3f9fb7c181f..41ead6b4f0a 100644
--- a/drivers/mtd/ubi/fastmap.c
+++ b/drivers/mtd/ubi/fastmap.c
@@ -558,7 +558,7 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
 	struct ubi_fm_ec *fmec;
 	struct ubi_fm_volhdr *fmvhdr;
 	struct ubi_fm_eba *fm_eba;
-	int ret, i, j, pool_size, wl_pool_size;
+	int ret, i, j, pnum, pool_size, wl_pool_size;
 	size_t fm_pos = 0, fm_size = ubi->fm_size;
 	unsigned long long max_sqnum = 0;
 	void *fm_raw = ubi->fm_buf;
@@ -640,8 +640,13 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
 		if (fm_pos >= fm_size)
 			goto fail_bad;
 
-		ret = add_aeb(ai, &ai->free, be32_to_cpu(fmec->pnum),
-			      be32_to_cpu(fmec->ec), 0);
+		pnum = be32_to_cpu(fmec->pnum);
+		if (pnum < 0 || pnum >= ubi->peb_count) {
+			ubi_err(ubi, "bad PEB %i in fastmap free list", pnum);
+			goto fail_bad;
+		}
+
+		ret = add_aeb(ai, &ai->free, pnum, be32_to_cpu(fmec->ec), 0);
 		if (ret)
 			goto fail;
 	}
@@ -653,8 +658,13 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
 		if (fm_pos >= fm_size)
 			goto fail_bad;
 
-		ret = add_aeb(ai, &used, be32_to_cpu(fmec->pnum),
-			      be32_to_cpu(fmec->ec), 0);
+		pnum = be32_to_cpu(fmec->pnum);
+		if (pnum < 0 || pnum >= ubi->peb_count) {
+			ubi_err(ubi, "bad PEB %i in fastmap used list", pnum);
+			goto fail_bad;
+		}
+
+		ret = add_aeb(ai, &used, pnum, be32_to_cpu(fmec->ec), 0);
 		if (ret)
 			goto fail;
 	}
@@ -666,8 +676,13 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
 		if (fm_pos >= fm_size)
 			goto fail_bad;
 
-		ret = add_aeb(ai, &used, be32_to_cpu(fmec->pnum),
-			      be32_to_cpu(fmec->ec), 1);
+		pnum = be32_to_cpu(fmec->pnum);
+		if (pnum < 0 || pnum >= ubi->peb_count) {
+			ubi_err(ubi, "bad PEB %i in fastmap scrub list", pnum);
+			goto fail_bad;
+		}
+
+		ret = add_aeb(ai, &used, pnum, be32_to_cpu(fmec->ec), 1);
 		if (ret)
 			goto fail;
 	}
@@ -679,8 +694,13 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
 		if (fm_pos >= fm_size)
 			goto fail_bad;
 
-		ret = add_aeb(ai, &ai->erase, be32_to_cpu(fmec->pnum),
-			      be32_to_cpu(fmec->ec), 1);
+		pnum = be32_to_cpu(fmec->pnum);
+		if (pnum < 0 || pnum >= ubi->peb_count) {
+			ubi_err(ubi, "bad PEB %i in fastmap erase list", pnum);
+			goto fail_bad;
+		}
+
+		ret = add_aeb(ai, &ai->erase, pnum, be32_to_cpu(fmec->ec), 1);
 		if (ret)
 			goto fail;
 	}
@@ -732,7 +752,7 @@ static int ubi_attach_fastmap(struct ubi_device *ubi,
 		}
 
 		for (j = 0; j < be32_to_cpu(fm_eba->reserved_pebs); j++) {
-			int pnum = be32_to_cpu(fm_eba->pnum[j]);
+			pnum = be32_to_cpu(fm_eba->pnum[j]);
 
 			if (pnum < 0)
 				continue;

[PATCH 2/2] ubi: Check the data size of the last LEB of static volumes

[Alejandro Pacheco]

From: Alejandro Pacheco <ale@alepacheco.dev>

For a static volume, init_volumes() takes the data size of its last LEB
from the attach information, av->last_data_size, as vol->last_eb_bytes.
When the volume is first opened, ubi_check_volume() reads that many
bytes of the last LEB into a buffer of vol->usable_leb_size bytes.

Nothing bounds that size by vol->usable_leb_size. When UBI attaches by
scanning, it is the data_size of the last LEB's VID header, which
validate_vid_hdr() only checks against ubi->leb_size; that is larger
than the usable LEB size of a volume with a non-zero data_pad. When UBI
attaches from a fastmap, it can be the last_eb_bytes of the volume's
fastmap header, which is not checked at all. A larger size makes
ubi_check_volume() write past the end of its buffer.

Before UBI comes up, check_attaching_info() runs check_av() on every
volume that has attach information, to check it against the volume's
record in the volume table. Also reject there a static volume whose last
LEB data size is negative or larger than vol->usable_leb_size. Images
written by UBI or ubinize are not affected: no LEB of a static volume
holds more data than its usable LEB size.

This needs a crafted image (the VID header and the fastmap carry CRCs
that catch random corruption) on a device that UBI attaches at boot or
that a user with CAP_SYS_RESOURCE attaches.

An LLM coding assistant was used to find this bug and to write this
patch and its changelog.

Fixes: 801c135ce73d ("UBI: Unsorted Block Images")
Cc: stable@vger.kernel.org
Signed-off-by: Alejandro Pacheco <ale@alepacheco.dev>
---
 drivers/mtd/ubi/vtbl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/mtd/ubi/vtbl.c b/drivers/mtd/ubi/vtbl.c
index 06d87a3013a..f1bd6e1e488 100644
--- a/drivers/mtd/ubi/vtbl.c
+++ b/drivers/mtd/ubi/vtbl.c
@@ -696,6 +696,12 @@ static int check_av(const struct ubi_volume *vol,
 		err = 5;
 		goto bad;
 	}
+	if (vol->vol_type == UBI_STATIC_VOLUME &&
+	    (av->last_data_size < 0 ||
+	     av->last_data_size > vol->usable_leb_size)) {
+		err = 6;
+		goto bad;
+	}
 	return 0;
 
 bad: