svn commit: r302284 - head/sys/geom/uzip

Maxim Sobolev sobomax at FreeBSD.org
Wed Jun 29 18:19:07 UTC 2016


Author: sobomax
Date: Wed Jun 29 18:19:05 2016
New Revision: 302284
URL: https://svnweb.freebsd.org/changeset/base/302284

Log:
  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

Modified:
  head/sys/geom/uzip/g_uzip.c
  head/sys/geom/uzip/g_uzip_dapi.h
  head/sys/geom/uzip/g_uzip_lzma.c
  head/sys/geom/uzip/g_uzip_zlib.c

Modified: head/sys/geom/uzip/g_uzip.c
==============================================================================
--- head/sys/geom/uzip/g_uzip.c	Wed Jun 29 17:25:46 2016	(r302283)
+++ head/sys/geom/uzip/g_uzip.c	Wed Jun 29 18:19:05 2016	(r302284)
@@ -60,6 +60,8 @@ FEATURE(geom_uzip, "GEOM read-only compr
 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
 	}
 
 	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
 	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, struc
 		    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
 {
 	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
 			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
 			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
 		}
 		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 
 	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 
 		    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 
 	/* 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 
 	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 
 	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:

Modified: head/sys/geom/uzip/g_uzip_dapi.h
==============================================================================
--- head/sys/geom/uzip/g_uzip_dapi.h	Wed Jun 29 17:25:46 2016	(r302283)
+++ head/sys/geom/uzip/g_uzip_dapi.h	Wed Jun 29 18:19:05 2016	(r302284)
@@ -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;
 };

Modified: head/sys/geom/uzip/g_uzip_lzma.c
==============================================================================
--- head/sys/geom/uzip/g_uzip_lzma.c	Wed Jun 29 17:25:46 2016	(r302283)
+++ head/sys/geom/uzip/g_uzip_lzma.c	Wed Jun 29 18:19:05 2016	(r302284)
@@ -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_dap
 	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;

Modified: head/sys/geom/uzip/g_uzip_zlib.c
==============================================================================
--- head/sys/geom/uzip/g_uzip_zlib.c	Wed Jun 29 17:25:46 2016	(r302283)
+++ head/sys/geom/uzip/g_uzip_zlib.c	Wed Jun 29 18:19:05 2016	(r302284)
@@ -97,6 +97,13 @@ g_uzip_zlib_rewind(struct g_uzip_dapi *z
 	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;


More information about the svn-src-head mailing list