Date: Sat, 7 Mar 2015 18:01:31 -0600 From: "Matthew D. Fuller" <fullermd@over-yonder.net> To: freebsd-geom@freebsd.org Subject: RFC: Pass TRIM through GELI Message-ID: <20150308000131.GP1742@over-yonder.net>
next in thread | raw e-mail | index | archive | help
[-- Attachment #1 --] [ bcc'd to -fs because it's been discussed there in the past; followup to -geom because that seems where it belongs ] I've done a quick implementation of TRIM passthru support for GELI. Patch attached is against late Feb -CURRENT; however, a glance at svn suggests this code hasn't changed much, so it's probably pretty portable forward and back. It applies cleanly to a stable/10 tree though I haven't tried compiling or using it there. This has been lightly tested and seems to work fine. It adds a '-t' flag to init and onetime to enable passthru on the new provider, and '-t/-T' options to configure to en/disable on existing (but see caveat below about metadata versions; as written, you can't use this on existing partitions). Since all we're doing is passing it through instead of denying it, I'd expect "seems to work" to be pretty much the same as "does work"; the requests go through, space clears up, and messing with a little ZFS on top of it doesn't show up any data corruption. The patch to gnop in <https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=198405>; is a useful adjunct in checking when/if the requests actually go through, by making the .eli on top of the .nop and seeing when the counters tick. In this implementation, I added a new G_ELI_VERSION and required it for setting the flag. I actually think this is probably not necessary; all we do is set a value in the flags field, and if it's loaded onto a system that doesn't know what to do with that value, it'll just do nothing, which IMO is fine. And since there is no `geli upgrade`, this means that you can't enable it on any existing partitions, but have to kill/init from scratch, which isn't very user-friendly. So I propose that it actually be done sans those version changes, but they are in the patch as attached for now. One not-quite-related change in here is that I added a denial for 'geli configure' attempts on onetime providers. Trying this causes a panic, as the metadata version field on onetime providers is nonsensical. As far as I can tell in quick testing, this isn't related to my changes; it happens with stock GELI code as well. Presumably it's never been noticed before since the only prior use for 'geli configure' was to un/set the BOOT flag, which makes no sense on a onetime anyway, so nobody ever bothered trying. But with -[tT], somebody might try. Evidence: I did, and found the panic :) -- Matthew Fuller (MF4839) | fullermd@over-yonder.net Systems/Network Administrator | http://www.over-yonder.net/~fullermd/ On the Internet, nobody can hear you scream. [-- Attachment #2 --] Index: sbin/geom/class/eli/geli.8 =================================================================== --- sbin/geom/class/eli/geli.8 (revision 279210) +++ sbin/geom/class/eli/geli.8 (working copy) @@ -51,7 +51,7 @@ .Pp .Nm .Cm init -.Op Fl bPv +.Op Fl bPtv .Op Fl a Ar aalgo .Op Fl B Ar backupfile .Op Fl e Ar ealgo @@ -80,7 +80,7 @@ .Cm detach .Nm .Cm onetime -.Op Fl d +.Op Fl dt .Op Fl a Ar aalgo .Op Fl e Ar ealgo .Op Fl l Ar keylen @@ -88,7 +88,7 @@ .Ar prov .Nm .Cm configure -.Op Fl bB +.Op Fl bBtT .Ar prov ... .Nm .Cm setkey @@ -351,6 +351,12 @@ Increasing the sector size allows increased performance, because encryption/decryption which requires an initialization vector is done per sector; fewer sectors means less computational work. +.It Fl t +Pass through BIO_DELETE calls (i.e., TRIM/UNMAP). +This can be useful when encrypting on a SSD, by allowing it to clean up +sectors you're no longer using. +It will, however, provide some information (how much space you're actually +using, and which sectors contain real data) to an attacker. .It Fl V Ar version Metadata version to use. This option is helpful when creating a provider that may be used by older @@ -456,6 +462,11 @@ For more information, see the description of the .Cm init subcommand. +.It Fl t +Enable TRIM/UNMAP passthru. +For more information, see the description of the +.Cm init +subcommand. .El .It Cm configure Change configuration of the given providers. @@ -469,6 +480,13 @@ subcommand. .It Fl B Remove the BOOT flag from the given providers. +.It Fl t +Enable TRIM/UNMAP passthru. +For more information, see the description of the +.Cm init +subcommand. +.It Fl T +Disable TRIM/UNMAP passthru. .El .It Cm setkey Install a copy of the Master Key into the selected slot, encrypted with Index: sbin/geom/class/eli/geom_eli.c =================================================================== --- sbin/geom/class/eli/geom_eli.c (revision 279210) +++ sbin/geom/class/eli/geom_eli.c (working copy) @@ -114,10 +114,11 @@ { 'l', "keylen", "0", G_TYPE_NUMBER }, { 'P', "nonewpassphrase", NULL, G_TYPE_BOOL }, { 's', "sectorsize", "0", G_TYPE_NUMBER }, + { 't', "trim", NULL, G_TYPE_BOOL }, { 'V', "mdversion", "-1", G_TYPE_NUMBER }, G_OPT_SENTINEL }, - "[-bPv] [-a aalgo] [-B backupfile] [-e ealgo] [-i iterations] [-l keylen] [-J newpassfile] [-K newkeyfile] [-s sectorsize] [-V version] prov" + "[-bPtv] [-a aalgo] [-B backupfile] [-e ealgo] [-i iterations] [-l keylen] [-J newpassfile] [-K newkeyfile] [-s sectorsize] [-V version] prov" }, { "label", G_FLAG_VERBOSE, eli_main, { @@ -170,17 +171,20 @@ { 'e', "ealgo", GELI_ENC_ALGO, G_TYPE_STRING }, { 'l', "keylen", "0", G_TYPE_NUMBER }, { 's', "sectorsize", "0", G_TYPE_NUMBER }, + { 't', "trim", NULL, G_TYPE_BOOL }, G_OPT_SENTINEL }, - "[-d] [-a aalgo] [-e ealgo] [-l keylen] [-s sectorsize] prov" + "[-dt] [-a aalgo] [-e ealgo] [-l keylen] [-s sectorsize] prov" }, { "configure", G_FLAG_VERBOSE, eli_main, { { 'b', "boot", NULL, G_TYPE_BOOL }, { 'B', "noboot", NULL, G_TYPE_BOOL }, + { 't', "trim", NULL, G_TYPE_BOOL }, + { 'T', "notrim", NULL, G_TYPE_BOOL }, G_OPT_SENTINEL }, - "[-bB] prov ..." + "[-bBtT] prov ..." }, { "setkey", G_FLAG_VERBOSE, eli_main, { @@ -698,6 +702,14 @@ md.md_flags = 0; if (gctl_get_int(req, "boot")) md.md_flags |= G_ELI_FLAG_BOOT; + if (gctl_get_int(req, "trim")) { + if (version < G_ELI_VERSION_08) { + gctl_error(req, "TRIM is supported starting from version %u.", + G_ELI_VERSION_08); + return; + } + md.md_flags |= G_ELI_FLAG_DELETE; + } md.md_ealgo = CRYPTO_ALGORITHM_MIN - 1; str = gctl_get_ascii(req, "aalgo"); if (*str != '\0') { @@ -899,26 +911,50 @@ } static void -eli_configure_detached(struct gctl_req *req, const char *prov, bool boot) +eli_configure_detached(struct gctl_req *req, const char *prov, int boot, + int trim) { struct g_eli_metadata md; + bool changed = 0; if (eli_metadata_read(req, prov, &md) == -1) return; - if (boot && (md.md_flags & G_ELI_FLAG_BOOT)) { + if (boot == 1 && (md.md_flags & G_ELI_FLAG_BOOT)) { if (verbose) printf("BOOT flag already configured for %s.\n", prov); - } else if (!boot && !(md.md_flags & G_ELI_FLAG_BOOT)) { + } else if (boot == 0 && !(md.md_flags & G_ELI_FLAG_BOOT)) { if (verbose) printf("BOOT flag not configured for %s.\n", prov); - } else { + } else if (boot >= 0) { if (boot) md.md_flags |= G_ELI_FLAG_BOOT; else md.md_flags &= ~G_ELI_FLAG_BOOT; + changed = 1; + } + + if (trim == 1 && (md.md_flags & G_ELI_FLAG_DELETE)) { + if (verbose) + printf("TRIM flag already configured for %s.\n", prov); + } else if (trim == 0 && !(md.md_flags & G_ELI_FLAG_DELETE)) { + if (verbose) + printf("TRIM flag not configured for %s.\n", prov); + } else if (trim >= 0) { + if(md.md_version < G_ELI_VERSION_08) { + printf("TRIM requires version %u, %s is %u\n", G_ELI_VERSION_08, + prov, md.md_version); + } else { + if (trim) + md.md_flags |= G_ELI_FLAG_DELETE; + else + md.md_flags &= ~G_ELI_FLAG_DELETE; + changed = 1; + } + } + + if(changed) eli_metadata_store(req, prov, &md); - } bzero(&md, sizeof(md)); } @@ -926,7 +962,8 @@ eli_configure(struct gctl_req *req) { const char *prov; - bool boot, noboot; + bool boot, noboot, trim, notrim; + int doboot, dotrim; int i, nargs; nargs = gctl_get_int(req, "nargs"); @@ -937,12 +974,30 @@ boot = gctl_get_int(req, "boot"); noboot = gctl_get_int(req, "noboot"); + trim = gctl_get_int(req, "trim"); + notrim = gctl_get_int(req, "notrim"); + doboot = -1; if (boot && noboot) { gctl_error(req, "Options -b and -B are mutually exclusive."); return; } - if (!boot && !noboot) { + if (boot) + doboot = 1; + else if (noboot) + doboot = 0; + + dotrim = -1; + if (trim && notrim) { + gctl_error(req, "Options -t and -T are mutually exclusive."); + return; + } + if (trim) + dotrim = 1; + else if (notrim) + dotrim = 0; + + if (doboot == -1 && dotrim == -1) { gctl_error(req, "No option given."); return; } @@ -953,7 +1008,7 @@ for (i = 0; i < nargs; i++) { prov = gctl_get_ascii(req, "arg%d", i); if (!eli_is_attached(prov)) - eli_configure_detached(req, prov, boot); + eli_configure_detached(req, prov, doboot, dotrim); } } Index: sys/geom/eli/g_eli.c =================================================================== --- sys/geom/eli/g_eli.c (revision 279210) +++ sys/geom/eli/g_eli.c (working copy) @@ -314,8 +314,11 @@ /* * We could eventually support BIO_DELETE request. * It could be done by overwritting requested sector with - * random data g_eli_overwrites number of times. + * random data g_eli_overwrites number of times. Or if the user + * has set the DELETE flag, we just pass it down the stack. */ + if (sc->sc_flags & G_ELI_FLAG_DELETE) + break; default: g_io_deliver(bp, EOPNOTSUPP); return; @@ -342,6 +345,7 @@ break; case BIO_GETATTR: case BIO_FLUSH: + case BIO_DELETE: cbp->bio_done = g_std_done; cp = LIST_FIRST(&sc->sc_geom->consumer); cbp->bio_to = cp->provider; @@ -1245,6 +1249,7 @@ ADD_FLAG(G_ELI_FLAG_WOPEN, "W-OPEN"); ADD_FLAG(G_ELI_FLAG_DESTROY, "DESTROY"); ADD_FLAG(G_ELI_FLAG_RO, "READ-ONLY"); + ADD_FLAG(G_ELI_FLAG_DELETE, "DELETE"); #undef ADD_FLAG } sbuf_printf(sb, "</Flags>\n"); Index: sys/geom/eli/g_eli.h =================================================================== --- sys/geom/eli/g_eli.h (revision 279210) +++ sys/geom/eli/g_eli.h (working copy) @@ -70,6 +70,7 @@ * G_ELI_FLAG_FIRST_KEY flag will be set for older versions). * 7 - Encryption keys are now generated from the Data Key and not from the * IV Key (the G_ELI_FLAG_ENC_IVKEY flag will be set for older versions). + * 8 - Added G_ELI_FLAG_DELETE. */ #define G_ELI_VERSION_00 0 #define G_ELI_VERSION_01 1 @@ -79,7 +80,8 @@ #define G_ELI_VERSION_05 5 #define G_ELI_VERSION_06 6 #define G_ELI_VERSION_07 7 -#define G_ELI_VERSION G_ELI_VERSION_07 +#define G_ELI_VERSION_08 8 +#define G_ELI_VERSION G_ELI_VERSION_08 /* ON DISK FLAGS. */ /* Use random, onetime keys. */ @@ -94,6 +96,8 @@ #define G_ELI_FLAG_AUTH 0x00000010 /* Provider is read-only, we should deny all write attempts. */ #define G_ELI_FLAG_RO 0x00000020 +/* Pass through BIO_DELETE requests */ +#define G_ELI_FLAG_DELETE 0x00000040 /* RUNTIME FLAGS. */ /* Provider was open for writing. */ #define G_ELI_FLAG_WOPEN 0x00010000 @@ -242,7 +246,7 @@ *datap = p; } static __inline void -eli_metadata_encode_v1v2v3v4v5v6v7(struct g_eli_metadata *md, u_char **datap) +eli_metadata_encode_v1_to_8(struct g_eli_metadata *md, u_char **datap) { u_char *p; @@ -281,7 +285,8 @@ case G_ELI_VERSION_05: case G_ELI_VERSION_06: case G_ELI_VERSION_07: - eli_metadata_encode_v1v2v3v4v5v6v7(md, &p); + case G_ELI_VERSION_08: + eli_metadata_encode_v1_to_8(md, &p); break; default: #ifdef _KERNEL @@ -321,7 +326,7 @@ } static __inline int -eli_metadata_decode_v1v2v3v4v5v6v7(const u_char *data, struct g_eli_metadata *md) +eli_metadata_decode_v1_to_8(const u_char *data, struct g_eli_metadata *md) { MD5_CTX ctx; const u_char *p; @@ -364,7 +369,8 @@ case G_ELI_VERSION_05: case G_ELI_VERSION_06: case G_ELI_VERSION_07: - error = eli_metadata_decode_v1v2v3v4v5v6v7(data, md); + case G_ELI_VERSION_08: + error = eli_metadata_decode_v1_to_8(data, md); break; default: error = EOPNOTSUPP; Index: sys/geom/eli/g_eli_ctl.c =================================================================== --- sys/geom/eli/g_eli_ctl.c (revision 279210) +++ sys/geom/eli/g_eli_ctl.c (working copy) @@ -236,7 +236,7 @@ const char *name; intmax_t *keylen, *sectorsize; u_char mkey[G_ELI_DATAIVKEYLEN]; - int *nargs, *detach; + int *nargs, *detach, *trim; g_topology_assert(); bzero(&md, sizeof(md)); @@ -256,6 +256,11 @@ gctl_error(req, "No '%s' argument.", "detach"); return; } + trim = gctl_get_paraml(req, "trim", sizeof(*trim)); + if (detach == NULL) { + gctl_error(req, "No '%s' argument.", "trim"); + return; + } strlcpy(md.md_magic, G_ELI_MAGIC, sizeof(md.md_magic)); md.md_version = G_ELI_VERSION; @@ -262,6 +267,8 @@ md.md_flags |= G_ELI_FLAG_ONETIME; if (*detach) md.md_flags |= G_ELI_FLAG_WO_DETACH; + if (*trim) + md.md_flags |= G_ELI_FLAG_DELETE; md.md_ealgo = CRYPTO_ALGORITHM_MIN - 1; name = gctl_get_asciiparam(req, "aalgo"); @@ -377,11 +384,13 @@ char param[16]; const char *prov; u_char *sector; - int *nargs, *boot, *noboot; + int *nargs, *boot, *noboot, *trim, *notrim; int error; + int changed; u_int i; g_topology_assert(); + changed = 0; nargs = gctl_get_paraml(req, "nargs", sizeof(*nargs)); if (nargs == NULL) { @@ -407,7 +416,27 @@ gctl_error(req, "Options -b and -B are mutually exclusive."); return; } - if (!*boot && !*noboot) { + if (*boot || *noboot) + changed = 1; + + trim = gctl_get_paraml(req, "trim", sizeof(*trim)); + if (trim == NULL) { + gctl_error(req, "No '%s' argument.", "trim"); + return; + } + notrim = gctl_get_paraml(req, "notrim", sizeof(*notrim)); + if (notrim == NULL) { + gctl_error(req, "No '%s' argument.", "notrim"); + return; + } + if (*trim && *notrim) { + gctl_error(req, "Options -t and -T are mutually exclusive."); + return; + } + if (*trim || *notrim) + changed = 1; + + if (!changed) { gctl_error(req, "No option given."); return; } @@ -429,20 +458,41 @@ "provider %s.", prov); continue; } + if (sc->sc_flags & G_ELI_FLAG_RO) { + gctl_error(req, "Cannot change configuration of " + "read-only provider %s.", prov); + continue; + } + if (sc->sc_flags & G_ELI_FLAG_ONETIME) { + gctl_error(req, "Cannot change configuration of " + "onetime provider %s.", prov); + continue; + } + if (*boot && (sc->sc_flags & G_ELI_FLAG_BOOT)) { G_ELI_DEBUG(1, "BOOT flag already configured for %s.", prov); continue; - } else if (!*boot && !(sc->sc_flags & G_ELI_FLAG_BOOT)) { + } else if (*noboot && !(sc->sc_flags & G_ELI_FLAG_BOOT)) { G_ELI_DEBUG(1, "BOOT flag not configured for %s.", prov); continue; } - if (sc->sc_flags & G_ELI_FLAG_RO) { - gctl_error(req, "Cannot change configuration of " - "read-only provider %s.", prov); + + if (*trim && (sc->sc_flags & G_ELI_FLAG_DELETE)) { + G_ELI_DEBUG(1, "TRIM flag already configured for %s.", + prov); continue; + } else if (*notrim && !(sc->sc_flags & G_ELI_FLAG_DELETE)) { + G_ELI_DEBUG(1, "TRIM flag not configured for %s.", + prov); + continue; + } else if ((*trim || *notrim) && sc->sc_version < G_ELI_VERSION_08) { + gctl_error(req, "TRIM requires version %u, %s is %u.", + G_ELI_VERSION_08, prov, sc->sc_version); + continue; } + cp = LIST_FIRST(&sc->sc_geom->consumer); pp = cp->provider; error = g_eli_read_metadata(mp, pp, &md); @@ -456,11 +506,19 @@ if (*boot) { md.md_flags |= G_ELI_FLAG_BOOT; sc->sc_flags |= G_ELI_FLAG_BOOT; - } else { + } else if (*noboot) { md.md_flags &= ~G_ELI_FLAG_BOOT; sc->sc_flags &= ~G_ELI_FLAG_BOOT; } + if (*trim) { + md.md_flags |= G_ELI_FLAG_DELETE; + sc->sc_flags |= G_ELI_FLAG_DELETE; + } else if (*notrim) { + md.md_flags &= ~G_ELI_FLAG_DELETE; + sc->sc_flags &= ~G_ELI_FLAG_DELETE; + } + sector = malloc(pp->sectorsize, M_ELI, M_WAITOK | M_ZERO); eli_metadata_encode(&md, sector); error = g_write_data(cp, pp->mediasize - pp->sectorsize, sector,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150308000131.GP1742>