Date: Mon, 3 Dec 2018 03:58:30 +0000 (UTC) From: Ian Lepore <ian@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r341420 - in head/stand: i386/common i386/gptboot i386/loader i386/zfsboot libsa/zfs Message-ID: <201812030358.wB33wUl1081824@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: ian Date: Mon Dec 3 03:58:30 2018 New Revision: 341420 URL: https://svnweb.freebsd.org/changeset/base/341420 Log: Eliminate duplicated code and struct member definitions in the handoff of args data between gptboot/zfsboot and loader(8). Despite what seems like a lot of changes here, there are no actual changes in behavior, or in the data layout in the structures involved. This is just eliminating identical code pasted into multiple locations. In detail, the changes are... - Move struct zfs_boot_args definition from libsa/zfs/libzfs.h to i386/common/bootargs.h because it is specific to x86 booting and the handoff between zfsboot and loader, and has no relation to the zfs library code in general. - The geli_boot_args and zfs_boot_args structs both contain an identical set of member variables containing geli information. Extract this out to a new geli_boot_data struct, and embed it in the arg-passing structs. - Provide new routines geli_import_boot_data() and geli_export_boot_data() that can be shared between gptboot, zfsboot, and loader instead of pasting identical code into several different .c files. - Remove some checks for a NULL pointer that can never be true because the pointer being tested was set using pointer math (kargs + 1) and that can never result in NULL in this code. Modified: head/stand/i386/common/bootargs.h head/stand/i386/gptboot/gptboot.c head/stand/i386/loader/main.c head/stand/i386/zfsboot/zfsboot.c head/stand/libsa/zfs/libzfs.h Modified: head/stand/i386/common/bootargs.h ============================================================================== --- head/stand/i386/common/bootargs.h Mon Dec 3 02:38:15 2018 (r341419) +++ head/stand/i386/common/bootargs.h Mon Dec 3 03:58:30 2018 (r341420) @@ -84,11 +84,15 @@ struct bootargs #ifdef LOADER_GELI_SUPPORT #include <crypto/intake.h> +#include "geliboot.h" #endif -struct geli_boot_args +/* + * geli_boot_data is embedded in geli_boot_args (passed from gptboot to loader) + * and in zfs_boot_args (passed from zfsboot and gptzfsboot to loader). + */ +struct geli_boot_data { - uint32_t size; union { char gelipw[256]; struct { @@ -104,6 +108,49 @@ struct geli_boot_args #endif }; }; +}; + +#ifdef LOADER_GELI_SUPPORT + +static inline void +export_geli_boot_data(struct geli_boot_data *gbdata) +{ + + gbdata->notapw = '\0'; + gbdata->keybuf_sentinel = KEYBUF_SENTINEL; + gbdata->keybuf = malloc(sizeof(struct keybuf) + + (GELI_MAX_KEYS * sizeof(struct keybuf_ent))); + geli_export_key_buffer(gbdata->keybuf); +} + +static inline void +import_geli_boot_data(struct geli_boot_data *gbdata) +{ + + if (gbdata->gelipw[0] != '\0') { + setenv("kern.geom.eli.passphrase", gbdata->gelipw, 1); + explicit_bzero(gbdata->gelipw, sizeof(gbdata->gelipw)); + } else if (gbdata->keybuf_sentinel == KEYBUF_SENTINEL) { + geli_import_key_buffer(gbdata->keybuf); + } +} +#endif /* LOADER_GELI_SUPPORT */ + +struct geli_boot_args +{ + uint32_t size; + struct geli_boot_data gelidata; +}; + +struct zfs_boot_args +{ + uint32_t size; + uint32_t reserved; + uint64_t pool; + uint64_t root; + uint64_t primary_pool; + uint64_t primary_vdev; + struct geli_boot_data gelidata; }; #endif /*__ASSEMBLER__*/ Modified: head/stand/i386/gptboot/gptboot.c ============================================================================== --- head/stand/i386/gptboot/gptboot.c Mon Dec 3 02:38:15 2018 (r341419) +++ head/stand/i386/gptboot/gptboot.c Mon Dec 3 03:58:30 2018 (r341420) @@ -114,7 +114,6 @@ static int vdev_read(void *vdev __unused, void *priv, #ifdef LOADER_GELI_SUPPORT #include "geliboot.h" static char gelipw[GELI_PW_MAXLEN]; -static struct keybuf *gelibuf; #endif struct gptdsk { @@ -480,12 +479,7 @@ load(void) #ifdef LOADER_GELI_SUPPORT geliargs.size = sizeof(geliargs); explicit_bzero(gelipw, sizeof(gelipw)); - gelibuf = malloc(sizeof(struct keybuf) + - (GELI_MAX_KEYS * sizeof(struct keybuf_ent))); - geli_export_key_buffer(gelibuf); - geliargs.notapw = '\0'; - geliargs.keybuf_sentinel = KEYBUF_SENTINEL; - geliargs.keybuf = gelibuf; + export_geli_boot_data(&geliargs.gelidata); #endif /* * Note that the geliargs struct is passed by value, not by pointer. Modified: head/stand/i386/loader/main.c ============================================================================== --- head/stand/i386/loader/main.c Mon Dec 3 02:38:15 2018 (r341419) +++ head/stand/i386/loader/main.c Mon Dec 3 03:58:30 2018 (r341420) @@ -172,15 +172,8 @@ main(void) #ifdef LOADER_GELI_SUPPORT if ((kargs->bootflags & KARGS_FLAGS_EXTARG) != 0) { zargs = (struct zfs_boot_args *)(kargs + 1); - if (zargs != NULL && zargs->size >= offsetof(struct zfs_boot_args, gelipw)) { - if (zargs->size >= offsetof(struct zfs_boot_args, keybuf_sentinel) && - zargs->keybuf_sentinel == KEYBUF_SENTINEL) { - geli_import_key_buffer(zargs->keybuf); - } - if (zargs->gelipw[0] != '\0') { - setenv("kern.geom.eli.passphrase", zargs->gelipw, 1); - explicit_bzero(zargs->gelipw, sizeof(zargs->gelipw)); - } + if (zargs->size > offsetof(struct zfs_boot_args, gelidata)) { + import_geli_boot_data(&zargs->gelidata); } } #endif /* LOADER_GELI_SUPPORT */ @@ -188,14 +181,8 @@ main(void) #ifdef LOADER_GELI_SUPPORT if ((kargs->bootflags & KARGS_FLAGS_EXTARG) != 0) { gargs = (struct geli_boot_args *)(kargs + 1); - if (gargs != NULL && gargs->size >= offsetof(struct geli_boot_args, gelipw)) { - if (gargs->keybuf_sentinel == KEYBUF_SENTINEL) { - geli_import_key_buffer(gargs->keybuf); - } - if (gargs->gelipw[0] != '\0') { - setenv("kern.geom.eli.passphrase", gargs->gelipw, 1); - explicit_bzero(gargs->gelipw, sizeof(gargs->gelipw)); - } + if (gargs->size >= offsetof(struct geli_boot_args, gelidata)) { + import_geli_boot_data(&gargs->gelidata); } } #endif /* LOADER_GELI_SUPPORT */ Modified: head/stand/i386/zfsboot/zfsboot.c ============================================================================== --- head/stand/i386/zfsboot/zfsboot.c Mon Dec 3 02:38:15 2018 (r341419) +++ head/stand/i386/zfsboot/zfsboot.c Mon Dec 3 03:58:30 2018 (r341420) @@ -129,7 +129,6 @@ int main(void); #ifdef LOADER_GELI_SUPPORT #include "geliboot.h" static char gelipw[GELI_PW_MAXLEN]; -static struct keybuf *gelibuf; #endif struct zfsdsk { @@ -993,13 +992,7 @@ load(void) zfsargs.primary_pool = primary_spa->spa_guid; #ifdef LOADER_GELI_SUPPORT explicit_bzero(gelipw, sizeof(gelipw)); - gelibuf = malloc(sizeof(struct keybuf) + (GELI_MAX_KEYS * sizeof(struct keybuf_ent))); - geli_export_key_buffer(gelibuf); - zfsargs.notapw = '\0'; - zfsargs.keybuf_sentinel = KEYBUF_SENTINEL; - zfsargs.keybuf = gelibuf; -#else - zfsargs.gelipw[0] = '\0'; + export_geli_boot_data(&zfsargs.gelidata); #endif if (primary_vdev != NULL) zfsargs.primary_vdev = primary_vdev->v_guid; Modified: head/stand/libsa/zfs/libzfs.h ============================================================================== --- head/stand/libsa/zfs/libzfs.h Mon Dec 3 02:38:15 2018 (r341419) +++ head/stand/libsa/zfs/libzfs.h Mon Dec 3 03:58:30 2018 (r341420) @@ -44,31 +44,6 @@ struct zfs_devdesc { #include <crypto/intake.h> #endif -struct zfs_boot_args -{ - uint32_t size; - uint32_t reserved; - uint64_t pool; - uint64_t root; - uint64_t primary_pool; - uint64_t primary_vdev; - union { - char gelipw[256]; - struct { - char notapw; /* - * single null byte to stop keybuf - * being interpreted as a password - */ - uint32_t keybuf_sentinel; -#ifdef LOADER_GELI_SUPPORT - struct keybuf *keybuf; -#else - void *keybuf; -#endif - }; - }; -}; - int zfs_parsedev(struct zfs_devdesc *dev, const char *devspec, const char **path); char *zfs_fmtdev(void *vdev);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201812030358.wB33wUl1081824>