[PATCH 00/26] boot: pxe: Add three-phase API and fix memory leaks
From: Simon Glass <simon.glass@canonical.com> This series adds a callback-free API for PXE/extlinux parsing that eparates the process into three distinct phases: parse, load, and boot. The traditional API uses callbacks during parsing to load files immediately. The new API instead collects file information (kernel, initrd, FDT, overlays) into a list during parsing, allowing callers to load files manually before booting. Include files are handled in a similar way. Rather than requiring the config files to be nul-termiated, the size of the file is provided and used to determine the end of the configuration file. New functions: - pxe_parse() - Parse config and return menu with file list - pxe_load() - Load files for a selected label (optional helper) - pxe_boot() - Boot using loaded files (renamed from pxe_do_boot) - pxe_cleanup() - Clean up context and menu The series also fixes several memory leaks in the parser found through malloc dump analysis. Size growth is unfortunately significant. On Thumb2 the memory-leak fixes add 116 bytes; removing the need for nul-termination takes 116 bytes and the alists and and general logic take another 60 Simon Glass (26): lib: abuf: Add abuf_init_addr() helper boot: pxe: Free bmp in pxe_menu_uninit() boot: pxe: Fix token memory leaks in parser boot: pxe: Free menu label in label_destroy() boot: pxe: Fix memory leak in parse_fdtoverlays() boot: pxe: Make pxe_prepare() static and document it boot: pxe: Simplify pxe_parse_include() to take only address boot: pxe: Add struct pxe_file for file loading boot: pxe: Replace pxe_fdtoverlay with pxe_file boot: pxe: Use the files list instead of fdtoverlays boot: pxe: Add a helper to add to the file list boot: pxe: Add the kernel to the files list when parsing boot: pxe: Add the initrd to the files list when parsing boot: pxe: Add the FDT to the files list when parsing boot: pxe: Set pxe_file_size in get_pxe_file() boot: pxe: Set pxe_file_size in extlinux_read_all() boot: pxe: Add size parameter to parse_pxefile() boot: pxe: Add pxe_process_str() for nul-terminated strings boot: pxe: Update parse_pxefile() to take struct abuf boot: pxe: Remove unused base parameter from parse_menu() boot: pxe: Remove unused base parameter from parse_pxefile_top() boot: pxe: Add a limit parameter to parser functions boot: pxe: Add a callback-free PXE API test: pxe: Add a test for the callback-free files API test: pxe: Add memory-leak check to files-API test doc: bootstd: Add PXE parser API documentation boot/ext_pxe_common.c | 5 +- boot/pxe_parse.c | 242 +++++++++++++++++++---------- boot/pxe_utils.c | 127 ++++++++++++--- cmd/pxe.c | 2 +- cmd/sysboot.c | 2 +- doc/develop/bootstd/extlinux.rst | 2 +- doc/develop/bootstd/index.rst | 1 + doc/develop/bootstd/pxe_api.rst | 194 +++++++++++++++++++++++ doc/develop/bootstd/pxelinux.rst | 2 +- include/abuf.h | 12 ++ include/pxe_utils.h | 176 ++++++++++++++++----- lib/abuf.c | 6 + test/boot/pxe.c | 255 +++++++++++++++++++++++++++---- test/lib/abuf.c | 23 +++ test/py/tests/test_pxe_parser.py | 11 ++ 15 files changed, 876 insertions(+), 184 deletions(-) create mode 100644 doc/develop/bootstd/pxe_api.rst -- 2.43.0 base-commit: c0b7ed1ad434eabf50bc8bdf7ef7f31423f4977d branch: fite
From: Simon Glass <simon.glass@canonical.com> Add a helper function that combines abuf_init() and abuf_map_sysmem() into a single call. This simplifies code that needs to set up an abuf from a memory address. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- include/abuf.h | 12 ++++++++++++ lib/abuf.c | 6 ++++++ test/lib/abuf.c | 23 +++++++++++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/include/abuf.h b/include/abuf.h index 804ab0e3e57..42a80819d82 100644 --- a/include/abuf.h +++ b/include/abuf.h @@ -214,6 +214,18 @@ void abuf_init_const(struct abuf *abuf, const void *data, size_t size); */ void abuf_init_const_addr(struct abuf *abuf, ulong addr, size_t size); +/** + * abuf_init_addr() - Set up a new buffer at a given address + * + * This is similar to abuf_init_const_addr() but uses abuf_map_sysmem() + * internally. Note that the abuf is unallocated. + * + * @abuf: abuf to set up + * @addr: Address to use + * @size: Size of buffer + */ +void abuf_init_addr(struct abuf *abuf, ulong addr, size_t size); + /** * abuf_init_size() - Set up an allocated abuf * diff --git a/lib/abuf.c b/lib/abuf.c index 440f6a40023..f283f8e6e69 100644 --- a/lib/abuf.c +++ b/lib/abuf.c @@ -188,6 +188,12 @@ void abuf_init_const_addr(struct abuf *abuf, ulong addr, size_t size) { return abuf_init_const(abuf, map_sysmem(addr, size), size); } + +void abuf_init_addr(struct abuf *abuf, ulong addr, size_t size) +{ + abuf_init(abuf); + abuf_map_sysmem(abuf, addr, size); +} #endif void abuf_init_move(struct abuf *abuf, void *data, size_t size) diff --git a/test/lib/abuf.c b/test/lib/abuf.c index e97bb8b66bc..e6032998e94 100644 --- a/test/lib/abuf.c +++ b/test/lib/abuf.c @@ -91,6 +91,29 @@ static int lib_test_abuf_init_const_addr(struct unit_test_state *uts) } LIB_TEST(lib_test_abuf_init_const_addr, 0); +/* Test abuf_init_addr() */ +static int lib_test_abuf_init_addr(struct unit_test_state *uts) +{ + struct abuf buf; + ulong start; + void *ptr; + + start = ut_check_free(); + + ptr = map_sysmem(0x100, 0); + + abuf_init_addr(&buf, 0x100, 10); + ut_asserteq_ptr(ptr, buf.data); + ut_asserteq(10, buf.size); + ut_asserteq(false, buf.alloced); + + /* No memory should have been allocated */ + ut_assertok(ut_check_delta(start)); + + return 0; +} +LIB_TEST(lib_test_abuf_init_addr, 0); + /* Test abuf_map_sysmem() and abuf_addr() */ static int lib_test_abuf_map_sysmem(struct unit_test_state *uts) { -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The bmp field of pxe_menu is allocated by parse_sliteral() in parse_menu() but is never freed. Add the missing free() call in pxe_menu_uninit() to prevent a memory leak. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 1 + 1 file changed, 1 insertion(+) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 52032d683c3..9444effbb6a 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -985,6 +985,7 @@ void pxe_menu_uninit(struct pxe_menu *cfg) free(cfg->title); free(cfg->default_label); free(cfg->fallback_label); + free(cfg->bmp); list_for_each_safe(pos, n, &cfg->labels) { label = list_entry(pos, struct pxe_label, list); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The get_token() function allocates memory for t.val when parsing keywords and string literals, but callers do not free this memory. Fix this by: - Initialising t.val to NULL in get_token() so free() is safe for T_EOL/T_EOF tokens which do not allocate - Adding free(t.val) calls in parse_menu(), parse_label_menu(), parse_label(), and parse_pxefile_top() after token processing Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index 6d148756b0d..8c720db5e75 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -232,6 +232,7 @@ static void get_token(char **p, struct token *t, enum lex_state state) char *c = *p; t->type = T_INVALID; + t->val = NULL; /* eat non EOL whitespace */ while (isblank(*c)) @@ -418,6 +419,7 @@ static int parse_menu(struct pxe_context *ctx, char **c, struct pxe_menu *cfg, char *s = *c; int err = 0; + t.val = NULL; get_token(c, &t, L_KEYWORD); switch (t.type) { @@ -434,6 +436,7 @@ static int parse_menu(struct pxe_context *ctx, char **c, struct pxe_menu *cfg, printf("Ignoring malformed menu command: %.*s\n", (int)(*c - s), s); } + free(t.val); if (err < 0) return err; @@ -452,6 +455,7 @@ static int parse_label_menu(char **c, struct pxe_menu *cfg, char *s; s = *c; + t.val = NULL; get_token(c, &t, L_KEYWORD); switch (t.type) { @@ -471,6 +475,7 @@ static int parse_label_menu(char **c, struct pxe_menu *cfg, (int)(*c - s), s); } + free(t.val); eol_or_eof(c); return 0; @@ -534,8 +539,10 @@ static int parse_label(char **c, struct pxe_menu *cfg) } list_add_tail(&label->list, &cfg->labels); + t.val = NULL; while (1) { s = *c; + free(t.val); get_token(c, &t, L_KEYWORD); err = 0; @@ -595,8 +602,10 @@ static int parse_label(char **c, struct pxe_menu *cfg) if (p) { label->say = strndup(*c + 1, p - *c - 1); - if (!label->say) + if (!label->say) { + free(t.val); return -ENOMEM; + } *c = p; } break; @@ -608,11 +617,14 @@ static int parse_label(char **c, struct pxe_menu *cfg) * something for the menu level context to handle. */ *c = s; + free(t.val); return 1; } - if (err < 0) + if (err < 0) { + free(t.val); return err; + } } } @@ -637,8 +649,10 @@ int parse_pxefile_top(struct pxe_context *ctx, char *p, ulong base, return -EMLINK; } + t.val = NULL; while (1) { s = p; + free(t.val); get_token(&p, &t, L_KEYWORD); err = 0; @@ -686,6 +700,7 @@ int parse_pxefile_top(struct pxe_context *ctx, char *p, ulong base, case T_EOL: break; case T_EOF: + free(t.val); return 1; default: printf("Ignoring unknown command: %.*s\n", @@ -693,7 +708,9 @@ int parse_pxefile_top(struct pxe_context *ctx, char *p, ulong base, eol_or_eof(&p); } - if (err < 0) + if (err < 0) { + free(t.val); return err; + } } } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The menu field of pxe_label is allocated by parse_sliteral() in parse_label_menu() but is never freed. Add the missing free() call in label_destroy() to prevent a memory leak. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 1 + 1 file changed, 1 insertion(+) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index 8c720db5e75..e5af05d4120 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -116,6 +116,7 @@ void label_destroy(struct pxe_label *label) struct pxe_fdtoverlay *overlay; free(label->name); + free(label->menu); free(label->kernel_label); free(label->kernel); free(label->config); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The parse_sliteral() call allocates a string for val, but it is never freed after parsing the overlay paths. Each path is duplicated by strndup/strdup, so the original string can be freed. Save the original pointer and free it on exit and error paths. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index e5af05d4120..f6b7887f603 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -317,12 +317,13 @@ static int parse_sliteral(char **c, char **dst) */ static int parse_fdtoverlays(char **c, struct alist *overlays) { - char *val; + char *val, *start; int err; err = parse_sliteral(c, &val); if (err < 0) return err; + start = val; while (*val) { struct pxe_fdtoverlay item; @@ -348,10 +349,13 @@ static int parse_fdtoverlays(char **c, struct alist *overlays) if (!item.path || !alist_add(overlays, item)) { free(item.path); + free(start); return -ENOMEM; } } + free(start); + return 1; } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> This function is only used within pxe_utils.c so make it static. Add documentation while we are here. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 9444effbb6a..b55d736beb8 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -1282,8 +1282,18 @@ void pxe_destroy_ctx(struct pxe_context *ctx) free(ctx->bootdir); } -struct pxe_menu *pxe_prepare(struct pxe_context *ctx, ulong pxefile_addr_r, - bool prompt) +/** + * pxe_prepare() - Prepare a PXE menu by parsing and processing includes + * + * Parses the PXE config file and processes any include directives. + * + * @ctx: PXE context + * @pxefile_addr_r: Address where config file is loaded + * @prompt: true to force the menu prompt + * Return: Parsed menu on success, NULL on error + */ +static struct pxe_menu *pxe_prepare(struct pxe_context *ctx, + ulong pxefile_addr_r, bool prompt) { struct pxe_menu *cfg; int ret; -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Change pxe_parse_include() to take just the memory address instead of both a buffer pointer and address. The function now calls map_sysmem() internally to get the buffer, simplifying the caller interface. Move the function to the bottom of the header file while we are here, since it will form part of the new API. Also mark 'inc' const since it is not used. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 18 +++++++++++------- include/pxe_utils.h | 31 +++++++++++++++---------------- 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index b55d736beb8..384a4e4f8ab 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -1035,7 +1035,6 @@ int pxe_process_includes(struct pxe_context *ctx, struct pxe_menu *cfg, ulong base) { struct pxe_include *inc; - char *buf; uint i; int r; @@ -1053,9 +1052,7 @@ int pxe_process_includes(struct pxe_context *ctx, struct pxe_menu *cfg, return r; } - buf = map_sysmem(base, 0); - r = pxe_parse_include(ctx, inc, buf, base); - unmap_sysmem(buf); + r = pxe_parse_include(ctx, inc, base); if (r < 0) return r; @@ -1064,10 +1061,17 @@ int pxe_process_includes(struct pxe_context *ctx, struct pxe_menu *cfg, return 0; } -int pxe_parse_include(struct pxe_context *ctx, struct pxe_include *inc, - char *buf, ulong base) +int pxe_parse_include(struct pxe_context *ctx, const struct pxe_include *inc, + ulong addr) { - return parse_pxefile_top(ctx, buf, base, inc->cfg, inc->nest_level); + char *buf; + int ret; + + buf = map_sysmem(addr, 0); + ret = parse_pxefile_top(ctx, buf, addr, inc->cfg, inc->nest_level); + unmap_sysmem(buf); + + return ret; } /* diff --git a/include/pxe_utils.h b/include/pxe_utils.h index a03e00e2a4a..f6ee0417a9b 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -299,22 +299,6 @@ struct pxe_menu *parse_pxefile(struct pxe_context *ctx, ulong menucfg); 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 * @@ -512,4 +496,19 @@ int parse_pxefile_top(struct pxe_context *ctx, char *p, ulong base, */ void label_destroy(struct pxe_label *label); +/** + * 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 + * @addr: Memory address where file is located + * Return: 1 on success, -ve on error + */ +int pxe_parse_include(struct pxe_context *ctx, const struct pxe_include *inc, + ulong addr); + #endif /* __PXE_UTILS_H */ -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a struct to hold information about files that need to be loaded for PXE boot. This allows the caller to load files without using callbacks, similar to how include files are handled. The new struct pxe_file contains: - path: file path to load - type: file type (kernel, initrd, fdt, fdtoverlay) - addr: address where file was loaded (filled by caller) - size: size of loaded file (filled by caller) Also add a files alist to pxe_label to hold the list of files needed for booting that label. For now this is unused. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- include/pxe_utils.h | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/include/pxe_utils.h b/include/pxe_utils.h index f6ee0417a9b..2f194b91683 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -51,6 +51,7 @@ struct pxe_fdtoverlay { * @fdt: path to FDT to use * @fdtdir: path to FDT directory to use * @fdtoverlays: list of FDT overlays to apply (alist of struct pxe_fdtoverlay) + * @files: list of files to load (alist of struct pxe_file) * @say: message to print when this label is selected for booting * @ipappend: flags for appending IP address (0x1) and MAC address (0x3) * @attempted: 0 if we haven't tried to boot this label, 1 if we have @@ -71,6 +72,7 @@ struct pxe_label { char *fdt; char *fdtdir; struct alist fdtoverlays; + struct alist files; char *say; int ipappend; int attempted; @@ -93,6 +95,36 @@ struct pxe_include { int nest_level; }; +/** + * enum pxe_file_type_t - type of file to load for PXE boot + * + * @PFT_KERNEL: Kernel image + * @PFT_INITRD: Initial ramdisk + * @PFT_FDT: Flattened device tree + * @PFT_FDTOVERLAY: Device tree overlay + */ +enum pxe_file_type_t { + PFT_KERNEL, + PFT_INITRD, + PFT_FDT, + PFT_FDTOVERLAY, +}; + +/** + * struct pxe_file - a file that needs to be loaded + * + * @path: Path to the file + * @type: Type of file (kernel, initrd, etc.) + * @addr: Address where file was loaded (filled by caller) + * @size: Size of loaded file (filled by caller) + */ +struct pxe_file { + char *path; + enum pxe_file_type_t type; + ulong addr; + ulong size; +}; + /* * Describes a pxe menu as given via pxe files. * -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The new struct pxe_file provides a more general way to describe files that need to be loaded. Replace the FDT-overlay-specific struct with the new general-purpose one, setting the type field to PFT_FDTOVERLAY. This change prepares for moving away from callback-based file loading to a model where PXE returns a list of files to load and the caller handles the actual loading. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 8 +++++--- boot/pxe_utils.c | 4 ++-- include/pxe_utils.h | 13 +------------ test/boot/pxe.c | 12 ++++++------ 4 files changed, 14 insertions(+), 23 deletions(-) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index f6b7887f603..29bee4d908c 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -106,14 +106,14 @@ static struct pxe_label *label_create(void) if (!label) return NULL; memset(label, 0, sizeof(struct pxe_label)); - alist_init_struct(&label->fdtoverlays, struct pxe_fdtoverlay); + alist_init_struct(&label->fdtoverlays, struct pxe_file); return label; } void label_destroy(struct pxe_label *label) { - struct pxe_fdtoverlay *overlay; + struct pxe_file *overlay; free(label->name); free(label->menu); @@ -326,7 +326,7 @@ static int parse_fdtoverlays(char **c, struct alist *overlays) start = val; while (*val) { - struct pxe_fdtoverlay item; + struct pxe_file item; char *end; /* Skip leading spaces */ @@ -345,7 +345,9 @@ static int parse_fdtoverlays(char **c, struct alist *overlays) item.path = strdup(val); val += strlen(val); } + item.type = PFT_FDTOVERLAY; item.addr = 0; + item.size = 0; if (!item.path || !alist_add(overlays, item)) { free(item.path); diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 384a4e4f8ab..b4bcba422b8 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -308,7 +308,7 @@ static void label_boot_kaslrseed(struct pxe_context *ctx) static void label_load_fdtoverlays(struct pxe_context *ctx, struct pxe_label *label) { - struct pxe_fdtoverlay *overlay; + struct pxe_file *overlay; ulong fdtoverlay_addr; bool use_lmb; char *envaddr; @@ -357,7 +357,7 @@ static void label_load_fdtoverlays(struct pxe_context *ctx, static void label_apply_fdtoverlays(struct pxe_context *ctx, struct pxe_label *label) { - struct pxe_fdtoverlay *overlay; + struct pxe_file *overlay; struct fdt_header *blob; int err; diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 2f194b91683..75437885dd3 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -24,17 +24,6 @@ * take a 'include file getter' function. */ -/** - * struct pxe_fdtoverlay - info about an FDT overlay - * - * @path: Path to the overlay file - * @addr: Address where the overlay was loaded (0 if not yet loaded) - */ -struct pxe_fdtoverlay { - char *path; - ulong addr; -}; - /** * struct pxe_label - describes a single label in a pxe file * @@ -50,7 +39,7 @@ struct pxe_fdtoverlay { * @initrd: path to the initrd to use for this label. * @fdt: path to FDT to use * @fdtdir: path to FDT directory to use - * @fdtoverlays: list of FDT overlays to apply (alist of struct pxe_fdtoverlay) + * @fdtoverlays: list of FDT overlays to apply (alist of struct pxe_file) * @files: list of files to load (alist of struct pxe_file) * @say: message to print when this label is selected for booting * @ipappend: flags for appending IP address (0x1) and MAC address (0x3) diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 97049e27ab6..260fc918592 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -192,9 +192,9 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assertnull(label->fdtdir); ut_asserteq(2, label->fdtoverlays.count); ut_asserteq_str("/dtb/overlay1.dtbo", - alist_get(&label->fdtoverlays, 0, struct pxe_fdtoverlay)->path); + alist_get(&label->fdtoverlays, 0, struct pxe_file)->path); ut_asserteq_str("/dtb/overlay2.dtbo", - alist_get(&label->fdtoverlays, 1, struct pxe_fdtoverlay)->path); + alist_get(&label->fdtoverlays, 1, struct pxe_file)->path); ut_asserteq_str("Booting default Linux kernel", label->say); ut_asserteq(0, label->ipappend); ut_asserteq(0, label->attempted); @@ -317,13 +317,13 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) /* Verify overlays were loaded to valid addresses */ ut_assert(alist_get(&label->fdtoverlays, 0, - struct pxe_fdtoverlay)->addr >= PXE_OVERLAY_ADDR); + struct pxe_file)->addr >= PXE_OVERLAY_ADDR); ut_assert(alist_get(&label->fdtoverlays, 1, - struct pxe_fdtoverlay)->addr >= PXE_OVERLAY_ADDR); + struct pxe_file)->addr >= PXE_OVERLAY_ADDR); /* Second overlay should be at a higher address than the first */ - ut_assert(alist_get(&label->fdtoverlays, 1, struct pxe_fdtoverlay)->addr > - alist_get(&label->fdtoverlays, 0, struct pxe_fdtoverlay)->addr); + ut_assert(alist_get(&label->fdtoverlays, 1, struct pxe_file)->addr > + alist_get(&label->fdtoverlays, 0, struct pxe_file)->addr); /* Verify no more console output */ ut_assert_console_end(); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Use the new files list for handling the FDT overlays, using the type PFT_FDTOVERLAY. Add a helper function to check if any overlays exist in the list, to preserve the existing behaviour of only adding new overlays if not done already. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 33 ++++++++++++++++++++++++--------- boot/pxe_utils.c | 13 ++++++++----- include/pxe_utils.h | 2 -- test/boot/pxe.c | 26 +++++++++++++------------- 4 files changed, 45 insertions(+), 29 deletions(-) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index 29bee4d908c..b516da8f759 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -106,14 +106,14 @@ static struct pxe_label *label_create(void) if (!label) return NULL; memset(label, 0, sizeof(struct pxe_label)); - alist_init_struct(&label->fdtoverlays, struct pxe_file); + alist_init_struct(&label->files, struct pxe_file); return label; } void label_destroy(struct pxe_label *label) { - struct pxe_file *overlay; + struct pxe_file *file; free(label->name); free(label->menu); @@ -124,9 +124,9 @@ void label_destroy(struct pxe_label *label) free(label->initrd); free(label->fdt); free(label->fdtdir); - alist_for_each(overlay, &label->fdtoverlays) - free(overlay->path); - alist_uninit(&label->fdtoverlays); + alist_for_each(file, &label->files) + free(file->path); + alist_uninit(&label->files); free(label->say); free(label); } @@ -312,10 +312,25 @@ static int parse_sliteral(char **c, char **dst) return 1; } +/* + * Check if a files list contains any FDT overlays. + */ +static bool has_fdtoverlays(struct alist *files) +{ + struct pxe_file *file; + + alist_for_each(file, files) { + if (file->type == PFT_FDTOVERLAY) + return true; + } + + return false; +} + /* * Parse a space-separated list of overlay paths into an alist. */ -static int parse_fdtoverlays(char **c, struct alist *overlays) +static int parse_fdtoverlays(char **c, struct alist *files) { char *val, *start; int err; @@ -349,7 +364,7 @@ static int parse_fdtoverlays(char **c, struct alist *overlays) item.addr = 0; item.size = 0; - if (!item.path || !alist_add(overlays, item)) { + if (!item.path || !alist_add(files, item)) { free(item.path); free(start); return -ENOMEM; @@ -589,8 +604,8 @@ static int parse_label(char **c, struct pxe_menu *cfg) err = parse_sliteral(c, &label->fdtdir); break; case T_FDTOVERLAYS: - if (!label->fdtoverlays.count) - err = parse_fdtoverlays(c, &label->fdtoverlays); + if (!has_fdtoverlays(&label->files)) + err = parse_fdtoverlays(c, &label->files); break; case T_LOCALBOOT: label->localboot = 1; diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index b4bcba422b8..a2a2b986de1 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -327,11 +327,14 @@ static void label_load_fdtoverlays(struct pxe_context *ctx, use_lmb = true; } - alist_for_each(overlay, &label->fdtoverlays) { + alist_for_each(overlay, &label->files) { ulong addr = fdtoverlay_addr; ulong size; int err; + if (overlay->type != PFT_FDTOVERLAY) + continue; + err = get_relfile(ctx, overlay->path, &addr, SZ_4K, (enum bootflow_img_t)IH_TYPE_FLATDT, &size); if (err < 0) { @@ -368,8 +371,8 @@ static void label_apply_fdtoverlays(struct pxe_context *ctx, /* Resize main fdt to make room for overlays */ fdt_shrink_to_minimum(ctx->fdt, 8192); - alist_for_each(overlay, &label->fdtoverlays) { - if (!overlay->addr) + alist_for_each(overlay, &label->files) { + if (overlay->type != PFT_FDTOVERLAY || !overlay->addr) continue; blob = map_sysmem(overlay->addr, 0); @@ -515,7 +518,7 @@ static int label_process_fdt(struct pxe_context *ctx, struct pxe_label *label) if (label->kaslrseed) label_boot_kaslrseed(ctx); - if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY) && label->fdtoverlays.count) + if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY) && label->files.count) label_apply_fdtoverlays(ctx, label); return 0; @@ -679,7 +682,7 @@ int pxe_load_files(struct pxe_context *ctx, struct pxe_label *label, } } - if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY) && label->fdtoverlays.count) + if (IS_ENABLED(CONFIG_OF_LIBFDT_OVERLAY) && label->files.count) label_load_fdtoverlays(ctx, label); return 0; diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 75437885dd3..266204b97ef 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -39,7 +39,6 @@ * @initrd: path to the initrd to use for this label. * @fdt: path to FDT to use * @fdtdir: path to FDT directory to use - * @fdtoverlays: list of FDT overlays to apply (alist of struct pxe_file) * @files: list of files to load (alist of struct pxe_file) * @say: message to print when this label is selected for booting * @ipappend: flags for appending IP address (0x1) and MAC address (0x3) @@ -60,7 +59,6 @@ struct pxe_label { char *initrd; char *fdt; char *fdtdir; - struct alist fdtoverlays; struct alist files; char *say; int ipappend; diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 260fc918592..df7cf2a781a 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -190,11 +190,11 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_asserteq_str("/initrd.img", label->initrd); ut_asserteq_str("/dtb/board.dtb", label->fdt); ut_assertnull(label->fdtdir); - ut_asserteq(2, label->fdtoverlays.count); + ut_asserteq(2, label->files.count); ut_asserteq_str("/dtb/overlay1.dtbo", - alist_get(&label->fdtoverlays, 0, struct pxe_file)->path); + alist_get(&label->files, 0, struct pxe_file)->path); ut_asserteq_str("/dtb/overlay2.dtbo", - alist_get(&label->fdtoverlays, 1, struct pxe_file)->path); + alist_get(&label->files, 1, struct pxe_file)->path); ut_asserteq_str("Booting default Linux kernel", label->say); ut_asserteq(0, label->ipappend); ut_asserteq(0, label->attempted); @@ -214,7 +214,7 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assertnull(label->initrd); ut_assertnull(label->fdt); ut_asserteq_str("/dtb/", label->fdtdir); - ut_asserteq(0, label->fdtoverlays.count); + ut_asserteq(0, label->files.count); ut_assertnull(label->say); ut_asserteq(3, label->ipappend); ut_asserteq(0, label->attempted); @@ -234,7 +234,7 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assertnull(label->initrd); ut_assertnull(label->fdt); ut_assertnull(label->fdtdir); - ut_asserteq(0, label->fdtoverlays.count); + ut_asserteq(0, label->files.count); ut_assertnull(label->say); ut_asserteq(0, label->ipappend); ut_asserteq(0, label->attempted); @@ -254,7 +254,7 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assertnull(label->initrd); ut_assertnull(label->fdt); ut_assertnull(label->fdtdir); - ut_asserteq(0, label->fdtoverlays.count); + ut_asserteq(0, label->files.count); ut_assertnull(label->say); ut_asserteq(0, label->ipappend); ut_asserteq(0, label->attempted); @@ -274,7 +274,7 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assertnull(label->initrd); ut_assertnull(label->fdt); ut_assertnull(label->fdtdir); - ut_asserteq(0, label->fdtoverlays.count); + ut_asserteq(0, label->files.count); ut_assertnull(label->say); ut_asserteq(0, label->ipappend); ut_asserteq(0, label->attempted); @@ -296,7 +296,7 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) * environment, and verify overlay files can be loaded. */ label = list_first_entry(&cfg->labels, struct pxe_label, list); - ut_asserteq(2, label->fdtoverlays.count); + ut_asserteq(2, label->files.count); /* Set environment variables for file loading */ ut_assertok(env_set_hex("kernel_addr_r", PXE_KERNEL_ADDR)); @@ -316,14 +316,14 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_asserteq(PXE_FDT_ADDR, ctx.fdt_addr); /* Verify overlays were loaded to valid addresses */ - ut_assert(alist_get(&label->fdtoverlays, 0, + ut_assert(alist_get(&label->files, 0, struct pxe_file)->addr >= PXE_OVERLAY_ADDR); - ut_assert(alist_get(&label->fdtoverlays, 1, + ut_assert(alist_get(&label->files, 1, struct pxe_file)->addr >= PXE_OVERLAY_ADDR); /* Second overlay should be at a higher address than the first */ - ut_assert(alist_get(&label->fdtoverlays, 1, struct pxe_file)->addr > - alist_get(&label->fdtoverlays, 0, struct pxe_file)->addr); + ut_assert(alist_get(&label->files, 1, struct pxe_file)->addr > + alist_get(&label->files, 0, struct pxe_file)->addr); /* Verify no more console output */ ut_assert_console_end(); @@ -702,7 +702,7 @@ static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts) /* Get the first label (linux) which has fdtoverlays */ label = list_first_entry(&cfg->labels, struct pxe_label, list); ut_asserteq_str("linux", label->name); - ut_assert(label->fdtoverlays.count > 0); + ut_assert(label->files.count > 0); /* Enable output for loading phase */ ctx.quiet = false; -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a helper function to add files to a label's file list. This makes it easier to add files. Refactor parse_fdtoverlays() to use the new helper, taking a label pointer instead of an alist pointer. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 61 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index b516da8f759..51fce1d5737 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -327,10 +327,37 @@ static bool has_fdtoverlays(struct alist *files) return false; } +/** + * label_add_file() - Add a file to a label's file list + * + * @label: Label to add file to + * @path: Path to file (will be duplicated) + * @type: Type of file (PFT_KERNEL, PFT_INITRD, etc.) + * Return: 0 on success, -ENOMEM on allocation failure + */ +static int label_add_file(struct pxe_label *label, const char *path, + enum pxe_file_type_t type) +{ + struct pxe_file item; + + item.path = strdup(path); + if (!item.path) + return -ENOMEM; + item.type = type; + item.addr = 0; + item.size = 0; + if (!alist_add(&label->files, item)) { + free(item.path); + return -ENOMEM; + } + + return 0; +} + /* - * Parse a space-separated list of overlay paths into an alist. + * Parse a space-separated list of overlay paths into a label's file list. */ -static int parse_fdtoverlays(char **c, struct alist *files) +static int parse_fdtoverlays(char **c, struct pxe_label *label) { char *val, *start; int err; @@ -341,7 +368,6 @@ static int parse_fdtoverlays(char **c, struct alist *files) start = val; while (*val) { - struct pxe_file item; char *end; /* Skip leading spaces */ @@ -351,23 +377,22 @@ static int parse_fdtoverlays(char **c, struct alist *files) if (!*val) break; - /* Find end of this path */ + /* Find end of this path and temporarily null-terminate */ end = strchr(val, ' '); - if (end) { - item.path = strndup(val, end - val); - val = end; - } else { - item.path = strdup(val); - val += strlen(val); - } - item.type = PFT_FDTOVERLAY; - item.addr = 0; - item.size = 0; + if (end) + *end = '\0'; - if (!item.path || !alist_add(files, item)) { - free(item.path); + err = label_add_file(label, val, PFT_FDTOVERLAY); + if (err) { free(start); - return -ENOMEM; + return err; + } + + if (end) { + *end = ' '; + val = end + 1; + } else { + break; } } @@ -605,7 +630,7 @@ static int parse_label(char **c, struct pxe_menu *cfg) break; case T_FDTOVERLAYS: if (!has_fdtoverlays(&label->files)) - err = parse_fdtoverlays(c, &label->files); + err = parse_fdtoverlays(c, label); break; case T_LOCALBOOT: label->localboot = 1; -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> When parsing a kernel label, add the kernel path to the files list with type PFT_KERNEL. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 15 +++++++-------- test/boot/pxe.c | 32 ++++++++++++++++++++------------ 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index 51fce1d5737..8bd9d66f486 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -547,16 +547,15 @@ static int parse_label_kernel(char **c, struct pxe_label *label) return -ENOMEM; s = strstr(label->kernel, "#"); - if (!s) - return 1; - - label->config = strdup(s); - if (!label->config) - return -ENOMEM; + if (s) { + label->config = strdup(s); + if (!label->config) + return -ENOMEM; - *s = 0; + *s = 0; + } - return 1; + return label_add_file(label, label->kernel, PFT_KERNEL) ? : 1; } /* diff --git a/test/boot/pxe.c b/test/boot/pxe.c index df7cf2a781a..6b2d521c8e8 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -190,11 +190,13 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_asserteq_str("/initrd.img", label->initrd); ut_asserteq_str("/dtb/board.dtb", label->fdt); ut_assertnull(label->fdtdir); - ut_asserteq(2, label->files.count); - ut_asserteq_str("/dtb/overlay1.dtbo", + ut_asserteq(3, label->files.count); + ut_asserteq_str("/vmlinuz", alist_get(&label->files, 0, struct pxe_file)->path); - ut_asserteq_str("/dtb/overlay2.dtbo", + ut_asserteq_str("/dtb/overlay1.dtbo", alist_get(&label->files, 1, struct pxe_file)->path); + ut_asserteq_str("/dtb/overlay2.dtbo", + alist_get(&label->files, 2, struct pxe_file)->path); ut_asserteq_str("Booting default Linux kernel", label->say); ut_asserteq(0, label->ipappend); ut_asserteq(0, label->attempted); @@ -214,7 +216,9 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assertnull(label->initrd); ut_assertnull(label->fdt); ut_asserteq_str("/dtb/", label->fdtdir); - ut_asserteq(0, label->files.count); + ut_asserteq(1, label->files.count); + ut_asserteq_str("/vmlinuz-rescue", + alist_get(&label->files, 0, struct pxe_file)->path); ut_assertnull(label->say); ut_asserteq(3, label->ipappend); ut_asserteq(0, label->attempted); @@ -254,7 +258,9 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assertnull(label->initrd); ut_assertnull(label->fdt); ut_assertnull(label->fdtdir); - ut_asserteq(0, label->files.count); + ut_asserteq(1, label->files.count); + ut_asserteq_str("/boot/image.fit", + alist_get(&label->files, 0, struct pxe_file)->path); ut_assertnull(label->say); ut_asserteq(0, label->ipappend); ut_asserteq(0, label->attempted); @@ -274,7 +280,9 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_assertnull(label->initrd); ut_assertnull(label->fdt); ut_assertnull(label->fdtdir); - ut_asserteq(0, label->files.count); + ut_asserteq(1, label->files.count); + ut_asserteq_str("/boot/included-kernel", + alist_get(&label->files, 0, struct pxe_file)->path); ut_assertnull(label->say); ut_asserteq(0, label->ipappend); ut_asserteq(0, label->attempted); @@ -296,7 +304,7 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) * environment, and verify overlay files can be loaded. */ label = list_first_entry(&cfg->labels, struct pxe_label, list); - ut_asserteq(2, label->files.count); + ut_asserteq(3, label->files.count); /* Set environment variables for file loading */ ut_assertok(env_set_hex("kernel_addr_r", PXE_KERNEL_ADDR)); @@ -315,15 +323,15 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_asserteq(PXE_KERNEL_ADDR, ctx.kern_addr); ut_asserteq(PXE_FDT_ADDR, ctx.fdt_addr); - /* Verify overlays were loaded to valid addresses */ - ut_assert(alist_get(&label->files, 0, - struct pxe_file)->addr >= PXE_OVERLAY_ADDR); + /* Verify overlays were loaded to valid addresses (indices 1 and 2) */ ut_assert(alist_get(&label->files, 1, struct pxe_file)->addr >= PXE_OVERLAY_ADDR); + ut_assert(alist_get(&label->files, 2, + struct pxe_file)->addr >= PXE_OVERLAY_ADDR); /* Second overlay should be at a higher address than the first */ - ut_assert(alist_get(&label->files, 1, struct pxe_file)->addr > - alist_get(&label->files, 0, struct pxe_file)->addr); + ut_assert(alist_get(&label->files, 2, struct pxe_file)->addr > + alist_get(&label->files, 1, struct pxe_file)->addr); /* Verify no more console output */ ut_assert_console_end(); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> When parsing an initrd, add the path to the files list with type PFT_INITRD. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 7 ++++++- test/boot/pxe.c | 20 +++++++++++--------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index 8bd9d66f486..7dac5c94759 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -616,8 +616,13 @@ static int parse_label(char **c, struct pxe_menu *cfg) break; case T_INITRD: - if (!label->initrd) + if (!label->initrd) { err = parse_sliteral(c, &label->initrd); + if (err < 0) + break; + err = label_add_file(label, label->initrd, + PFT_INITRD); + } break; case T_FDT: if (!label->fdt) diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 6b2d521c8e8..0751228dc99 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -190,13 +190,15 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_asserteq_str("/initrd.img", label->initrd); ut_asserteq_str("/dtb/board.dtb", label->fdt); ut_assertnull(label->fdtdir); - ut_asserteq(3, label->files.count); + ut_asserteq(4, label->files.count); ut_asserteq_str("/vmlinuz", alist_get(&label->files, 0, struct pxe_file)->path); - ut_asserteq_str("/dtb/overlay1.dtbo", + ut_asserteq_str("/initrd.img", alist_get(&label->files, 1, struct pxe_file)->path); - ut_asserteq_str("/dtb/overlay2.dtbo", + ut_asserteq_str("/dtb/overlay1.dtbo", alist_get(&label->files, 2, struct pxe_file)->path); + ut_asserteq_str("/dtb/overlay2.dtbo", + alist_get(&label->files, 3, struct pxe_file)->path); ut_asserteq_str("Booting default Linux kernel", label->say); ut_asserteq(0, label->ipappend); ut_asserteq(0, label->attempted); @@ -304,7 +306,7 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) * environment, and verify overlay files can be loaded. */ label = list_first_entry(&cfg->labels, struct pxe_label, list); - ut_asserteq(3, label->files.count); + ut_asserteq(4, label->files.count); /* Set environment variables for file loading */ ut_assertok(env_set_hex("kernel_addr_r", PXE_KERNEL_ADDR)); @@ -323,15 +325,15 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_asserteq(PXE_KERNEL_ADDR, ctx.kern_addr); ut_asserteq(PXE_FDT_ADDR, ctx.fdt_addr); - /* Verify overlays were loaded to valid addresses (indices 1 and 2) */ - ut_assert(alist_get(&label->files, 1, - struct pxe_file)->addr >= PXE_OVERLAY_ADDR); + /* Verify overlays were loaded to valid addresses (indices 2 and 3) */ ut_assert(alist_get(&label->files, 2, struct pxe_file)->addr >= PXE_OVERLAY_ADDR); + ut_assert(alist_get(&label->files, 3, + struct pxe_file)->addr >= PXE_OVERLAY_ADDR); /* Second overlay should be at a higher address than the first */ - ut_assert(alist_get(&label->files, 2, struct pxe_file)->addr > - alist_get(&label->files, 1, struct pxe_file)->addr); + ut_assert(alist_get(&label->files, 3, struct pxe_file)->addr > + alist_get(&label->files, 2, struct pxe_file)->addr); /* Verify no more console output */ ut_assert_console_end(); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> When parsing an fdt directive, add the path to the files list with type PFT_FDT. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 6 +++++- test/boot/pxe.c | 20 +++++++++++--------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index 7dac5c94759..bcbe0495cbb 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -625,8 +625,12 @@ static int parse_label(char **c, struct pxe_menu *cfg) } break; case T_FDT: - if (!label->fdt) + if (!label->fdt) { err = parse_sliteral(c, &label->fdt); + if (err < 0) + break; + err = label_add_file(label, label->fdt, PFT_FDT); + } break; case T_FDTDIR: if (!label->fdtdir) diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 0751228dc99..2d690141349 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -190,15 +190,17 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_asserteq_str("/initrd.img", label->initrd); ut_asserteq_str("/dtb/board.dtb", label->fdt); ut_assertnull(label->fdtdir); - ut_asserteq(4, label->files.count); + ut_asserteq(5, label->files.count); ut_asserteq_str("/vmlinuz", alist_get(&label->files, 0, struct pxe_file)->path); ut_asserteq_str("/initrd.img", alist_get(&label->files, 1, struct pxe_file)->path); - ut_asserteq_str("/dtb/overlay1.dtbo", + ut_asserteq_str("/dtb/board.dtb", alist_get(&label->files, 2, struct pxe_file)->path); - ut_asserteq_str("/dtb/overlay2.dtbo", + ut_asserteq_str("/dtb/overlay1.dtbo", alist_get(&label->files, 3, struct pxe_file)->path); + ut_asserteq_str("/dtb/overlay2.dtbo", + alist_get(&label->files, 4, struct pxe_file)->path); ut_asserteq_str("Booting default Linux kernel", label->say); ut_asserteq(0, label->ipappend); ut_asserteq(0, label->attempted); @@ -306,7 +308,7 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) * environment, and verify overlay files can be loaded. */ label = list_first_entry(&cfg->labels, struct pxe_label, list); - ut_asserteq(4, label->files.count); + ut_asserteq(5, label->files.count); /* Set environment variables for file loading */ ut_assertok(env_set_hex("kernel_addr_r", PXE_KERNEL_ADDR)); @@ -325,15 +327,15 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_asserteq(PXE_KERNEL_ADDR, ctx.kern_addr); ut_asserteq(PXE_FDT_ADDR, ctx.fdt_addr); - /* Verify overlays were loaded to valid addresses (indices 2 and 3) */ - ut_assert(alist_get(&label->files, 2, - struct pxe_file)->addr >= PXE_OVERLAY_ADDR); + /* Verify overlays were loaded to valid addresses (indices 3 and 4) */ ut_assert(alist_get(&label->files, 3, struct pxe_file)->addr >= PXE_OVERLAY_ADDR); + ut_assert(alist_get(&label->files, 4, + struct pxe_file)->addr >= PXE_OVERLAY_ADDR); /* Second overlay should be at a higher address than the first */ - ut_assert(alist_get(&label->files, 3, struct pxe_file)->addr > - alist_get(&label->files, 2, struct pxe_file)->addr); + ut_assert(alist_get(&label->files, 4, struct pxe_file)->addr > + alist_get(&label->files, 3, struct pxe_file)->addr); /* Verify no more console output */ ut_assert_console_end(); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Currently pxe_file_size is only set by the cmd/pxe.c callback, so other callers of get_pxe_file() (like tests) do not have access to the file size. Add an assignment into get_pxe_file() so the size is always available in the context after loading a PXE config file. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 1 + test/boot/pxe.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index a2a2b986de1..2d9081e6e9b 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -151,6 +151,7 @@ int get_pxe_file(struct pxe_context *ctx, const char *file_path, buf = map_sysmem(file_addr + size, 1); *buf = '\0'; unmap_sysmem(buf); + ctx->pxe_file_size = size; return 1; } diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 2d690141349..a1ce27ea2a7 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -1178,7 +1178,7 @@ static int pxe_test_alloc_norun(struct unit_test_state *uts) ut_asserteq_ptr(&info, ctx.userdata); ut_asserteq(true, ctx.allow_abs_path); ut_assertnonnull(ctx.bootdir); - ut_asserteq(0, ctx.pxe_file_size); /* only set by cmd/pxe.c */ + ut_assert(ctx.pxe_file_size); /* set by get_pxe_file() */ ut_asserteq(false, ctx.use_ipv6); ut_asserteq(false, ctx.use_fallback); ut_asserteq(true, ctx.no_boot); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The extlinux_read_all() function calls pxe_probe() with a config file that is already loaded in bflow->buf. However, ctx->pxe_file_size is not set, so the parser receives a size of 0 and fails. Set ctx->pxe_file_size from bflow->size before calling pxe_probe(). Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/ext_pxe_common.c | 1 + 1 file changed, 1 insertion(+) diff --git a/boot/ext_pxe_common.c b/boot/ext_pxe_common.c index 3b1412b86d3..930e54ea84d 100644 --- a/boot/ext_pxe_common.c +++ b/boot/ext_pxe_common.c @@ -136,6 +136,7 @@ int extlinux_read_all(struct udevice *dev, struct bootflow *bflow, if (ret) return log_msg_ret("era", ret); addr = map_to_sysmem(bflow->buf); + plat->ctx.pxe_file_size = bflow->size; ret = pxe_probe(&plat->ctx, addr, false); if (ret) return log_msg_ret("elb", -EFAULT); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The parse_pxefile() function uses map_sysmem() to map the file contents. Currently it passes 0 for the size, which works but is not ideal. Add an explicit size parameter so callers can provide the actual file size. Internal callers use ctx->pxe_file_size which is set by get_pxe_file(). Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 7 ++++--- include/pxe_utils.h | 4 +++- test/boot/pxe.c | 10 +++++----- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 2d9081e6e9b..6416ee3ddff 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -1004,7 +1004,8 @@ void pxe_menu_uninit(struct pxe_menu *cfg) free(cfg); } -struct pxe_menu *parse_pxefile(struct pxe_context *ctx, unsigned long menucfg) +struct pxe_menu *parse_pxefile(struct pxe_context *ctx, unsigned long menucfg, + ulong size) { struct pxe_menu *cfg; char *buf; @@ -1014,7 +1015,7 @@ struct pxe_menu *parse_pxefile(struct pxe_context *ctx, unsigned long menucfg) if (!cfg) return NULL; - buf = map_sysmem(menucfg, 0); + buf = map_sysmem(menucfg, size); r = parse_pxefile_top(ctx, buf, menucfg, cfg, 1); unmap_sysmem(buf); @@ -1306,7 +1307,7 @@ static struct pxe_menu *pxe_prepare(struct pxe_context *ctx, struct pxe_menu *cfg; int ret; - cfg = parse_pxefile(ctx, pxefile_addr_r); + cfg = parse_pxefile(ctx, pxefile_addr_r, ctx->pxe_file_size); if (!cfg) { printf("Error parsing config file\n"); return NULL; diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 266204b97ef..50e0c0c6b93 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -299,10 +299,12 @@ void handle_pxe_menu(struct pxe_context *ctx, struct pxe_menu *cfg); * * @ctx: PXE context (provided by the caller) * @menucfg: Address of the PXE file in memory + * @size: Size of file in bytes * 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); +struct pxe_menu *parse_pxefile(struct pxe_context *ctx, ulong menucfg, + ulong size); /** * pxe_process_includes() - Process include files in a parsed menu diff --git a/test/boot/pxe.c b/test/boot/pxe.c index a1ce27ea2a7..1a990a8e348 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -161,7 +161,7 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_asserteq(1, ret); /* get_pxe_file returns 1 on success */ /* Parse the config file */ - cfg = parse_pxefile(&ctx, addr); + cfg = parse_pxefile(&ctx, addr, ctx.pxe_file_size); ut_assertnonnull(cfg); /* Process any include files */ @@ -465,7 +465,7 @@ static int pxe_test_fdtdir_norun(struct unit_test_state *uts) /* Read and parse the config file */ ut_asserteq(1, get_pxe_file(&ctx, cfg_path, addr)); - cfg = parse_pxefile(&ctx, addr); + cfg = parse_pxefile(&ctx, addr, ctx.pxe_file_size); ut_assertnonnull(cfg); /* Consume parsing output */ @@ -574,7 +574,7 @@ static int pxe_test_errors_norun(struct unit_test_state *uts) /* Read and parse the config file */ ut_asserteq(1, get_pxe_file(&ctx, cfg_path, addr)); - cfg = parse_pxefile(&ctx, addr); + cfg = parse_pxefile(&ctx, addr, ctx.pxe_file_size); ut_assertnonnull(cfg); /* Consume parsing output */ @@ -693,7 +693,7 @@ static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts) ctx.quiet = true; ut_asserteq(1, get_pxe_file(&ctx, cfg_path, addr)); - cfg = parse_pxefile(&ctx, addr); + cfg = parse_pxefile(&ctx, addr, ctx.pxe_file_size); ut_assertnonnull(cfg); /* Process any include files */ @@ -1268,7 +1268,7 @@ static int pxe_test_fit_embedded_fdt_norun(struct unit_test_state *uts) /* Read and parse the config file */ ut_asserteq(1, get_pxe_file(&ctx, cfg_path, addr)); - cfg = parse_pxefile(&ctx, addr); + cfg = parse_pxefile(&ctx, addr, ctx.pxe_file_size); ut_assertnonnull(cfg); /* Consume parsing output */ -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The pxe_process() function requires a nul-terminated string at the given address. Rename the function to pxe_process_str() to make this requirement clear, and add a note to the documentation. Add a size parameter to pxe_process() Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/ext_pxe_common.c | 2 +- boot/pxe_utils.c | 17 +++++++++++++++-- cmd/pxe.c | 2 +- cmd/sysboot.c | 2 +- doc/develop/bootstd/extlinux.rst | 2 +- doc/develop/bootstd/pxelinux.rst | 2 +- include/pxe_utils.h | 16 ++++++++++++++-- 7 files changed, 34 insertions(+), 9 deletions(-) diff --git a/boot/ext_pxe_common.c b/boot/ext_pxe_common.c index 930e54ea84d..2bc76a4d0fb 100644 --- a/boot/ext_pxe_common.c +++ b/boot/ext_pxe_common.c @@ -115,7 +115,7 @@ int extlinux_boot(struct udevice *dev, struct bootflow *bflow, return log_msg_ret("elb", ret); plat->ctx.restart = restart; addr = map_to_sysmem(bflow->buf); - ret = pxe_process(&plat->ctx, addr, false); + ret = pxe_process_str(&plat->ctx, addr, false); } if (ret) return log_msg_ret("elb", -EFAULT); diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 6416ee3ddff..3f751f25bea 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -1325,11 +1325,12 @@ static struct pxe_menu *pxe_prepare(struct pxe_context *ctx, return cfg; } -int pxe_process(struct pxe_context *ctx, ulong pxefile_addr_r, bool prompt) +int pxe_process(struct pxe_context *ctx, ulong addr, ulong size, bool prompt) { struct pxe_menu *cfg; - cfg = pxe_prepare(ctx, pxefile_addr_r, prompt); + ctx->pxe_file_size = size; + cfg = pxe_prepare(ctx, addr, prompt); if (!cfg) return 1; @@ -1340,6 +1341,18 @@ int pxe_process(struct pxe_context *ctx, ulong pxefile_addr_r, bool prompt) return 0; } +int pxe_process_str(struct pxe_context *ctx, ulong pxefile_addr_r, bool prompt) +{ + void *ptr; + int len; + + ptr = map_sysmem(pxefile_addr_r, 0); + len = strnlen(ptr, SZ_64K); + unmap_sysmem(ptr); + + return pxe_process(ctx, pxefile_addr_r, len, prompt); +} + int pxe_probe(struct pxe_context *ctx, ulong pxefile_addr_r, bool prompt) { ctx->cfg = pxe_prepare(ctx, pxefile_addr_r, prompt); diff --git a/cmd/pxe.c b/cmd/pxe.c index 9f40f13c201..2cda2597f28 100644 --- a/cmd/pxe.c +++ b/cmd/pxe.c @@ -286,7 +286,7 @@ do_pxe_boot(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) printf("Out of memory\n"); return CMD_RET_FAILURE; } - ret = pxe_process(&ctx, pxefile_addr_r, false); + ret = pxe_process(&ctx, pxefile_addr_r, ctx.pxe_file_size, false); pxe_destroy_ctx(&ctx); if (ret) return CMD_RET_FAILURE; diff --git a/cmd/sysboot.c b/cmd/sysboot.c index e6f89c999cc..b48272b2126 100644 --- a/cmd/sysboot.c +++ b/cmd/sysboot.c @@ -119,7 +119,7 @@ static int do_sysboot(struct cmd_tbl *cmdtp, int flag, int argc, return 1; } - ret = pxe_process(&ctx, pxefile_addr_r, prompt); + ret = pxe_process(&ctx, pxefile_addr_r, ctx.pxe_file_size, prompt); pxe_destroy_ctx(&ctx); if (ret) return CMD_RET_FAILURE; diff --git a/doc/develop/bootstd/extlinux.rst b/doc/develop/bootstd/extlinux.rst index bf27dc57aaa..0c8526d7325 100644 --- a/doc/develop/bootstd/extlinux.rst +++ b/doc/develop/bootstd/extlinux.rst @@ -21,7 +21,7 @@ When invoked on a bootdev, this bootmeth searches for the file and creates a bootflow if found. When the bootflow is booted, the bootmeth calls ``pxe_setup_ctx()`` to set up -the context, then ``pxe_process()`` to process the file. Depending on the +the context, then ``pxe_process_str()`` to process the file. Depending on the contents, this may boot an operating system or provide a list of options to the user, perhaps with a timeout. diff --git a/doc/develop/bootstd/pxelinux.rst b/doc/develop/bootstd/pxelinux.rst index c4b7fbb4c9c..c3d5e6f2408 100644 --- a/doc/develop/bootstd/pxelinux.rst +++ b/doc/develop/bootstd/pxelinux.rst @@ -19,7 +19,7 @@ bootflow if found. See a full description of the search procedure. When the bootflow is booted, the bootmeth calls ``pxe_setup_ctx()`` to set up -the context, then ``pxe_process()`` to process the file. Depending on the +the context, then ``pxe_process_str()`` to process the file. Depending on the contents, this may boot an Operating System or provide a list of options to the user, perhaps with a timeout. diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 50e0c0c6b93..e8045511c6a 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -366,14 +366,26 @@ int pxe_setup_ctx(struct pxe_context *ctx, pxe_getfile_func getfile, */ void pxe_destroy_ctx(struct pxe_context *ctx); +/** + * pxe_process_str() - Process a PXE file through to boot + * + * Note: The file at @pxefile_addr_r must be a nul-terminated string. + * + * @ctx: PXE context created with pxe_setup_ctx() + * @pxefile_addr_r: Address of config to process + * @prompt: Force a prompt for the user + */ +int pxe_process_str(struct pxe_context *ctx, ulong pxefile_addr_r, bool prompt); + /** * pxe_process() - Process a PXE file through to boot * * @ctx: PXE context created with pxe_setup_ctx() - * @pxefile_addr_r: Address to load file + * @addr: Address of config to process + * @size: Size of continue to process * @prompt: Force a prompt for the user */ -int pxe_process(struct pxe_context *ctx, ulong pxefile_addr_r, bool prompt); +int pxe_process(struct pxe_context *ctx, ulong addr, ulong size, bool prompt); /** * pxe_get_file_size() - Read the value of the 'filesize' environment variable -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Change parse_pxefile() to accept a struct abuf instead of separate address and size parameters. This provides a cleaner interface and allows the function to use abuf_data() directly without needing to call map_sysmem(). Callers now create an abuf with abuf_map_sysmem() to wrap the memory region before calling parse_pxefile(). Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_utils.c | 12 +++++------- include/pxe_utils.h | 9 ++++----- test/boot/pxe.c | 20 +++++++++++++++----- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 3f751f25bea..d2ba0412906 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -1004,20 +1004,16 @@ void pxe_menu_uninit(struct pxe_menu *cfg) free(cfg); } -struct pxe_menu *parse_pxefile(struct pxe_context *ctx, unsigned long menucfg, - ulong size) +struct pxe_menu *parse_pxefile(struct pxe_context *ctx, struct abuf *buf) { struct pxe_menu *cfg; - char *buf; int r; cfg = pxe_menu_init(); if (!cfg) return NULL; - buf = map_sysmem(menucfg, size); - r = parse_pxefile_top(ctx, buf, menucfg, cfg, 1); - unmap_sysmem(buf); + r = parse_pxefile_top(ctx, abuf_data(buf), abuf_addr(buf), cfg, 1); if (r < 0) { pxe_menu_uninit(cfg); @@ -1305,9 +1301,11 @@ static struct pxe_menu *pxe_prepare(struct pxe_context *ctx, ulong pxefile_addr_r, bool prompt) { struct pxe_menu *cfg; + struct abuf buf; int ret; - cfg = parse_pxefile(ctx, pxefile_addr_r, ctx->pxe_file_size); + abuf_init_addr(&buf, pxefile_addr_r, ctx->pxe_file_size); + cfg = parse_pxefile(ctx, &buf); if (!cfg) { printf("Error parsing config file\n"); return NULL; diff --git a/include/pxe_utils.h b/include/pxe_utils.h index e8045511c6a..9682956932b 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -3,6 +3,7 @@ #ifndef __PXE_UTILS_H #define __PXE_UTILS_H +#include <abuf.h> #include <alist.h> #include <bootflow.h> #include <linux/list.h> @@ -292,19 +293,17 @@ 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() - Parse a pxe file + * parse_pxefile() - Parse a PXE 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) - * @menucfg: Address of the PXE file in memory - * @size: Size of file in bytes + * @buf: Buffer containing the PXE file * 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, - ulong size); +struct pxe_menu *parse_pxefile(struct pxe_context *ctx, struct abuf *buf); /** * pxe_process_includes() - Process include files in a parsed menu diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 1a990a8e348..56b1dfb4c49 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -139,6 +139,7 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) struct pxe_context ctx; struct pxe_label *label; struct pxe_menu *cfg; + struct abuf buf; char name[16]; uint i; int ret; @@ -161,7 +162,8 @@ static int pxe_test_parse_norun(struct unit_test_state *uts) ut_asserteq(1, ret); /* get_pxe_file returns 1 on success */ /* Parse the config file */ - cfg = parse_pxefile(&ctx, addr, ctx.pxe_file_size); + abuf_init_addr(&buf, addr, ctx.pxe_file_size); + cfg = parse_pxefile(&ctx, &buf); ut_assertnonnull(cfg); /* Process any include files */ @@ -448,6 +450,7 @@ static int pxe_test_fdtdir_norun(struct unit_test_state *uts) struct pxe_label *label; struct pxe_menu *cfg; ulong addr = PXE_LOAD_ADDR; + struct abuf buf; void *fdt; ut_assertnonnull(fs_image); @@ -465,7 +468,8 @@ static int pxe_test_fdtdir_norun(struct unit_test_state *uts) /* Read and parse the config file */ ut_asserteq(1, get_pxe_file(&ctx, cfg_path, addr)); - cfg = parse_pxefile(&ctx, addr, ctx.pxe_file_size); + abuf_init_addr(&buf, addr, ctx.pxe_file_size); + cfg = parse_pxefile(&ctx, &buf); ut_assertnonnull(cfg); /* Consume parsing output */ @@ -557,6 +561,7 @@ static int pxe_test_errors_norun(struct unit_test_state *uts) struct pxe_label *label; struct pxe_menu *cfg; ulong addr = PXE_LOAD_ADDR; + struct abuf buf; void *fdt; ut_assertnonnull(fs_image); @@ -574,7 +579,8 @@ static int pxe_test_errors_norun(struct unit_test_state *uts) /* Read and parse the config file */ ut_asserteq(1, get_pxe_file(&ctx, cfg_path, addr)); - cfg = parse_pxefile(&ctx, addr, ctx.pxe_file_size); + abuf_init_addr(&buf, addr, ctx.pxe_file_size); + cfg = parse_pxefile(&ctx, &buf); ut_assertnonnull(cfg); /* Consume parsing output */ @@ -675,6 +681,7 @@ static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts) struct pxe_label *label; struct pxe_menu *cfg; ulong addr = PXE_LOAD_ADDR; + struct abuf buf; void *fdt; ut_assertnonnull(fs_image); @@ -693,7 +700,8 @@ static int pxe_test_overlay_no_addr_norun(struct unit_test_state *uts) ctx.quiet = true; ut_asserteq(1, get_pxe_file(&ctx, cfg_path, addr)); - cfg = parse_pxefile(&ctx, addr, ctx.pxe_file_size); + abuf_init_addr(&buf, addr, ctx.pxe_file_size); + cfg = parse_pxefile(&ctx, &buf); ut_assertnonnull(cfg); /* Process any include files */ @@ -1248,6 +1256,7 @@ static int pxe_test_fit_embedded_fdt_norun(struct unit_test_state *uts) struct pxe_label *label; struct pxe_menu *cfg; ulong addr = PXE_LOAD_ADDR; + struct abuf buf; ut_assertnonnull(fs_image); ut_assertnonnull(cfg_path); @@ -1268,7 +1277,8 @@ static int pxe_test_fit_embedded_fdt_norun(struct unit_test_state *uts) /* Read and parse the config file */ ut_asserteq(1, get_pxe_file(&ctx, cfg_path, addr)); - cfg = parse_pxefile(&ctx, addr, ctx.pxe_file_size); + abuf_init_addr(&buf, addr, ctx.pxe_file_size); + cfg = parse_pxefile(&ctx, &buf); ut_assertnonnull(cfg); /* Consume parsing output */ -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The parse_menu() function takes a base parameter but never uses it. Remove this dead parameter. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index bcbe0495cbb..52216c0b7ef 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -460,7 +460,7 @@ static int handle_include(char **c, struct pxe_menu *cfg, int nest_level) * a file it includes, 3 when parsing a file included by that file, and so on. */ static int parse_menu(struct pxe_context *ctx, char **c, struct pxe_menu *cfg, - unsigned long base, int nest_level) + int nest_level) { struct token t; char *s = *c; @@ -714,9 +714,7 @@ int parse_pxefile_top(struct pxe_context *ctx, char *p, ulong base, switch (t.type) { case T_MENU: cfg->prompt = 1; - err = parse_menu(ctx, &p, cfg, - base + ALIGN(strlen(b) + 1, 4), - nest_level); + err = parse_menu(ctx, &p, cfg, nest_level); break; case T_TIMEOUT: err = parse_integer(&p, &cfg->timeout); -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> The parse_pxefile_top() function takes a base parameter but never uses it. Remove this dead parameter. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 2 +- boot/pxe_utils.c | 4 ++-- include/pxe_utils.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index 52216c0b7ef..3d113a6fd83 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -691,7 +691,7 @@ static int parse_label(char **c, struct pxe_menu *cfg) */ #define MAX_NEST_LEVEL 16 -int parse_pxefile_top(struct pxe_context *ctx, char *p, ulong base, +int parse_pxefile_top(struct pxe_context *ctx, char *p, struct pxe_menu *cfg, int nest_level) { struct token t; diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index d2ba0412906..b5320f2f6cf 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -1013,7 +1013,7 @@ struct pxe_menu *parse_pxefile(struct pxe_context *ctx, struct abuf *buf) if (!cfg) return NULL; - r = parse_pxefile_top(ctx, abuf_data(buf), abuf_addr(buf), cfg, 1); + r = parse_pxefile_top(ctx, abuf_data(buf), cfg, 1); if (r < 0) { pxe_menu_uninit(cfg); @@ -1069,7 +1069,7 @@ int pxe_parse_include(struct pxe_context *ctx, const struct pxe_include *inc, int ret; buf = map_sysmem(addr, 0); - ret = parse_pxefile_top(ctx, buf, addr, inc->cfg, inc->nest_level); + ret = parse_pxefile_top(ctx, buf, inc->cfg, inc->nest_level); unmap_sysmem(buf); return ret; diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 9682956932b..11be00da199 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -510,7 +510,7 @@ int pxe_setup_label(struct pxe_context *ctx, struct pxe_label *label); * * Returns 1 on success, < 0 on error. */ -int parse_pxefile_top(struct pxe_context *ctx, char *p, ulong base, +int parse_pxefile_top(struct pxe_context *ctx, char *p, struct pxe_menu *cfg, int nest_level); /** -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a `const char *limit` parameter to get_string() and propagate it through all parser functions. This allows the parser to properly detect the end of the buffer without relying solely on a nul terminator. The limit parameter represents the position of the nul terminator (i.e. the end of valid data), allowing the while loop in get_string() to check both *e and e < limit. This provides an additional safety check against buffer overruns. Update all callers in parse_pxefile() and pxe_parse_include() to pass the appropriate limit based on buffer size. Also add a size parameter to pxe_parse_include() so callers pass the size explicitly. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/pxe_parse.c | 102 ++++++++++++++++++++++++-------------------- boot/pxe_utils.c | 13 +++--- include/pxe_utils.h | 15 ++++--- 3 files changed, 73 insertions(+), 57 deletions(-) diff --git a/boot/pxe_parse.c b/boot/pxe_parse.c index 3d113a6fd83..42f9125cb1d 100644 --- a/boot/pxe_parse.c +++ b/boot/pxe_parse.c @@ -155,9 +155,11 @@ void label_destroy(struct pxe_label *label) * @t: Pointers to a token to fill in * @delim: Delimiter character to look for, either newline or space * @lower: true to convert the string to lower case when storing + * @limit: End of buffer (position of nul terminator) * Returns the new value of t->val, on success, NULL if out of memory */ -static char *get_string(char **p, struct token *t, char delim, int lower) +static char *get_string(char **p, struct token *t, char delim, int lower, + const char *limit) { char *b, *e; size_t len, i; @@ -170,7 +172,7 @@ static char *get_string(char **p, struct token *t, char delim, int lower) */ b = *p; e = *p; - while (*e) { + while (*e && e < limit) { if ((delim == ' ' && isspace(*e)) || delim == *e) break; e++; @@ -227,8 +229,12 @@ static void get_keyword(struct token *t) * * @p: Points to a pointer to the current position in the input being processed. * Updated to point at the first character after the current token + * @t: Token to fill in + * @state: Lexer state (keyword or string literal) + * @limit: End of buffer (position of nul terminator) */ -static void get_token(char **p, struct token *t, enum lex_state state) +static void get_token(char **p, struct token *t, enum lex_state state, + const char *limit) { char *c = *p; @@ -251,11 +257,11 @@ static void get_token(char **p, struct token *t, enum lex_state state) if (*c == '\n') { t->type = T_EOL; c++; - } else if (*c == '\0') { + } else if (*c == '\0' || c >= limit) { t->type = T_EOF; c++; } else if (state == L_SLITERAL) { - get_string(&c, t, '\n', 0); + get_string(&c, t, '\n', 0, limit); } else if (state == L_KEYWORD) { /* * when we expect a keyword, we first get the next string @@ -264,7 +270,7 @@ static void get_token(char **p, struct token *t, enum lex_state state) * converted to a keyword token of the appropriate type, and * if not, it remains a string token. */ - get_string(&c, t, ' ', 1); + get_string(&c, t, ' ', 1, limit); get_keyword(t); } @@ -296,12 +302,12 @@ static void eol_or_eof(char **c) * Parse a string literal and store a pointer it at *dst. String literals * terminate at the end of the line. */ -static int parse_sliteral(char **c, char **dst) +static int parse_sliteral(char **c, char **dst, const char *limit) { struct token t; char *s = *c; - get_token(c, &t, L_SLITERAL); + get_token(c, &t, L_SLITERAL, limit); if (t.type != T_STRING) { printf("Expected string literal: %.*s\n", (int)(*c - s), s); return -EINVAL; @@ -357,12 +363,13 @@ static int label_add_file(struct pxe_label *label, const char *path, /* * Parse a space-separated list of overlay paths into a label's file list. */ -static int parse_fdtoverlays(char **c, struct pxe_label *label) +static int parse_fdtoverlays(char **c, struct pxe_label *label, + const char *limit) { char *val, *start; int err; - err = parse_sliteral(c, &val); + err = parse_sliteral(c, &val, limit); if (err < 0) return err; start = val; @@ -404,12 +411,12 @@ static int parse_fdtoverlays(char **c, struct pxe_label *label) /* * Parse a base 10 (unsigned) integer and store it at *dst. */ -static int parse_integer(char **c, int *dst) +static int parse_integer(char **c, int *dst, const char *limit) { struct token t; char *s = *c; - get_token(c, &t, L_SLITERAL); + get_token(c, &t, L_SLITERAL, limit); if (t.type != T_STRING) { printf("Expected string: %.*s\n", (int)(*c - s), s); return -EINVAL; @@ -427,13 +434,14 @@ static int parse_integer(char **c, int *dst) * 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(char **c, struct pxe_menu *cfg, int nest_level) +static int handle_include(char **c, struct pxe_menu *cfg, int nest_level, + const char *limit) { struct pxe_include inc; char *s = *c; int err; - err = parse_sliteral(c, &inc.path); + err = parse_sliteral(c, &inc.path, limit); if (err < 0) { printf("Expected include path: %.*s\n", (int)(*c - s), s); return err; @@ -460,24 +468,24 @@ static int handle_include(char **c, struct pxe_menu *cfg, int nest_level) * a file it includes, 3 when parsing a file included by that file, and so on. */ static int parse_menu(struct pxe_context *ctx, char **c, struct pxe_menu *cfg, - int nest_level) + int nest_level, const char *limit) { struct token t; char *s = *c; int err = 0; t.val = NULL; - get_token(c, &t, L_KEYWORD); + get_token(c, &t, L_KEYWORD, limit); switch (t.type) { case T_TITLE: - err = parse_sliteral(c, &cfg->title); + err = parse_sliteral(c, &cfg->title, limit); break; case T_INCLUDE: - err = handle_include(c, cfg, nest_level); + err = handle_include(c, cfg, nest_level, limit); break; case T_BACKGROUND: - err = parse_sliteral(c, &cfg->bmp); + err = parse_sliteral(c, &cfg->bmp, limit); break; default: printf("Ignoring malformed menu command: %.*s\n", @@ -496,14 +504,14 @@ static int parse_menu(struct pxe_context *ctx, char **c, struct pxe_menu *cfg, * Handles parsing a 'menu line' when we're parsing a label. */ static int parse_label_menu(char **c, struct pxe_menu *cfg, - struct pxe_label *label) + struct pxe_label *label, const char *limit) { struct token t; char *s; s = *c; t.val = NULL; - get_token(c, &t, L_KEYWORD); + get_token(c, &t, L_KEYWORD, limit); switch (t.type) { case T_DEFAULT: @@ -515,7 +523,7 @@ static int parse_label_menu(char **c, struct pxe_menu *cfg, break; case T_LABEL: - parse_sliteral(c, &label->menu); + parse_sliteral(c, &label->menu, limit); break; default: printf("Ignoring malformed menu command: %.*s\n", @@ -532,12 +540,13 @@ static int parse_label_menu(char **c, struct pxe_menu *cfg, * Handles parsing a 'kernel' label. * expecting "filename" or "<fit_filename>#cfg" */ -static int parse_label_kernel(char **c, struct pxe_label *label) +static int parse_label_kernel(char **c, struct pxe_label *label, + const char *limit) { char *s; int err; - err = parse_sliteral(c, &label->kernel); + err = parse_sliteral(c, &label->kernel, limit); if (err < 0) return err; @@ -565,7 +574,7 @@ static int parse_label_kernel(char **c, struct pxe_label *label) * get some input we otherwise don't have a handler defined * for. */ -static int parse_label(char **c, struct pxe_menu *cfg) +static int parse_label(char **c, struct pxe_menu *cfg, const char *limit) { struct token t; int len; @@ -577,7 +586,7 @@ static int parse_label(char **c, struct pxe_menu *cfg) if (!label) return -ENOMEM; - err = parse_sliteral(c, &label->name); + err = parse_sliteral(c, &label->name, limit); if (err < 0) { printf("Expected label name: %.*s\n", (int)(*c - s), s); label_destroy(label); @@ -589,20 +598,20 @@ static int parse_label(char **c, struct pxe_menu *cfg) while (1) { s = *c; free(t.val); - get_token(c, &t, L_KEYWORD); + get_token(c, &t, L_KEYWORD, limit); err = 0; switch (t.type) { case T_MENU: - err = parse_label_menu(c, cfg, label); + err = parse_label_menu(c, cfg, label, limit); break; case T_KERNEL: case T_LINUX: case T_FIT: - err = parse_label_kernel(c, label); + err = parse_label_kernel(c, label, limit); break; case T_APPEND: - err = parse_sliteral(c, &label->append); + err = parse_sliteral(c, &label->append, limit); if (label->initrd) break; s = strstr(label->append, "initrd="); @@ -617,7 +626,7 @@ static int parse_label(char **c, struct pxe_menu *cfg) break; case T_INITRD: if (!label->initrd) { - err = parse_sliteral(c, &label->initrd); + err = parse_sliteral(c, &label->initrd, limit); if (err < 0) break; err = label_add_file(label, label->initrd, @@ -626,7 +635,7 @@ static int parse_label(char **c, struct pxe_menu *cfg) break; case T_FDT: if (!label->fdt) { - err = parse_sliteral(c, &label->fdt); + err = parse_sliteral(c, &label->fdt, limit); if (err < 0) break; err = label_add_file(label, label->fdt, PFT_FDT); @@ -634,18 +643,18 @@ static int parse_label(char **c, struct pxe_menu *cfg) break; case T_FDTDIR: if (!label->fdtdir) - err = parse_sliteral(c, &label->fdtdir); + err = parse_sliteral(c, &label->fdtdir, limit); break; case T_FDTOVERLAYS: if (!has_fdtoverlays(&label->files)) - err = parse_fdtoverlays(c, label); + err = parse_fdtoverlays(c, label, limit); break; case T_LOCALBOOT: label->localboot = 1; - err = parse_integer(c, &label->localboot_val); + err = parse_integer(c, &label->localboot_val, limit); break; case T_IPAPPEND: - err = parse_integer(c, &label->ipappend); + err = parse_integer(c, &label->ipappend, limit); break; case T_KASLRSEED: label->kaslrseed = 1; @@ -691,14 +700,13 @@ static int parse_label(char **c, struct pxe_menu *cfg) */ #define MAX_NEST_LEVEL 16 -int parse_pxefile_top(struct pxe_context *ctx, char *p, +int parse_pxefile_top(struct pxe_context *ctx, char *p, const char *limit, struct pxe_menu *cfg, int nest_level) { struct token t; - char *s, *b, *label_name; + char *s, *label_name; int err; - b = p; if (nest_level > MAX_NEST_LEVEL) { printf("Maximum nesting (%d) exceeded\n", MAX_NEST_LEVEL); return -EMLINK; @@ -708,23 +716,23 @@ int parse_pxefile_top(struct pxe_context *ctx, char *p, while (1) { s = p; free(t.val); - get_token(&p, &t, L_KEYWORD); + get_token(&p, &t, L_KEYWORD, limit); err = 0; switch (t.type) { case T_MENU: cfg->prompt = 1; - err = parse_menu(ctx, &p, cfg, nest_level); + err = parse_menu(ctx, &p, cfg, nest_level, limit); break; case T_TIMEOUT: - err = parse_integer(&p, &cfg->timeout); + err = parse_integer(&p, &cfg->timeout, limit); break; case T_LABEL: - err = parse_label(&p, cfg); + err = parse_label(&p, cfg, limit); break; case T_DEFAULT: case T_ONTIMEOUT: - err = parse_sliteral(&p, &label_name); + err = parse_sliteral(&p, &label_name, limit); if (label_name) { if (cfg->default_label) free(cfg->default_label); @@ -733,7 +741,7 @@ int parse_pxefile_top(struct pxe_context *ctx, char *p, } break; case T_FALLBACK: - err = parse_sliteral(&p, &label_name); + err = parse_sliteral(&p, &label_name, limit); if (label_name) { if (cfg->fallback_label) free(cfg->fallback_label); @@ -742,10 +750,10 @@ int parse_pxefile_top(struct pxe_context *ctx, char *p, } break; case T_INCLUDE: - err = handle_include(&p, cfg, nest_level); + err = handle_include(&p, cfg, nest_level, limit); break; case T_PROMPT: - err = parse_integer(&p, &cfg->prompt); + err = parse_integer(&p, &cfg->prompt, limit); // Do not fail if prompt configuration is undefined if (err < 0) eol_or_eof(&p); diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index b5320f2f6cf..45f4d365878 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -1007,13 +1007,15 @@ void pxe_menu_uninit(struct pxe_menu *cfg) struct pxe_menu *parse_pxefile(struct pxe_context *ctx, struct abuf *buf) { struct pxe_menu *cfg; + char *base; int r; cfg = pxe_menu_init(); if (!cfg) return NULL; - r = parse_pxefile_top(ctx, abuf_data(buf), cfg, 1); + base = abuf_data(buf); + r = parse_pxefile_top(ctx, base, base + buf->size, cfg, 1); if (r < 0) { pxe_menu_uninit(cfg); @@ -1053,7 +1055,7 @@ int pxe_process_includes(struct pxe_context *ctx, struct pxe_menu *cfg, return r; } - r = pxe_parse_include(ctx, inc, base); + r = pxe_parse_include(ctx, inc, base, ctx->pxe_file_size); if (r < 0) return r; @@ -1063,13 +1065,14 @@ int pxe_process_includes(struct pxe_context *ctx, struct pxe_menu *cfg, } int pxe_parse_include(struct pxe_context *ctx, const struct pxe_include *inc, - ulong addr) + ulong addr, ulong size) { char *buf; int ret; - buf = map_sysmem(addr, 0); - ret = parse_pxefile_top(ctx, buf, inc->cfg, inc->nest_level); + buf = map_sysmem(addr, size); + ret = parse_pxefile_top(ctx, buf, buf + size, inc->cfg, + inc->nest_level); unmap_sysmem(buf); return ret; diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 11be00da199..6f2ac604d6e 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -504,13 +504,17 @@ int pxe_load_label(struct pxe_context *ctx, struct pxe_label *label); */ int pxe_setup_label(struct pxe_context *ctx, struct pxe_label *label); -/* - * Entry point for parsing a menu file. nest_level indicates how many times - * we've nested in includes. It will be 1 for the top level menu file. +/** + * parse_pxefile_top() - Entry point for parsing a menu file * + * @ctx: PXE context + * @p: Start of buffer containing the PXE file + * @limit: End of buffer (position of nul terminator) + * @cfg: Menu to parse into + * @nest_level: Nesting level (1 for top level, higher for includes) * Returns 1 on success, < 0 on error. */ -int parse_pxefile_top(struct pxe_context *ctx, char *p, +int parse_pxefile_top(struct pxe_context *ctx, char *p, const char *limit, struct pxe_menu *cfg, int nest_level); /** @@ -538,9 +542,10 @@ void label_destroy(struct pxe_label *label); * @ctx: PXE context * @inc: Include info with path and target menu * @addr: Memory address where file is located + * @size: Size of the file in bytes * Return: 1 on success, -ve on error */ int pxe_parse_include(struct pxe_context *ctx, const struct pxe_include *inc, - ulong addr); + ulong addr, ulong size); #endif /* __PXE_UTILS_H */ -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add functions for a callback-free PXE API: - pxe_parse() - Parse a PXE config file and return an allocated context - pxe_load() - Record file load address and size - pxe_cleanup() - Free menu and destroy context Also rename pxe_do_boot() to pxe_boot() for consistency. This API allows callers to parse PXE files, load files manually without callbacks, and boot the selected label. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- boot/ext_pxe_common.c | 2 +- boot/pxe_utils.c | 45 +++++++++++++++++++++++++++++- include/pxe_utils.h | 64 +++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 106 insertions(+), 5 deletions(-) diff --git a/boot/ext_pxe_common.c b/boot/ext_pxe_common.c index 2bc76a4d0fb..59d878883bf 100644 --- a/boot/ext_pxe_common.c +++ b/boot/ext_pxe_common.c @@ -107,7 +107,7 @@ int extlinux_boot(struct udevice *dev, struct bootflow *bflow, /* if we have already selected a label, just boot it */ if (plat->ctx.label) { plat->ctx.fake_go = bflow->flags & BOOTFLOWF_FAKE_GO; - ret = pxe_do_boot(&plat->ctx); + ret = pxe_boot(&plat->ctx); } else { ret = extlinux_setup(dev, bflow, getfile, allow_abs_path, bootfile, &plat->ctx); diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c index 45f4d365878..5c1d08feebf 100644 --- a/boot/pxe_utils.c +++ b/boot/pxe_utils.c @@ -1326,6 +1326,49 @@ static struct pxe_menu *pxe_prepare(struct pxe_context *ctx, return cfg; } +struct pxe_context *pxe_parse(ulong addr, ulong size, const char *bootfile) +{ + struct pxe_context *ctx; + struct abuf buf; + int ret; + + ctx = calloc(1, sizeof(*ctx)); + if (!ctx) + return NULL; + + ret = pxe_setup_ctx(ctx, NULL, NULL, true, bootfile, false, false, + NULL); + if (ret) { + free(ctx); + return NULL; + } + ctx->pxe_file_size = size; + + abuf_init_addr(&buf, addr, size); + ctx->cfg = parse_pxefile(ctx, &buf); + if (!ctx->cfg) { + pxe_destroy_ctx(ctx); + free(ctx); + return NULL; + } + + return ctx; +} + +void pxe_load(struct pxe_file *file, ulong addr, ulong size) +{ + file->addr = addr; + file->size = size; +} + +void pxe_cleanup(struct pxe_context *ctx) +{ + if (ctx->cfg) + pxe_menu_uninit(ctx->cfg); + pxe_destroy_ctx(ctx); + free(ctx); +} + int pxe_process(struct pxe_context *ctx, ulong addr, ulong size, bool prompt) { struct pxe_menu *cfg; @@ -1366,7 +1409,7 @@ int pxe_probe(struct pxe_context *ctx, ulong pxefile_addr_r, bool prompt) return 0; } -int pxe_do_boot(struct pxe_context *ctx) +int pxe_boot(struct pxe_context *ctx) { int ret; diff --git a/include/pxe_utils.h b/include/pxe_utils.h index 6f2ac604d6e..bc9e3f29417 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -428,7 +428,7 @@ int pxe_get(ulong pxefile_addr_r, char **bootdirp, ulong *sizep, bool use_ipv6); * pxe_probe() - Process a PXE file to find the label to boot * * This fills in the label, etc. fields in @ctx, assuming it funds something to - * boot. Then pxe_do_boot() can be called to boot it. + * boot. Then pxe_boot() can be called to boot it. * * @ctx: PXE context created with pxe_setup_ctx() * @pxefile_addr_r: Address to load file @@ -438,13 +438,13 @@ int pxe_get(ulong pxefile_addr_r, char **bootdirp, ulong *sizep, bool use_ipv6); int pxe_probe(struct pxe_context *ctx, ulong pxefile_addr_r, bool prompt); /** - * pxe_do_boot() - Boot the selected label + * pxe_boot() - Boot the selected label * * This boots the label discovered by pxe_probe() * * Return: Does not return, on success, otherwise returns a -ve error code */ -int pxe_do_boot(struct pxe_context *ctx); +int pxe_boot(struct pxe_context *ctx); /** * pxe_select_label() - Select a label from a parsed menu @@ -539,6 +539,16 @@ void label_destroy(struct pxe_label *label); * 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. * + * Example usage:: + * + * ctx = pxe_parse(addr, size, bootfile); + * + * // Load and process any includes + * alist_for_each(inc, &ctx->cfg->includes) { + * // read file inc->path to address 'addr', getting 'size' + * pxe_parse_include(ctx, inc, addr, size); + * } + * * @ctx: PXE context * @inc: Include info with path and target menu * @addr: Memory address where file is located @@ -548,4 +558,52 @@ void label_destroy(struct pxe_label *label); int pxe_parse_include(struct pxe_context *ctx, const struct pxe_include *inc, ulong addr, ulong size); +/** + * pxe_parse() - Parse a PXE config file and return an allocated context + * + * This allocates a PXE context, sets it up, and parses the config file at the + * given address. The menu is stored in ctx->cfg and can be used to inspect + * labels or select one for loading. + * + * Use pxe_cleanup() to clean up when done. + * + * Note: This does not process include files. Call pxe_process_includes() + * after this if needed. + * + * @addr: Address where config file is loaded + * @size: Size of config file in bytes + * @bootfile: Path to config file (for relative path resolution) + * Return: Allocated context on success, NULL on error + */ +struct pxe_context *pxe_parse(ulong addr, ulong size, const char *bootfile); + +/** + * pxe_load() - Report that a file has been loaded + * + * After loading a file from a label's files list, call this to record + * the load address and size. This information is used later during boot. + * + * Example:: + * + * alist_for_each(file, &label->files) { + * // choose 'addr', load file->path there, getting 'size' + * pxe_load(file, addr, size); + * } + * + * @file: File that was loaded + * @addr: Address where file was loaded + * @size: Size of loaded file + */ +void pxe_load(struct pxe_file *file, ulong addr, ulong size); + +/** + * pxe_cleanup() - Free PXE menu and destroy context + * + * This combines pxe_menu_uninit() and pxe_destroy_ctx() for convenience + * when cleaning up after using the callback-free API. + * + * @ctx: Context to destroy (ctx->cfg is freed if set) + */ +void pxe_cleanup(struct pxe_context *ctx); + #endif /* __PXE_UTILS_H */ -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add pxe_test_files_api_norun() to test the new callback-free, file-loading API: 1. Parse config with pxe_parse() to get a menu with labels 2. Verify label->files contains expected files (kernel, initrd, fdt, overlays) with correct types and paths 3. Simulate loading by calling pxe_load() for each file 4. Verify addresses and sizes are recorded correctly Also update existing tests to expect the correct file counts now that kernel, initrd, and FDT are added to the files list during parsing. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/boot/pxe.c | 182 +++++++++++++++++++++++++++++-- test/py/tests/test_pxe_parser.py | 11 ++ 2 files changed, 184 insertions(+), 9 deletions(-) diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 56b1dfb4c49..80779b86aad 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -51,24 +51,22 @@ struct pxe_test_info { }; /** - * pxe_test_getfile() - Read a file from the host filesystem + * load_file() - Load a file from the host filesystem * - * This callback is used by the PXE parser to read included files. + * @path: Path to file within the mounted filesystem + * @addr: Address to load file to + * @sizep: Returns file size + * Return: 0 on success, -ve on error */ -static int pxe_test_getfile(struct pxe_context *ctx, const char *file_path, - ulong *addrp, ulong align, - enum bootflow_img_t type, ulong *sizep) +static int load_file(const char *path, ulong addr, ulong *sizep) { loff_t len_read; int ret; - if (!*addrp) - return -ENOTSUPP; - ret = fs_set_blk_dev("host", "0:0", FS_TYPE_ANY); if (ret) return ret; - ret = fs_legacy_read(file_path, *addrp, 0, 0, &len_read); + ret = fs_legacy_read(path, addr, 0, 0, &len_read); if (ret) return ret; *sizep = len_read; @@ -76,6 +74,21 @@ static int pxe_test_getfile(struct pxe_context *ctx, const char *file_path, return 0; } +/** + * pxe_test_getfile() - Read a file from the host filesystem + * + * This callback is used by the PXE parser to read included files. + */ +static int pxe_test_getfile(struct pxe_context *ctx, const char *file_path, + ulong *addrp, ulong align, + enum bootflow_img_t type, ulong *sizep) +{ + if (!*addrp) + return -ENOTSUPP; + + return load_file(file_path, *addrp, sizep); +} + /** * pxe_check_menu() - Check the standard menu output lines * @@ -1323,3 +1336,154 @@ static int pxe_test_fit_embedded_fdt_norun(struct unit_test_state *uts) PXE_TEST_ARGS(pxe_test_fit_embedded_fdt_norun, UTF_CONSOLE | UTF_MANUAL, { "fs_image", UT_ARG_STR }, { "cfg_path", UT_ARG_STR }); + +/** + * Test callback-free file loading API with pxe_load() + * + * This tests the new callback-free API where the caller: + * 1. Calls pxe_parse() to get a menu with labels containing files lists + * 2. Iterates over label->files and loads each file manually + * 3. Calls pxe_load() to record where each file was loaded + * + * This approach eliminates the need for getfile callbacks during loading. + */ +static int pxe_test_files_api_norun(struct unit_test_state *uts) +{ + const char *fs_image = ut_str(PXE_ARG_FS_IMAGE); + const char *cfg_path = ut_str(PXE_ARG_CFG_PATH); + struct pxe_context *ctx; + struct pxe_label *label; + struct pxe_menu *menu; + const struct pxe_file *file; + struct pxe_file *filep; + ulong addr = PXE_LOAD_ADDR; + ulong file_addr; + ulong size; + char *buf; + uint i; + + ut_assertnonnull(fs_image); + ut_assertnonnull(cfg_path); + + /* Bind the filesystem image */ + ut_assertok(run_commandf("host bind 0 %s", fs_image)); + + /* Load the config file first */ + ut_assertok(load_file(cfg_path, addr, &size)); + + /* Add null terminator - parser expects null-terminated string */ + buf = map_sysmem(addr, 0); + buf[size] = '\0'; + unmap_sysmem(buf); + + ctx = pxe_parse(addr, size, cfg_path); + ut_assertnonnull(ctx); + menu = ctx->cfg; + + /* Parsing with no getfile callback should produce no output */ + ut_assert_console_end(); + + /* + * Process includes manually - load each include file and parse it. + * Use an index-based loop since parsing may add more includes. + */ + for (i = 0; i < menu->includes.count; i++) { + const struct pxe_include *inc; + + inc = alist_get(&menu->includes, i, struct pxe_include); + ut_assertok(load_file(inc->path, addr, &size)); + + ut_asserteq(1, pxe_parse_include(ctx, inc, addr, size)); + } + + /* Include parsing should also produce no output */ + ut_assert_console_end(); + + /* Get the first label */ + label = list_first_entry(&menu->labels, struct pxe_label, list); + ut_asserteq_str("linux", label->name); + + /* + * Verify the files list contains expected files: + * - kernel (/vmlinuz) + * - initrd (/initrd.img) + * - fdt (/dtb/board.dtb) + * - 2 overlays (/dtb/overlay1.dtbo, /dtb/overlay2.dtbo) + */ + ut_asserteq(5, label->files.count); + + /* Check each file has correct type and path */ + file = alist_get(&label->files, 0, struct pxe_file); + ut_asserteq(PFT_KERNEL, file->type); + ut_asserteq_str("/vmlinuz", file->path); + ut_asserteq(0, file->addr); /* Not loaded yet */ + + file = alist_get(&label->files, 1, struct pxe_file); + ut_asserteq(PFT_INITRD, file->type); + ut_asserteq_str("/initrd.img", file->path); + + file = alist_get(&label->files, 2, struct pxe_file); + ut_asserteq(PFT_FDT, file->type); + ut_asserteq_str("/dtb/board.dtb", file->path); + + file = alist_get(&label->files, 3, struct pxe_file); + ut_asserteq(PFT_FDTOVERLAY, file->type); + ut_asserteq_str("/dtb/overlay1.dtbo", file->path); + + file = alist_get(&label->files, 4, struct pxe_file); + ut_asserteq(PFT_FDTOVERLAY, file->type); + ut_asserteq_str("/dtb/overlay2.dtbo", file->path); + + /* + * Load each file and call pxe_load() to record address/size. + * This demonstrates the callback-free file loading API. + */ + file_addr = PXE_KERNEL_ADDR; + alist_for_each(filep, &label->files) { + ulong size; + + ut_assertok(load_file(filep->path, file_addr, &size)); + pxe_load(filep, file_addr, size); + file_addr += ALIGN(size, SZ_64K); + } + + /* Verify files were loaded with valid addresses and sizes */ + file = alist_get(&label->files, 0, struct pxe_file); + ut_asserteq(PXE_KERNEL_ADDR, file->addr); + ut_assert(file->size > 0); + + /* + * Set up context for boot from the files list. + * In the callback-free API, the caller populates ctx fields. + */ + ctx->kern_addr = alist_get(&label->files, 0, struct pxe_file)->addr; + ctx->initrd_addr = alist_get(&label->files, 1, struct pxe_file)->addr; + ctx->fdt_addr = alist_get(&label->files, 2, struct pxe_file)->addr; + ctx->label = label; + + /* Boot - sandbox simulates this */ + pxe_boot(ctx); + + /* Sandbox cannot boot the kernel, so we get this error */ + ut_assert_nextline("Unrecognized zImage"); + ut_assert_console_end(); + + /* Verify the files are in memory at the correct addresses */ + file = alist_get(&label->files, 0, struct pxe_file); + buf = map_sysmem(file->addr, file->size); + ut_asserteq_mem("kernel", buf, 6); + unmap_sysmem(buf); + + file = alist_get(&label->files, 1, struct pxe_file); + buf = map_sysmem(file->addr, file->size); + ut_asserteq_mem("ramdisk", buf, 7); + unmap_sysmem(buf); + + /* Clean up */ + pxe_cleanup(ctx); + + return 0; +} +PXE_TEST_ARGS(pxe_test_files_api_norun, UTF_CONSOLE | UTF_MANUAL, + { "fs_image", UT_ARG_STR }, + { "cfg_path", UT_ARG_STR }); diff --git a/test/py/tests/test_pxe_parser.py b/test/py/tests/test_pxe_parser.py index bbcd63d58e2..5dfe9210b11 100644 --- a/test/py/tests/test_pxe_parser.py +++ b/test/py/tests/test_pxe_parser.py @@ -545,3 +545,14 @@ class TestPxeParser: with ubman.log.section('Test PXE FIT embedded FDT'): ubman.run_ut('pxe', 'pxe_test_fit_embedded_fdt', fs_image=fs_img, cfg_path=cfg_path) + + def test_pxe_files_api(self, ubman, pxe_image): + """Test three-phase files API (pxe_parse, pxe_load, pxe_boot) + + This tests the callback-free API where files are collected during + parsing and the caller loads them directly, then calls pxe_boot(). + """ + fs_img, cfg_path = pxe_image + with ubman.log.section('Test PXE files API'): + ubman.run_ut('pxe', 'pxe_test_files_api', + fs_image=fs_img, cfg_path=cfg_path) -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add a check for memory leaks in the callback-free files API test. This ensures that pxe_cleanup() properly frees all allocated memory. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- test/boot/pxe.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/boot/pxe.c b/test/boot/pxe.c index 80779b86aad..f4a124eafda 100644 --- a/test/boot/pxe.c +++ b/test/boot/pxe.c @@ -12,6 +12,7 @@ * */ +#include <blk.h> #include <dm.h> #include <env.h> #include <fdt_support.h> @@ -1358,6 +1359,7 @@ static int pxe_test_files_api_norun(struct unit_test_state *uts) struct pxe_file *filep; ulong addr = PXE_LOAD_ADDR; ulong file_addr; + ulong start_mem; ulong size; char *buf; uint i; @@ -1368,6 +1370,9 @@ static int pxe_test_files_api_norun(struct unit_test_state *uts) /* Bind the filesystem image */ ut_assertok(run_commandf("host bind 0 %s", fs_image)); + blkcache_free(); + start_mem = ut_check_delta(0); + /* Load the config file first */ ut_assertok(load_file(cfg_path, addr, &size)); @@ -1479,8 +1484,10 @@ static int pxe_test_files_api_norun(struct unit_test_state *uts) ut_asserteq_mem("ramdisk", buf, 7); unmap_sysmem(buf); - /* Clean up */ + /* Clean up and check for memory leaks */ pxe_cleanup(ctx); + blkcache_free(); + ut_assertok(ut_check_delta(start_mem)); return 0; } -- 2.43.0
From: Simon Glass <simon.glass@canonical.com> Add documentation for the PXE/extlinux parser API, covering both the traditional callback-based approach and the new callback-free API. The callback-free API separates parsing, loading, and booting into distinct phases, giving callers complete control over file operations. During parsing, the code collects file information (kernel, initrd, FDT, overlays) into a list that the caller can iterate over to load files manually. The documentation explains the file types, include handling, and provides an example showing typical usage of the callback-free API. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- doc/develop/bootstd/index.rst | 1 + doc/develop/bootstd/pxe_api.rst | 194 ++++++++++++++++++++++++++++++++ include/pxe_utils.h | 2 + 3 files changed, 197 insertions(+) create mode 100644 doc/develop/bootstd/pxe_api.rst diff --git a/doc/develop/bootstd/index.rst b/doc/develop/bootstd/index.rst index 342228064e6..35b7065aaad 100644 --- a/doc/develop/bootstd/index.rst +++ b/doc/develop/bootstd/index.rst @@ -9,6 +9,7 @@ Standard Boot overview extlinux pxelinux + pxe_api qfw android cros diff --git a/doc/develop/bootstd/pxe_api.rst b/doc/develop/bootstd/pxe_api.rst new file mode 100644 index 00000000000..18e4b6d8563 --- /dev/null +++ b/doc/develop/bootstd/pxe_api.rst @@ -0,0 +1,194 @@ +.. SPDX-License-Identifier: GPL-2.0+: + +PXE Parser API +============== + +The PXE parser handles configuration files in the extlinux/syslinux format, +which is widely used for boot menus on both local storage and network-boot +environments. This document describes the internal API for parsing these +configuration files and booting the selected operating system. + +Background +---------- + +The extlinux configuration format provides a simple way to define multiple boot +options, each with its own kernel, initial ramdisk, device tree, and command +line arguments. A typical configuration file looks like this:: + + menu title Boot Menu + timeout 50 + default linux + + label linux + menu label Ubuntu Linux + kernel /vmlinuz + initrd /initrd.img + fdt /dtb/board.dtb + fdtoverlays /dtb/overlay1.dtbo /dtb/overlay2.dtbo + append root=/dev/sda1 quiet + +The parser reads this configuration, presents a menu if appropriate, and boots +the selected label by loading the kernel and associated files into memory. + +Traditional API +--------------- + +The traditional approach uses callback functions to load files as they are +needed during parsing and booting. This works well when the caller has direct +access to the storage device and wants the PXE code to handle all file +operations. + +The entry point is ``pxe_process()``, which parses the configuration file, +handles the menu interaction, and boots the selected label. Before calling +this function, the caller must set up a context using ``pxe_setup_ctx()``, +providing a callback function that knows how to read files from the +appropriate source. + +The callback function receives a filename and a memory address, and is +responsible for loading the file contents to that address. This abstraction +allows the same parsing code to work with local filesystems, network TFTP, +or any other file source. + +When the parser encounters an ``include`` directive, it automatically calls +the callback to load the included file, then parses its contents. This +happens recursively for nested includes, up to a maximum depth of 16 levels. +The caller does not need to handle includes explicitly. + +For cases where the caller wants to inspect the parsed configuration before +booting, ``pxe_probe()`` provides a way to parse and select a label without +immediately booting. The caller can then examine the selected label's +properties and call ``pxe_boot()`` when ready to proceed. + +Callback-free API +----------------- + +Some callers prefer to handle file loading themselves rather than providing +callbacks. This is particularly useful in environments where file access +requires special handling, or where the caller wants complete control over +memory allocation and file placement. + +The callback-free API separates the boot process into distinct phases, giving +the caller full visibility into what files are needed and where they should +be loaded. + +The first phase uses ``pxe_parse()`` to parse the configuration file and +return a context containing a menu structure. The caller must first load the +configuration file into memory at a known address, then pass that address +and size to the parser. The function allocates and initialises the context +internally, so there is no need to call ``pxe_setup_ctx()`` beforehand. + +Note that ``pxe_parse()`` does not process ``include`` directives +automatically, since there is no callback to load files. The caller must +handle includes explicitly after parsing, as described below. + +During parsing, the code collects information about all files referenced by +each label. These are stored in a files list within each label structure, +with each entry recording the file path and type. The types distinguish +between kernels, initial ramdisks, device trees, and device tree overlays, +allowing the caller to handle each appropriately. + +After parsing, the caller can examine the menu structure to see what labels +are available and what files each one requires. For labels that use the +``include`` directive, the caller must load each included file and call +``pxe_parse_include()`` to merge its contents into the menu. The includes +list may grow as included files reference further includes, so the caller +should process includes in a loop until none remain. + +The second phase involves loading the files for the selected label. The +caller iterates over the label's files list, loads each file to an +appropriate memory address, and calls ``pxe_load()`` to record where the +file was placed. This function simply stores the address and size in the +file structure for later use. + +The final phase boots the selected label using ``pxe_boot()``. The caller +sets ``ctx->label`` to point to the selected label, and the function +automatically retrieves the kernel, initial ramdisk, and device tree +addresses from the files list. It then invokes the boot process, which +does not return on success. + +File Types +---------- + +The files list uses an enumeration to distinguish between different file +types. ``PFT_KERNEL`` indicates the kernel image, which may be a raw binary, +a compressed image, or a FIT image containing multiple components. +``PFT_INITRD`` marks the initial ramdisk, which the kernel uses as a +temporary root filesystem during early boot. ``PFT_FDT`` identifies the +flattened device tree that describes the hardware to the kernel. Finally, +``PFT_FDTOVERLAY`` marks device tree overlay files that modify the base +device tree, typically used to enable optional hardware or adjust +configuration. + +The caller can use these types to determine appropriate load addresses for +each file, or to apply special handling such as decompression or +verification. + +Include Handling +---------------- + +Configuration files may use the ``include`` directive to incorporate +additional configuration from other files. When using the traditional API +with callbacks, includes are processed automatically during parsing. + +With the callback-free API, includes require explicit handling. After the +initial parse, the menu's includes list contains entries for each include +directive encountered. Each entry records the path to the included file +and the nesting level. + +The caller loads each included file, adds a null terminator to the buffer +since the parser expects null-terminated strings, and calls +``pxe_parse_include()`` to parse and merge the contents. This may add more +entries to the includes list if the included file itself contains include +directives. Processing continues until all includes have been handled. + +The parser enforces a maximum nesting depth to prevent infinite recursion +from circular includes. + +Example Usage +------------- + +A typical use of the callback-free API follows this pattern:: + + struct pxe_context *ctx; + struct pxe_menu *menu; + struct pxe_label *label; + struct pxe_file *file; + ulong addr = CONFIG_SYS_LOAD_ADDR; + ulong file_addr; + ulong size; + + /* Load and parse the configuration file */ + size = load_config_file("/extlinux/extlinux.conf", addr); + ctx = pxe_parse(addr, size, "/extlinux/extlinux.conf"); + menu = ctx->cfg; + + /* Process any include directives */ + for (i = 0; i < menu->includes.count; i++) { + const struct pxe_include *inc; + + inc = alist_get(&menu->includes, i, struct pxe_include); + size = load_file(inc->path, addr); + pxe_parse_include(ctx, inc, addr, size); + } + + /* Select a label (here we just take the first one) */ + label = list_first_entry(&menu->labels, struct pxe_label, list); + + /* Load all files for this label */ + file_addr = KERNEL_LOAD_ADDR; + alist_for_each(file, &label->files) { + size = load_file(file->path, file_addr); + pxe_load(file, file_addr, size); + file_addr += ALIGN(size, SZ_64K); + } + + /* Boot - pxe_boot() gets addresses from the files list */ + ctx->label = label; + pxe_boot(ctx); + + /* Clean up (only reached if boot fails) */ + pxe_cleanup(ctx); + +This approach gives the caller complete control over file loading while +still benefiting from the parser's understanding of the configuration +format. diff --git a/include/pxe_utils.h b/include/pxe_utils.h index bc9e3f29417..48d36bdd14c 100644 --- a/include/pxe_utils.h +++ b/include/pxe_utils.h @@ -23,6 +23,8 @@ * pxe, and does a tftp download of a file listed as an include file in the * middle of the parsing operation. That could be handled by refactoring it to * take a 'include file getter' function. + * + * See doc/develop/bootstd/pxe_api.rst for API documentation. */ /** -- 2.43.0
participants (1)
-
Simon Glass