From 761d063e808061f78a2bd10e65cc88ff442124c6 Mon Sep 17 00:00:00 2001 From: phk Date: Wed, 18 Dec 2002 19:57:27 +0000 Subject: [PATCH] Fix two blunders in the mapping functions which can lead to corrupt data, for request sizes larger than the sectorsize or for multi-key setups. See warning mailed to current@ for details of recovery. Found by: Marcus Reid --- sbin/gbde/gbde.c | 22 ++++++++++++++++- sys/geom/bde/g_bde_crypt.c | 4 ++-- sys/geom/bde/g_bde_lock.c | 6 +++++ sys/geom/bde/g_bde_work.c | 49 ++++++-------------------------------- 4 files changed, 36 insertions(+), 45 deletions(-) diff --git a/sbin/gbde/gbde.c b/sbin/gbde/gbde.c index e3e278eb4237..041d206598cc 100644 --- a/sbin/gbde/gbde.c +++ b/sbin/gbde/gbde.c @@ -418,7 +418,16 @@ cmd_write(struct g_bde_key *gl, struct g_bde_softc *sc, int dfd , int key, const if (i != (int)gl->sectorsize) err(1, "write"); printf("Wrote key %d at %jd\n", key, (intmax_t)offset); - +#if 0 + printf("s0 = %jd\n", (intmax_t)gl->sector0); + printf("sN = %jd\n", (intmax_t)gl->sectorN); + printf("l[0] = %jd\n", (intmax_t)gl->lsector[0]); + printf("l[1] = %jd\n", (intmax_t)gl->lsector[1]); + printf("l[2] = %jd\n", (intmax_t)gl->lsector[2]); + printf("l[3] = %jd\n", (intmax_t)gl->lsector[3]); + printf("k = %jd\n", (intmax_t)gl->keyoffset); + printf("ss = %jd\n", (intmax_t)gl->sectorsize); +#endif } static void @@ -436,6 +445,16 @@ cmd_destroy(struct g_bde_key *gl, int nkey) gl->lsector[i] = ~0; } +static int +sorthelp(const void *a, const void *b) +{ + const off_t *oa, *ob; + + oa = a; + ob = b; + return (*oa - *ob); +} + static void cmd_init(struct g_bde_key *gl, int dfd, const char *f_opt, int i_opt, const char *l_opt) { @@ -597,6 +616,7 @@ cmd_init(struct g_bde_key *gl, int dfd, const char *f_opt, int i_opt, const char while (o < gl->sectorN); gl->lsector[u] = o; } + qsort(gl->lsector, G_BDE_MAXKEYS, sizeof gl->lsector[0], sorthelp); /* Flush sector zero if we use it for lockfile data */ if (gl->flags & 1) { diff --git a/sys/geom/bde/g_bde_crypt.c b/sys/geom/bde/g_bde_crypt.c index b769f684ef88..6b5619be01e8 100644 --- a/sys/geom/bde/g_bde_crypt.c +++ b/sys/geom/bde/g_bde_crypt.c @@ -275,7 +275,7 @@ g_bde_map_sector(struct g_bde_key *kp, /* Find physical sector address */ os = zone * kp->zone_width + zoff; os += kp->keyoffset; - os %= kp->media_width - (G_BDE_MAXKEYS * kp->sectorsize); + os %= kp->media_width; os += kp->sector0; /* Compensate for lock sectors */ @@ -288,7 +288,7 @@ g_bde_map_sector(struct g_bde_key *kp, /* The key sector is the last in this zone. */ os = (1 + zone) * kp->zone_width - kp->sectorsize; os += kp->keyoffset; - os %= kp->media_width - (G_BDE_MAXKEYS * kp->sectorsize); + os %= kp->media_width; os += kp->sector0; for (u = 0; u < G_BDE_MAXKEYS; u++) diff --git a/sys/geom/bde/g_bde_lock.c b/sys/geom/bde/g_bde_lock.c index e584c4d0385a..75aa40b51b99 100644 --- a/sys/geom/bde/g_bde_lock.c +++ b/sys/geom/bde/g_bde_lock.c @@ -430,6 +430,12 @@ g_bde_decrypt_lockx(struct g_bde_softc *sc, u_char *meta, off_t mediasize, u_int if (off[1] == 0) return (ENOENT); + /* If we have an unsorted lock-sequence, refuse */ + if (gl->lsector[0] > gl->lsector[1] || + gl->lsector[1] > gl->lsector[2] || + gl->lsector[2] > gl->lsector[3]) + return (EINVAL); + /* Finally, find out which key was used by matching the byte offset */ for (i = 0; i < G_BDE_MAXKEYS; i++) if (nkey != NULL && off[0] == gl->lsector[i]) diff --git a/sys/geom/bde/g_bde_work.c b/sys/geom/bde/g_bde_work.c index 521177118ebb..e173d804aa07 100644 --- a/sys/geom/bde/g_bde_work.c +++ b/sys/geom/bde/g_bde_work.c @@ -713,60 +713,25 @@ g_bde_start1(struct bio *bp) { struct g_bde_softc *sc; struct g_bde_work *wp; - off_t zone_start, left; - caddr_t p; + off_t left; sc = bp->bio_to->geom->softc; bp->bio_driver1 = sc; mtx_lock(&sc->worklist_mutex); - zone_start = bp->bio_offset - bp->bio_offset % sc->zone_cont; - wp = g_bde_new_work(sc); - if (wp == NULL) { - g_io_deliver(bp, ENOMEM); - mtx_unlock(&sc->worklist_mutex); - return; - } - left = bp->bio_length; - p = bp->bio_data; - - /* Do the first and possible only fragment */ - wp->bp = bp; - wp->offset = bp->bio_offset; - wp->data = p; - wp->length = zone_start + sc->zone_cont - wp->offset; - if (wp->length >= left) { - /* Only this one fragment needed */ - wp->length = left; - g_bde_start2(wp); - mtx_unlock(&sc->worklist_mutex); - return; - } - - /* Submit the first fragment */ - g_bde_start2(wp); - left -= wp->length; - p += wp->length; - - /* Do the subsequent fragments */ - for(;left > 0;) { + for(left = 0;left < bp->bio_length; left += sc->sectorsize) { wp = g_bde_new_work(sc); if (wp == NULL) { - g_bde_contribute(bp, left, ENOMEM); + g_io_deliver(bp, ENOMEM); mtx_unlock(&sc->worklist_mutex); return; } - zone_start += sc->zone_cont; wp->bp = bp; - wp->offset = zone_start; - wp->data = p; - if (left > sc->zone_cont) - wp->length = sc->zone_cont; - else - wp->length = left; - left -= wp->length; - p += wp->length; + wp->offset = bp->bio_offset + left; + wp->data = bp->bio_data + left; + wp->length = sc->sectorsize; g_bde_start2(wp); } mtx_unlock(&sc->worklist_mutex); + return; }