Date: Tue, 4 Dec 2018 16:43:50 +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: r341473 - in head/stand/i386: common gptboot loader Message-ID: <201812041643.wB4GhoSV025590@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: ian Date: Tue Dec 4 16:43:50 2018 New Revision: 341473 URL: https://svnweb.freebsd.org/changeset/base/341473 Log: Fix args cross-threading between gptboot(8) and loader(8) with zfs support. When loader(8) is built with zfs support enabled, it assumes that any extarg data present is a zfs_boot_args struct, but if the first-stage loader was gptboot(8) the extarg data is actually a geli_boot_args struct. Luckily, zfsboot(8) and gptzfsboot(8) have always passed KARGS_FLAGS_ZFS along with KARGS_FLAGS_EXTARG, so we can use KARGS_FLAGS_ZFS to decide whether the extarg data is a zfs_boot_args struct. To avoid similar problems in the future, gptboot(8) now passes a new KARGS_FLAGS_GELI to indicate that extarg data is geli_boot_args. In loader(8), if the neither KARGS_FLAGS_ZFS nor KARGS_FLAGS_GELI is set but extarg data is present (which will be the case for gptboot compiled before this change), we now check for the known size of the geli_boot_args struct passed by the older versions of gptboot as a way of confirming what type of extarg data is present. In a semi-related tidying up, since loader's main() has already decided what type of extarg data is present and set the global 'zargs' var accordingly, don't repeat the check in extract_currdev, just check whether zargs is NULL or not. X-MFC after: a few days, along with prior related changes. Modified: head/stand/i386/common/bootargs.h head/stand/i386/gptboot/gptboot.c head/stand/i386/loader/main.c Modified: head/stand/i386/common/bootargs.h ============================================================================== --- head/stand/i386/common/bootargs.h Tue Dec 4 16:12:43 2018 (r341472) +++ head/stand/i386/common/bootargs.h Tue Dec 4 16:43:50 2018 (r341473) @@ -18,10 +18,11 @@ #ifndef _BOOT_I386_ARGS_H_ #define _BOOT_I386_ARGS_H_ -#define KARGS_FLAGS_CD 0x1 -#define KARGS_FLAGS_PXE 0x2 -#define KARGS_FLAGS_ZFS 0x4 -#define KARGS_FLAGS_EXTARG 0x8 /* variably sized extended argument */ +#define KARGS_FLAGS_CD 0x0001 /* .bootdev is a bios CD dev */ +#define KARGS_FLAGS_PXE 0x0002 /* .pxeinfo is valid */ +#define KARGS_FLAGS_ZFS 0x0004 /* .zfspool is valid, EXTARG is zfs_boot_args */ +#define KARGS_FLAGS_EXTARG 0x0008 /* variably sized extended argument */ +#define KARGS_FLAGS_GELI 0x0010 /* EXTARG is geli_boot_args */ #define BOOTARGS_SIZE 24 /* sizeof(struct bootargs) */ #define BA_BOOTFLAGS 8 /* offsetof(struct bootargs, bootflags) */ Modified: head/stand/i386/gptboot/gptboot.c ============================================================================== --- head/stand/i386/gptboot/gptboot.c Tue Dec 4 16:12:43 2018 (r341472) +++ head/stand/i386/gptboot/gptboot.c Tue Dec 4 16:43:50 2018 (r341473) @@ -490,7 +490,7 @@ load(void) __exec((caddr_t)addr, RB_BOOTINFO | (opts & RBX_MASK), MAKEBOOTDEV(dev_maj[gdsk.dsk.type], gdsk.dsk.part + 1, gdsk.dsk.unit, 0xff), #ifdef LOADER_GELI_SUPPORT - KARGS_FLAGS_EXTARG, 0, 0, VTOP(&bootinfo), geliargs + KARGS_FLAGS_GELI | KARGS_FLAGS_EXTARG, 0, 0, VTOP(&bootinfo), geliargs #else 0, 0, 0, VTOP(&bootinfo) #endif Modified: head/stand/i386/loader/main.c ============================================================================== --- head/stand/i386/loader/main.c Tue Dec 4 16:12:43 2018 (r341472) +++ head/stand/i386/loader/main.c Tue Dec 4 16:43:50 2018 (r341473) @@ -73,6 +73,7 @@ void exit(int code); #ifdef LOADER_GELI_SUPPORT #include "geliboot.h" struct geli_boot_args *gargs; +struct geli_boot_data *gbdata; #endif #ifdef LOADER_ZFS_SUPPORT struct zfs_boot_args *zargs; @@ -169,24 +170,46 @@ main(void) #ifdef LOADER_ZFS_SUPPORT archsw.arch_zfs_probe = i386_zfs_probe; -#ifdef LOADER_GELI_SUPPORT - if ((kargs->bootflags & KARGS_FLAGS_EXTARG) != 0) { + /* + * zfsboot and gptzfsboot have always passed KARGS_FLAGS_ZFS, so if that is + * set along with KARGS_FLAGS_EXTARG we know we can interpret the extarg + * data as a struct zfs_boot_args. + */ +#define KARGS_EXTARGS_ZFS (KARGS_FLAGS_EXTARG | KARGS_FLAGS_ZFS) + + if ((kargs->bootflags & KARGS_EXTARGS_ZFS) == KARGS_EXTARGS_ZFS) { zargs = (struct zfs_boot_args *)(kargs + 1); - if (zargs->size > offsetof(struct zfs_boot_args, gelidata)) { - import_geli_boot_data(&zargs->gelidata); - } } -#endif /* LOADER_GELI_SUPPORT */ -#else /* !LOADER_ZFS_SUPPORT */ +#endif /* LOADER_ZFS_SUPPORT */ + #ifdef LOADER_GELI_SUPPORT - if ((kargs->bootflags & KARGS_FLAGS_EXTARG) != 0) { + /* + * If we decided earlier that we have zfs_boot_args extarg data, and it is + * big enough to contain the embedded geli data (the early zfs_boot_args + * structs weren't), then init the gbdata pointer accordingly. If there is + * extarg data which isn't zfs_boot_args data, determine whether it is + * geli_boot_args data. Recent versions of gptboot set KARGS_FLAGS_GELI to + * indicate that. Earlier versions didn't, but we presume that's what we + * have if the extarg size exactly matches the size of the geli_boot_args + * struct during that pre-flag era. + */ +#define LEGACY_GELI_ARGS_SIZE 260 /* This can never change */ + + if (zargs != NULL) { + if (zargs->size > offsetof(struct zfs_boot_args, gelidata)) { + gbdata = &zargs->gelidata; + } + } else if ((kargs->bootflags & KARGS_FLAGS_EXTARG) != 0) { gargs = (struct geli_boot_args *)(kargs + 1); - if (gargs->size >= offsetof(struct geli_boot_args, gelidata)) { - import_geli_boot_data(&gargs->gelidata); + if ((kargs->bootflags & KARGS_FLAGS_GELI) || + gargs->size == LEGACY_GELI_ARGS_SIZE) { + gbdata = &gargs->gelidata; } } + + if (gbdata != NULL) + import_geli_boot_data(gbdata); #endif /* LOADER_GELI_SUPPORT */ -#endif /* LOADER_ZFS_SUPPORT */ /* * March through the device switch probing for things. @@ -258,11 +281,7 @@ extract_currdev(void) } #ifdef LOADER_ZFS_SUPPORT } else if ((kargs->bootflags & KARGS_FLAGS_ZFS) != 0) { - zargs = NULL; - /* check for new style extended argument */ - if ((kargs->bootflags & KARGS_FLAGS_EXTARG) != 0) - zargs = (struct zfs_boot_args *)(kargs + 1); - + /* zargs was set in main() if we have new style extended argument */ if (zargs != NULL && zargs->size >= offsetof(struct zfs_boot_args, primary_pool)) { /* sufficient data is provided */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201812041643.wB4GhoSV025590>