From aabf97f885e8a18c1e3c28ad1310e4e9b265553c Mon Sep 17 00:00:00 2001 From: Marc Hartmayer Date: Thu, 1 Jun 2023 14:26:10 +0000 Subject: [PATCH] genprotimg: refactor arguments related to the control flags into own struct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactor arguments related to the SE header control flags into own struct with the name `PvControlFlagsArgs`. This change makes it easier to extend the control flags arguments further, without touching the signature of `pv_img_set_control_flags`. While at it, rename the struct members `allow_...` to `enable_...`. This matches with the command line option names. Reviewed-by: Steffen Eiden Signed-off-by: Marc Hartmayer Signed-off-by: Jan Höppner --- genprotimg/src/pv/pv_args.c | 49 +++++++++++++++++++----------------- genprotimg/src/pv/pv_args.h | 20 +++++++++------ genprotimg/src/pv/pv_image.c | 21 ++++++---------- 3 files changed, 47 insertions(+), 43 deletions(-) diff --git a/genprotimg/src/pv/pv_args.c b/genprotimg/src/pv/pv_args.c index aa6d4228..ca06541e 100644 --- a/genprotimg/src/pv/pv_args.c +++ b/genprotimg/src/pv/pv_args.c @@ -62,11 +62,12 @@ static gint pv_args_set_defaults(PvArgs *args, GError **err G_GNUC_UNUSED) static gint pv_args_validate_options(PvArgs *args, GError **err) { + const PvControlFlagsArgs *cf_args = &args->cf_args; PvComponentType KERNEL = PV_COMP_TYPE_KERNEL; /* Check for mutually exclusive arguments */ - if (args->pcf && !(args->allow_pckmo == PV_NOT_SET && - args->allow_dump == PV_NOT_SET)) { + if (cf_args->pcf && + !(cf_args->enable_pckmo == PV_NOT_SET && cf_args->enable_dump == PV_NOT_SET)) { g_set_error( err, PV_PARSE_ERROR, PV_PARSE_ERROR_SYNTAX, _("The '--x-pcf' option cannot be used with the '--(enable|disable)-pckmo' or" @@ -93,7 +94,7 @@ static gint pv_args_validate_options(PvArgs *args, GError **err) } /* Check for mandatory arguments */ - if (args->allow_dump == PV_TRUE && !args->cust_comm_key_path) { + if (cf_args->enable_dump == PV_TRUE && !args->cust_comm_key_path) { g_set_error(err, PV_PARSE_ERROR, PR_PARSE_ERROR_MISSING_ARGUMENT, _("Option '--enable-dump' requires the '--comm-key' option.\nUse 'genprotimg " "--help' for more information")); @@ -178,11 +179,11 @@ static gboolean cb_set_string_option(const gchar *option, const gchar *value, if (g_str_equal(option, "--x-header-key")) args_option = &args->cust_root_key_path; if (g_str_equal(option, "--x-pcf")) - args_option = &args->pcf; + args_option = &args->cf_args.pcf; if (g_str_equal(option, "--x-psw")) args_option = &args->psw_addr; if (g_str_equal(option, "--x-scf")) - args_option = &args->scf; + args_option = &args->cf_args.scf; if (!args_option) { g_set_error(err, PV_PARSE_ERROR, PV_PARSE_ERROR_SYNTAX, @@ -217,20 +218,20 @@ static gboolean cb_remaining_values(const gchar *option G_GNUC_UNUSED, } #define MUT_EXCL_BOOL_FLAG_CB_NAME(FLAG, VALUE) (cb_##FLAG##_##VALUE) -#define DEFINE_MUT_EXCL_BOOL_FLAG_CB(FLAG, VALUE) \ - static gboolean MUT_EXCL_BOOL_FLAG_CB_NAME(FLAG, VALUE)( \ - const gchar *option G_GNUC_UNUSED, const gchar *value G_GNUC_UNUSED, \ - PvArgs *args, GError **err) \ - { \ - if (!(args->allow_##FLAG == PV_NOT_SET || \ - args->allow_##FLAG == VALUE)) { \ - g_set_error(err, G_OPTION_ERROR, G_OPTION_ERROR_FAILED, \ - "'--enable-" #FLAG "' and '--disable-" #FLAG \ - "' are mutually exclusive"); \ - return FALSE; \ - } \ - args->allow_##FLAG = VALUE; \ - return TRUE; \ +#define DEFINE_MUT_EXCL_BOOL_FLAG_CB(FLAG, VALUE) \ + static gboolean MUT_EXCL_BOOL_FLAG_CB_NAME(FLAG, VALUE)(const gchar *option G_GNUC_UNUSED, \ + const gchar *value G_GNUC_UNUSED, \ + PvArgs *args, GError **err) \ + { \ + if (!(args->cf_args.enable_##FLAG == PV_NOT_SET || \ + args->cf_args.enable_##FLAG == VALUE)) { \ + g_set_error(err, G_OPTION_ERROR, G_OPTION_ERROR_FAILED, \ + "'--enable-" #FLAG "' and '--disable-" #FLAG \ + "' are mutually exclusive"); \ + return FALSE; \ + } \ + args->cf_args.enable_##FLAG = VALUE; \ + return TRUE; \ } #define DEFINE_MUT_EXCL_BOOL_FLAG_CBS(FLAG) \ @@ -486,8 +487,10 @@ PvArgs *pv_args_new(void) g_autoptr(PvArgs) args = g_new0(PvArgs, 1); args->unused_values = g_ptr_array_new_with_free_func(g_free); - args->allow_dump = PV_NOT_SET; - args->allow_pckmo = PV_NOT_SET; + /* `args->cf_args` is implicitly initialized with zeros since + * `g_new0` is used. So there is no reason to explicitly + * initialize the values as PV_NOT_SET == 0. + */ return g_steal_pointer(&args); } @@ -496,8 +499,8 @@ void pv_args_free(PvArgs *args) if (!args) return; - g_free(args->pcf); - g_free(args->scf); + g_free(args->cf_args.pcf); + g_free(args->cf_args.scf); g_free(args->psw_addr); g_free(args->cust_root_key_path); g_free(args->cust_comm_key_path); diff --git a/genprotimg/src/pv/pv_args.h b/genprotimg/src/pv/pv_args.h index ef659fd8..5a46367f 100644 --- a/genprotimg/src/pv/pv_args.h +++ b/genprotimg/src/pv/pv_args.h @@ -23,19 +23,25 @@ PvArg *pv_arg_new(PvComponentType type, const gchar *path); void pv_arg_free(PvArg *arg); typedef enum pv_tristate { - PV_NOT_SET = 0, - PV_TRUE, - PV_FALSE, + PV_NOT_SET = 0, + PV_TRUE, + PV_FALSE, } PvTristate; +/* The value of PV_NOT_SET is not allowed to be changed */ +STATIC_ASSERT(PV_NOT_SET == 0) + +typedef struct { + gchar *pcf; + gchar *scf; + PvTristate enable_dump; + PvTristate enable_pckmo; +} PvControlFlagsArgs; typedef struct { gint log_level; gint no_verify; gboolean offline; - gchar *pcf; - gchar *scf; - PvTristate allow_dump; - PvTristate allow_pckmo; + PvControlFlagsArgs cf_args; gchar *psw_addr; /* PSW address which will be used for the start of * the actual component (e.g. Linux kernel) */ diff --git a/genprotimg/src/pv/pv_image.c b/genprotimg/src/pv/pv_image.c index 12219639..6c8b6e22 100644 --- a/genprotimg/src/pv/pv_image.c +++ b/genprotimg/src/pv/pv_image.c @@ -236,28 +236,25 @@ static void pv_img_set_control_flag(uint64_t *flags, const PvTristate option, co *flags &= ~flag; } -static gint pv_img_set_control_flags(PvImage *img, const gchar *pcf_s, - const gchar *scf_s, - PvTristate allow_dump, - PvTristate allow_pckmo, GError **err) +static gint pv_img_set_control_flags(PvImage *img, const PvControlFlagsArgs *cf_args, GError **err) { uint64_t flags; /* Set plain control flags */ - if (pcf_s) { - if (hex_str_toull(pcf_s, &flags, err) < 0) + if (cf_args->pcf) { + if (hex_str_toull(cf_args->pcf, &flags, err) < 0) return -1; img->pcf = flags; } - pv_img_set_control_flag(&img->pcf, allow_dump, PV_PCF_ALLOW_DUMPING); - pv_img_set_control_flag(&img->pcf, allow_pckmo, + pv_img_set_control_flag(&img->pcf, cf_args->enable_dump, PV_PCF_ALLOW_DUMPING); + pv_img_set_control_flag(&img->pcf, cf_args->enable_pckmo, PV_PCF_PCKM_ECC | PV_PCF_PCKMO_AES | PV_PCF_PCKMO_DEA_TDEA); /* Set secret control flags */ - if (scf_s) { - if (hex_str_toull(scf_s, &flags, err) < 0) + if (cf_args->scf) { + if (hex_str_toull(cf_args->scf, &flags, err) < 0) return -1; img->scf = flags; @@ -614,9 +611,7 @@ PvImage *pv_img_new(PvArgs *args, const gchar *stage3a_path, GError **err) return NULL; /* set the control flags: PCF and SCF */ - if (pv_img_set_control_flags(ret, args->pcf, args->scf, - args->allow_dump, args->allow_pckmo, - err) < 0) + if (pv_img_set_control_flags(ret, &args->cf_args, err) < 0) return NULL; /* read in the keys */