From owner-svn-src-all@freebsd.org Wed Jun 29 18:19:07 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 465C5B869C6; Wed, 29 Jun 2016 18:19:07 +0000 (UTC) (envelope-from sobomax@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 233B520D3; Wed, 29 Jun 2016 18:19:07 +0000 (UTC) (envelope-from sobomax@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id u5TIJ6XI030915; Wed, 29 Jun 2016 18:19:06 GMT (envelope-from sobomax@FreeBSD.org) Received: (from sobomax@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id u5TIJ69s030911; Wed, 29 Jun 2016 18:19:06 GMT (envelope-from sobomax@FreeBSD.org) Message-Id: <201606291819.u5TIJ69s030911@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: sobomax set sender to sobomax@FreeBSD.org using -f From: Maxim Sobolev Date: Wed, 29 Jun 2016 18:19:06 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r302284 - head/sys/geom/uzip X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Jun 2016 18:19:07 -0000 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 #include +#include #include @@ -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;