1.Improve handling around last compressed block of the file, which is

  necessary because CLOOP format lacks explicit EOF or length, so that
  in the presence of padding or when the CLOOP is put onto a larger
  partition upper level provider size may be larger. Bound amount
  of extra data that we might touch to the max length of the compressed
  block and detect zero-padding in the last cluster, which when
  sector is all-zero might cause us to emit bogus I/O error after
  decompression of that fails. To not make code any more complicated
  that it needs to be deal with it in lazy-manner, i.e. when we
  first access that specific cluster.

  This change also fixes stupid mistake in the LZMA code, inherited
  from geom_lzma, which does not share length of the output buffer
  buffer with the decompression routine, so that in the presence
  of corrupted or purposedly tailored data may easily cause heap
  overflow and kernel memory corruption.

  Beef up validation of the CLOOP TOC by checking that lengths of
  all but the last compressed clusters match upper limit set by
  the decompressor and improve some error diagnostic output while
  I am here.

2.Add kern.geom.uzip.attach_to tunable to artifically limit
  attaching uzip to certain devices in the dev tree only.

    For example the following only makes us attaching to the
    GPT labels:

    kern.geom.uzip.attach_to="gpt/*"

3.Add kern.geom.uzip.noattach_to, which does opposite to the (2)
  above, i.e. prevents geom_uzip from tasting / attaching to
  providers matching some pattern. By default we don't attach
  to our own kind, i.e. kern.geom.uzip.noattach_to="*.uzip".
  It saves us quite some CPU cycles, esp on low-end embedded
  systems.

Approved by:	re (gjb)
Differential Revision:	https://reviews.freebsd.org/D7013

4 files changed, 117 insertions, 21 deletions

diff --git a/sys/geom/uzip/g_uzip.c b/sys/geom/uzip/g_uzip.c
index bdbf723..34f061c 100644
--- a/sys/geom/uzip/g_uzip.c
+++ b/sys/geom/uzip/g_uzip.c
@@ -60,6 +60,8 @@ FEATURE(geom_uzip, "GEOM read-only compressed disks support");
 struct g_uzip_blk {
         uint64_t offset;
         uint32_t blen;
+        unsigned char last:1;
+        unsigned char padded:1;
 #define BLEN_UNDEF      UINT32_MAX
 };
 
@@ -84,6 +86,16 @@ struct g_uzip_blk {
 #define	GUZ_DBG_IO	3
 #define	GUZ_DBG_TOC	4
 
+#define	GUZ_DEV_SUFX	".uzip"
+#define	GUZ_DEV_NAME(p)	(p GUZ_DEV_SUFX)
+
+static char g_uzip_attach_to[MAXPATHLEN] = {"*"};
+static char g_uzip_noattach_to[MAXPATHLEN] = {GUZ_DEV_NAME("*")};
+TUNABLE_STR("kern.geom.uzip.attach_to", g_uzip_attach_to,
+    sizeof(g_uzip_attach_to));
+TUNABLE_STR("kern.geom.uzip.noattach_to", g_uzip_noattach_to,
+    sizeof(g_uzip_noattach_to));
+
 SYSCTL_DECL(_kern_geom);
 SYSCTL_NODE(_kern_geom, OID_AUTO, uzip, CTLFLAG_RW, 0, "GEOM_UZIP stuff");
 static u_int g_uzip_debug = GEOM_UZIP_DBG_DEFAULT;
@@ -258,8 +270,9 @@ g_uzip_request(struct g_geom *gp, struct bio *bp)
 	}
 
 	DPRINTF_BRNG(GUZ_DBG_IO, start_blk, end_blk, ("%s/%s: %p: "
-	    "start=%u (%ju), end=%u (%ju)\n", __func__, gp->name, bp,
+	    "start=%u (%ju[%jd]), end=%u (%ju)\n", __func__, gp->name, bp,
 	    (u_int)start_blk, (uintmax_t)sc->toc[start_blk].offset,
+	    (intmax_t)sc->toc[start_blk].blen,
 	    (u_int)end_blk, (uintmax_t)BLK_ENDS(sc, end_blk - 1)));
 
 	bp2 = g_clone_bio(bp);
@@ -272,16 +285,18 @@ g_uzip_request(struct g_geom *gp, struct bio *bp)
 	bp2->bio_offset = TOFF_2_BOFF(sc, pp, start_blk);
 	while (1) {
 		bp2->bio_length = TLEN_2_BLEN(sc, pp, bp2, end_blk - 1);
-		if (bp2->bio_length <= MAXPHYS)
+		if (bp2->bio_length <= MAXPHYS) {
 			break;
+		}
 		if (end_blk == (start_blk + 1)) {
 			break;
 		}
 		end_blk--;
 	}
 
-	DPRINTF(GUZ_DBG_IO, ("%s/%s: bp2->bio_length = %jd\n",
-	    __func__, gp->name, (intmax_t)bp2->bio_length));
+	DPRINTF(GUZ_DBG_IO, ("%s/%s: bp2->bio_length = %jd, "
+	    "bp2->bio_offset = %jd\n", __func__, gp->name,
+	    (intmax_t)bp2->bio_length, (intmax_t)bp2->bio_offset));
 
 	bp2->bio_data = malloc(bp2->bio_length, M_GEOM_UZIP, M_NOWAIT);
 	if (bp2->bio_data == NULL) {
@@ -315,6 +330,15 @@ g_uzip_read_done(struct bio *bp)
 	wakeup(sc);
 }
 
+static int
+g_uzip_memvcmp(const void *memory, unsigned char val, size_t size)
+{
+	const u_char *mm;
+
+	mm = (const u_char *)memory;
+	return (*mm == val) && memcmp(mm, mm + 1, size - 1) == 0;
+}
+
 static void
 g_uzip_do(struct g_uzip_softc *sc, struct bio *bp)
 {
@@ -362,18 +386,33 @@ g_uzip_do(struct g_uzip_softc *sc, struct bio *bp)
 		    bp->bio_completed, data2, (u_int)ulen, data, (u_int)len));
 		if (len == 0) {
 			/* All zero block: no cache update */
+zero_block:
 			bzero(data2, ulen);
 		} else if (len <= bp->bio_completed) {
 			mtx_lock(&sc->last_mtx);
 			err = sc->dcp->decompress(sc->dcp, gp->name, data,
 			    len, sc->last_buf);
+			if (err != 0 && sc->toc[blk].last != 0) {
+				/*
+				 * Last block decompression has failed, check
+				 * if it's just zero padding.
+				 */
+				if (g_uzip_memvcmp(data, '\0', len) == 0) {
+					sc->toc[blk].blen = 0;
+					sc->last_blk = -1;
+					mtx_unlock(&sc->last_mtx);
+					len = 0;
+					goto zero_block;
+				}
+			}
 			if (err != 0) {
 				sc->last_blk = -1;
 				mtx_unlock(&sc->last_mtx);
 				bp2->bio_error = EILSEQ;
 				DPRINTF(GUZ_DBG_ERR, ("%s/%s: decompress"
-				    "(%p) failed\n", __func__, gp->name,
-				    sc->dcp));
+				    "(%p, %ju, %ju) failed\n", __func__,
+				    gp->name, sc->dcp, (uintmax_t)blk,
+				    (uintmax_t)len));
 				goto done;
 			}
 			sc->last_blk = blk;
@@ -471,6 +510,7 @@ g_uzip_spoiled(struct g_consumer *cp)
 {
 	struct g_geom *gp;
 
+	G_VALID_CONSUMER(cp);
 	gp = cp->geom;
 	g_trace(G_T_TOPOLOGY, "%s(%p/%s)", __func__, cp, gp->name);
 	g_topology_assert();
@@ -486,10 +526,12 @@ g_uzip_parse_toc(struct g_uzip_softc *sc, struct g_provider *pp,
 {
 	uint32_t i, j, backref_to;
 	uint64_t max_offset, min_offset;
+	struct g_uzip_blk *last_blk;
 
 	min_offset = sizeof(struct cloop_header) +
 	    (sc->nblocks + 1) * sizeof(uint64_t);
 	max_offset = sc->toc[0].offset - 1;
+	last_blk = &sc->toc[0];
 	for (i = 0; i < sc->nblocks; i++) {
 		/* First do some bounds checking */
 		if ((sc->toc[i].offset < min_offset) ||
@@ -497,7 +539,7 @@ g_uzip_parse_toc(struct g_uzip_softc *sc, struct g_provider *pp,
 			goto error_offset;
 		}
 		DPRINTF_BLK(GUZ_DBG_IO, i, ("%s: cluster #%u "
-		    "sc->toc[i].offset=%ju max_offset=%ju\n", gp->name,
+		    "offset=%ju max_offset=%ju\n", gp->name,
 		    (u_int)i, (uintmax_t)sc->toc[i].offset,
 		    (uintmax_t)max_offset));
 		backref_to = BLEN_UNDEF;
@@ -523,6 +565,7 @@ g_uzip_parse_toc(struct g_uzip_softc *sc, struct g_provider *pp,
 			sc->toc[i].blen = sc->toc[j].blen;
 			backref_to = j;
 		} else {
+			last_blk = &sc->toc[i];
 			/*
 			 * For the "normal blocks" seek forward until we hit
 			 * block whose offset is larger than ours and assume
@@ -557,6 +600,25 @@ g_uzip_parse_toc(struct g_uzip_softc *sc, struct g_provider *pp,
 		}
 		DPRINTF_BLK(GUZ_DBG_TOC, i, ("\n"));
 	}
+	last_blk->last = 1;
+	/* Do a second pass to validate block lengths */
+	for (i = 0; i < sc->nblocks; i++) {
+		if (sc->toc[i].blen > sc->dcp->max_blen) {
+			if (sc->toc[i].last == 0) {
+				DPRINTF(GUZ_DBG_ERR, ("%s: cluster #%u "
+				    "length (%ju) exceeds "
+				    "max_blen (%ju)\n", gp->name, i,
+				    (uintmax_t)sc->toc[i].blen,
+				    (uintmax_t)sc->dcp->max_blen));
+				return (-1);
+			}
+			DPRINTF(GUZ_DBG_INFO, ("%s: cluster #%u extra "
+			    "padding is detected, trimmed to %ju\n",
+			    gp->name, i, (uintmax_t)sc->dcp->max_blen));
+			    sc->toc[i].blen = sc->dcp->max_blen;
+			sc->toc[i].padded = 1;
+		}
+	}
 	return (0);
 
 error_offset:
@@ -589,12 +651,19 @@ g_uzip_taste(struct g_class *mp, struct g_provider *pp, int flags)
 	if (pp->acw > 0)
 		return (NULL);
 
+	if ((fnmatch(g_uzip_attach_to, pp->name, 0) != 0) ||
+	    (fnmatch(g_uzip_noattach_to, pp->name, 0) == 0)) {
+		DPRINTF(GUZ_DBG_INFO, ("%s(%s,%s), ignoring\n", __func__,
+		    mp->name, pp->name));
+		return (NULL);
+	}
+
 	buf = NULL;
 
 	/*
 	 * Create geom instance.
 	 */
-	gp = g_new_geomf(mp, "%s.uzip", pp->name);
+	gp = g_new_geomf(mp, GUZ_DEV_NAME("%s"), pp->name);
 	cp = g_new_consumer(gp);
 	error = g_attach(cp, pp);
 	if (error == 0)
@@ -712,6 +781,16 @@ g_uzip_taste(struct g_class *mp, struct g_provider *pp, int flags)
 		    sc->nblocks < offsets_read ? "more" : "less"));
 		goto e5;
 	}
+
+	if (type == G_UZIP) {
+		sc->dcp = g_uzip_zlib_ctor(sc->blksz);
+	} else {
+		sc->dcp = g_uzip_lzma_ctor(sc->blksz);
+	}
+	if (sc->dcp == NULL) {
+		goto e5;
+	}
+
 	/*
 	 * "Fake" last+1 block, to make it easier for the TOC parser to
 	 * iterate without making the last element a special case.
@@ -720,7 +799,7 @@ g_uzip_taste(struct g_class *mp, struct g_provider *pp, int flags)
 	/* Massage TOC (table of contents), make sure it is sound */
 	if (g_uzip_parse_toc(sc, pp, gp) != 0) {
 		DPRINTF(GUZ_DBG_ERR, ("%s: TOC error\n", gp->name));
-		goto e5;
+		goto e6;
 	}
 	mtx_init(&sc->last_mtx, "geom_uzip cache", NULL, MTX_DEF);
 	mtx_init(&sc->queue_mtx, "geom_uzip wrkthread", NULL, MTX_DEF);
@@ -730,15 +809,6 @@ g_uzip_taste(struct g_class *mp, struct g_provider *pp, int flags)
 	sc->req_total = 0;
 	sc->req_cached = 0;
 
-	if (type == G_UZIP) {
-		sc->dcp = g_uzip_zlib_ctor(sc->blksz);
-	} else {
-		sc->dcp = g_uzip_lzma_ctor(sc->blksz);
-	}
-	if (sc->dcp == NULL) {
-		goto e6;
-	}
-
 	sc->uzip_do = &g_uzip_do;
 
 	error = kproc_create(g_uzip_wrkthr, sc, &sc->procp, 0, 0, "%s",
@@ -764,11 +834,11 @@ g_uzip_taste(struct g_class *mp, struct g_provider *pp, int flags)
 	return (gp);
 
 e7:
-	sc->dcp->free(sc->dcp);
-e6:
 	free(sc->last_buf, M_GEOM);
 	mtx_destroy(&sc->queue_mtx);
 	mtx_destroy(&sc->last_mtx);
+e6:
+	sc->dcp->free(sc->dcp);
 e5:
 	free(sc->toc, M_GEOM);
 e4:
diff --git a/sys/geom/uzip/g_uzip_dapi.h b/sys/geom/uzip/g_uzip_dapi.h
index 0689c32..8c7b3e8 100644
--- a/sys/geom/uzip/g_uzip_dapi.h
+++ b/sys/geom/uzip/g_uzip_dapi.h
@@ -38,4 +38,5 @@ struct g_uzip_dapi {
 	g_uzip_dapi_free_t free;
 	g_uzip_dapi_rewind_t rewind;
 	void *pvt;
+	int max_blen;
 };
diff --git a/sys/geom/uzip/g_uzip_lzma.c b/sys/geom/uzip/g_uzip_lzma.c
index 0a981ba..9efb087 100644
--- a/sys/geom/uzip/g_uzip_lzma.c
+++ b/sys/geom/uzip/g_uzip_lzma.c
@@ -32,6 +32,7 @@ __FBSDID("$FreeBSD$");
 
 #include <sys/types.h>
 #include <sys/malloc.h>
+#include <sys/systm.h>
 
 #include <contrib/xz-embedded/linux/include/linux/xz.h>
 
@@ -41,6 +42,7 @@ __FBSDID("$FreeBSD$");
 
 struct g_uzip_lzma {
 	struct g_uzip_dapi pub;
+	uint32_t blksz;
 	/* XZ decoder structs */
 	struct xz_buf b;
 	struct xz_dec *s;
@@ -75,13 +77,26 @@ g_uzip_lzma_decompress(struct g_uzip_dapi *lzpp, const char *gp_name,
 	lzp->b.out = obp;
 	lzp->b.in_pos = lzp->b.out_pos = 0;
 	lzp->b.in_size = ilen;
-	lzp->b.out_size = (size_t)-1;
+	lzp->b.out_size = lzp->blksz;
 	err = (xz_dec_run(lzp->s, &lzp->b) != XZ_STREAM_END) ? 1 : 0;
 	/* TODO decoder recovery, if needed */
+	if (err != 0) {
+		printf("%s: ibp=%p, obp=%p, in_pos=%jd, out_pos=%jd, "
+		    "in_size=%jd, out_size=%jd\n", __func__, ibp, obp,
+		    (intmax_t)lzp->b.in_pos, (intmax_t)lzp->b.out_pos,
+		    (intmax_t)lzp->b.in_size, (intmax_t)lzp->b.out_size);
+	}
 
 	return (err);
 }
 
+static int
+LZ4_compressBound(int isize)
+{
+
+        return (isize + (isize / 255) + 16);
+}
+
 struct g_uzip_dapi *
 g_uzip_lzma_ctor(uint32_t blksz)
 {
@@ -93,6 +108,8 @@ g_uzip_lzma_ctor(uint32_t blksz)
 	if (lzp->s == NULL) {
 		goto e1;
 	}
+	lzp->blksz = blksz;
+	lzp->pub.max_blen = LZ4_compressBound(blksz);
 	lzp->pub.decompress = &g_uzip_lzma_decompress;
 	lzp->pub.free = &g_uzip_lzma_free;
 	lzp->pub.rewind = &g_uzip_lzma_nop;
diff --git a/sys/geom/uzip/g_uzip_zlib.c b/sys/geom/uzip/g_uzip_zlib.c
index cecac6e..895c92d 100644
--- a/sys/geom/uzip/g_uzip_zlib.c
+++ b/sys/geom/uzip/g_uzip_zlib.c
@@ -97,6 +97,13 @@ g_uzip_zlib_rewind(struct g_uzip_dapi *zpp, const char *gp_name)
 	return (err);
 }
 
+static int
+z_compressBound(int len)
+{
+
+	return (len + (len >> 12) + (len >> 14) + 11);
+}
+
 struct g_uzip_dapi *
 g_uzip_zlib_ctor(uint32_t blksz)
 {
@@ -109,6 +116,7 @@ g_uzip_zlib_ctor(uint32_t blksz)
 		goto e1;
 	}
 	zp->blksz = blksz;
+	zp->pub.max_blen = z_compressBound(blksz);
 	zp->pub.decompress = &g_uzip_zlib_decompress;
 	zp->pub.free = &g_uzip_zlib_free;
 	zp->pub.rewind = &g_uzip_zlib_rewind;