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.
This commit is contained in:
ian 2018-12-04 16:43:50 +00:00
parent 0d01acf0ac
commit 6b63037a39
3 changed files with 45 additions and 25 deletions

View File

@ -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) */

View File

@ -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

View File

@ -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,25 +170,47 @@ 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 */
#ifdef LOADER_GELI_SUPPORT
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);
}
}
#endif /* LOADER_GELI_SUPPORT */
#endif /* LOADER_ZFS_SUPPORT */
#ifdef LOADER_GELI_SUPPORT
/*
* 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 ((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 */
/*
* 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 */