From: Simon Glass <simon.glass@canonical.com> Instead of loading and parsing included files inline during parsing, store them in cfg->includes and process them after the initial parse completes. This separates the parsing logic from file loading. Add struct pxe_include to store the path, target menu pointer and nesting level. The nesting level ensures the MAX_NEST_LEVEL (16) limit is properly enforced for deeply nested includes. Add pxe_parse_include() for callers who want manual control over include processing. The parse_pxefile() function processes includes automatically to maintain backwards compatibility with existing callers. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 38 ++++++++++----------------- boot/pxe_utils.c | 62 +++++++++++++++++++++++++++++++++++++++++---- include/pxe_utils.h | 58 +++++++++++++++++++++++++++++++++++++----- test/boot/pxe.c | 19 +++++++++----- 4 files changed, 135 insertions(+), 42 deletions(-) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index e3412166a63..72a672354b5 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -326,39 +326,31 @@ static int parse_integer(char **c, int *dst) } /* - * Parse an include statement, and retrieve and parse the file it mentions. + * Parse an include statement and store the path for later loading. * - * base should point to a location where it's safe to store the file, and - * nest_level should indicate how many nested includes have occurred. For this - * include, nest_level has already been incremented and doesn't need to be - * incremented here. + * The include is added to cfg->includes. The caller is responsible for + * loading these files and calling pxe_parse_include() to parse them. */ -static int handle_include(struct pxe_context *ctx, char **c, unsigned long base, - struct pxe_menu *cfg, int nest_level) +static int handle_include(char **c, struct pxe_menu *cfg, int nest_level) { - char *include_path; + struct pxe_include inc; char *s = *c; int err; - char *buf; - int ret; - err = parse_sliteral(c, &include_path); + err = parse_sliteral(c, &inc.path); if (err < 0) { printf("Expected include path: %.*s\n", (int)(*c - s), s); return err; } + inc.cfg = cfg; + inc.nest_level = nest_level + 1; - err = get_pxe_file(ctx, include_path, base); - if (err < 0) { - printf("Couldn't retrieve %s\n", include_path); - return err; + if (!alist_add(&cfg->includes, inc)) { + free(inc.path); + return -ENOMEM; } - buf = map_sysmem(base, 0); - ret = parse_pxefile_top(ctx, buf, base, cfg, nest_level); - unmap_sysmem(buf); - - return ret; + return 1; } /* @@ -385,7 +377,7 @@ static int parse_menu(struct pxe_context *ctx, char **c, struct pxe_menu *cfg, err = parse_sliteral(c, &cfg->title); break; case T_INCLUDE: - err = handle_include(ctx, c, base, cfg, nest_level + 1); + err = handle_include(c, cfg, nest_level); break; case T_BACKGROUND: err = parse_sliteral(c, &cfg->bmp); @@ -635,9 +627,7 @@ int parse_pxefile_top(struct pxe_context *ctx, char *p, ulong base, } break; case T_INCLUDE: - err = handle_include(ctx, &p, - base + ALIGN(strlen(b), 4), cfg, - nest_level + 1); + err = handle_include(&p, cfg, nest_level); break; case T_PROMPT: err = parse_integer(&p, &cfg->prompt); diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index e0b7faddbd0..410f2d1eafe 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -910,12 +910,14 @@ struct pxe_menu *pxe_menu_init(void) memset(cfg, '\0', sizeof(struct pxe_menu)); INIT_LIST_HEAD(&cfg->labels); + alist_init(&cfg->includes, sizeof(struct pxe_include), 0); return cfg; } void pxe_menu_uninit(struct pxe_menu *cfg) { + struct pxe_include *inc; struct list_head *pos, *n; struct pxe_label *label; @@ -929,6 +931,10 @@ void pxe_menu_uninit(struct pxe_menu *cfg) label_destroy(label); } + alist_for_each(inc, &cfg->includes) + free(inc->path); + alist_uninit(&cfg->includes); + free(cfg); } @@ -944,6 +950,12 @@ struct pxe_menu *parse_pxefile(struct pxe_context *ctx, unsigned long menucfg) buf = map_sysmem(menucfg, 0); r = parse_pxefile_top(ctx, buf, menucfg, cfg, 1); + unmap_sysmem(buf); + + if (r < 0) { + pxe_menu_uninit(cfg); + return NULL; + } if (ctx->use_fallback) { if (cfg->fallback_label) { @@ -954,13 +966,46 @@ struct pxe_menu *parse_pxefile(struct pxe_context *ctx, unsigned long menucfg) } } - unmap_sysmem(buf); - if (r < 0) { - pxe_menu_uninit(cfg); - return NULL; + return cfg; +} + +int pxe_process_includes(struct pxe_context *ctx, struct pxe_menu *cfg, + ulong base) +{ + struct pxe_include *inc; + char *buf; + uint i; + int r; + + /* + * Process includes - load each file and parse it. Get the include + * fresh each iteration since parsing may add more includes and cause + * alist reallocation. + */ + for (i = 0; i < cfg->includes.count; i++) { + inc = alist_getw(&cfg->includes, i, struct pxe_include); + + r = get_pxe_file(ctx, inc->path, base); + if (r < 0) { + printf("Couldn't retrieve %s\n", inc->path); + return r; + } + + buf = map_sysmem(base, 0); + r = pxe_parse_include(ctx, inc, buf, base); + unmap_sysmem(buf); + + if (r < 0) + return r; } - return cfg; + return 0; +} + +int pxe_parse_include(struct pxe_context *ctx, struct pxe_include *inc, + char *buf, ulong base) +{ + return parse_pxefile_top(ctx, buf, base, inc->cfg, inc->nest_level); } /* @@ -1179,6 +1224,7 @@ struct pxe_menu *pxe_prepare(struct pxe_context *ctx, ulong pxefile_addr_r, bool prompt) { struct pxe_menu *cfg; + int ret; cfg = parse_pxefile(ctx, pxefile_addr_r); if (!cfg) { @@ -1186,6 +1232,12 @@ struct pxe_menu *pxe_prepare(struct pxe_context *ctx, ulong pxefile_addr_r, return NULL; } + ret = pxe_process_includes(ctx, cfg, pxefile_addr_r); + if (ret) { + pxe_menu_uninit(cfg); + return NULL; + } + if (prompt) cfg->prompt = prompt; diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 0ea250300d6..7f5b8c040d6 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -3,6 +3,7 @@ #ifndef __PXE_UTILS_H #define __PXE_UTILS_H +#include <alist.h> #include <bootflow.h> #include <linux/list.h> @@ -68,6 +69,19 @@ struct pxe_label { struct list_head list; }; +/** + * struct pxe_include - an include file that needs to be loaded + * + * @path: Path to the include file + * @cfg: Menu to parse the include into + * @nest_level: Nesting level to use when parsing this include + */ +struct pxe_include { + char *path; + struct pxe_menu *cfg; + int nest_level; +}; + /* * Describes a pxe menu as given via pxe files. * @@ -81,6 +95,7 @@ struct pxe_label { * interrupted. If 1, always prompt for a choice regardless of * timeout. * labels - a list of labels defined for the menu. + * includes - list of struct pxe_include for files that need loading/parsing */ struct pxe_menu { char *title; @@ -90,6 +105,7 @@ struct pxe_menu { int timeout; int prompt; struct list_head labels; + struct alist includes; }; struct pxe_context; @@ -242,18 +258,48 @@ int get_pxelinux_path(struct pxe_context *ctx, const char *file, void handle_pxe_menu(struct pxe_context *ctx, struct pxe_menu *cfg); /** - * parse_pxefile() - Parsing a pxe file + * parse_pxefile() - Parse a pxe file * - * This is only used for the top-level file. + * Parse the top-level file. Any includes are stored in cfg->includes and + * should be processed by calling pxe_process_includes(). * * @ctx: PXE context (provided by the caller) - * Returns NULL if there is an error, otherwise, returns a pointer to a - * pxe_menu struct populated with the results of parsing the pxe file (and any - * files it includes). The resulting pxe_menu struct can be free()'d by using - * the pxe_menu_uninit() function. + * @menucfg: Address of the PXE file in memory + * Return: NULL on error, otherwise a pointer to a pxe_menu struct. Use + * pxe_menu_uninit() to free it. */ struct pxe_menu *parse_pxefile(struct pxe_context *ctx, ulong menucfg); +/** + * pxe_process_includes() - Process include files in a parsed menu + * + * Load and parse all include files referenced in cfg->includes. This may + * add more includes if nested includes are found. + * + * @ctx: PXE context with getfile callback + * @cfg: Parsed PXE menu with includes to process + * @base: Memory address for loading include files + * Return: 0 on success, -ve on error + */ +int pxe_process_includes(struct pxe_context *ctx, struct pxe_menu *cfg, + ulong base); + +/** + * pxe_parse_include() - Parse an included file into its target menu + * + * After loading an include file referenced in cfg->includes, call this + * to parse it and merge any labels into the target menu. This may add + * more entries to cfg->includes if the included file has its own includes. + * + * @ctx: PXE context + * @inc: Include info with path and target menu + * @buf: Buffer containing the included file content + * @base: Memory address where buf is located + * Return: 1 on success, -ve on error + */ +int pxe_parse_include(struct pxe_context *ctx, struct pxe_include *inc, + char *buf, ulong base); + /** * format_mac_pxe() - Convert a MAC address to PXE format * diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 8d7c6f7f2ae..365aff9f37a 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -164,6 +164,9 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) cfg = parse_pxefile(&ctx, addr); ut_assertnonnull(cfg); + /* Process any include files */ + ut_assertok(pxe_process_includes(&ctx, cfg, addr)); + /* Verify no console output during parsing (say is printed on boot) */ ut_assert_console_end(); @@ -621,7 +624,6 @@ static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts) struct pxe_menu *cfg; ulong addr = PXE_LOAD_ADDR; void *fdt; - uint i; ut_assertnonnull(fs_image); ut_assertnonnull(cfg_path); @@ -635,17 +637,17 @@ static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts) ut_assertok(pxe_setup_ctx(&ctx, pxe_test_getfile, &info, true, cfg_path, false, false, NULL)); - /* Read and parse the config file */ + /* Read and parse the config file (quiet since we're just parsing) */ + ctx.quiet = true; ut_asserteq(1, get_pxe_file(&ctx, cfg_path, addr)); cfg = parse_pxefile(&ctx, addr); ut_assertnonnull(cfg); - /* Consume parsing output (say message is printed on boot, not parsing) */ - ut_assert_nextline("Retrieving file: %s", cfg_path); - ut_assert_nextline("Retrieving file: /extlinux/extra.conf"); - for (i = 3; i <= 16; i++) - ut_assert_nextline("Retrieving file: /extlinux/nest%d.conf", i); + /* Process any include files */ + ut_assertok(pxe_process_includes(&ctx, cfg, addr)); + + /* Verify no console output during parsing (say is printed on boot) */ ut_assert_console_end(); /* @@ -662,6 +664,9 @@ static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts) ut_asserteq_str("linux", label->name); ut_assertnonnull(label->fdtoverlays); + /* Enable output for loading phase */ + ctx.quiet = false; + /* Load the label - should succeed but skip overlays */ ut_assertok(pxe_load_label(&ctx, label)); -- 2.43.0