[syslinux] [5.10] PXE + dhcp opts 209, 210 and path issues in tftp/http
Gerardo Exequiel Pozzi
vmlinuz386 at yahoo.com.ar
Wed Jun 12 07:17:44 PDT 2013
On 06/12/2013 09:40 AM, Matt Fleming wrote:
> On Tue, 11 Jun, at 03:54:21AM, H. Peter Anvin wrote:
>> On 06/11/2013 01:03 AM, Matt Fleming wrote:
>>> On Mon, 10 Jun, at 07:57:50AM, H. Peter Anvin wrote:
>>>> Either that or make the path a list rather than a string, using the
>>>> normal word separators when entered on the command line, a bit like the
>>>> (t)csh does. That is a bigger change but is probably a better solution.
>>>
>>> How would this solution handle filenames containing spaces? Would we
>>> need to escape (presumably with a backslash) such filenames?
>>>
>>
>> Well, we currently don't support filenames with spaces at all. We would
>> have to come up with new syntax for that (quoted strings, what have you)
>> but then perhaps we can use it elsewhere, too.
>
> OK, how about something like this? It also change the internal PATH into
> a linked list instead of requiring us to parse a string constantly;
> which was complete madness.
Cool thanks!. Now looks better, but still not work.
For some reason, "ldlinux.c32" is apparently sent but "Failed to load"
by PXELINUX and few seconds later, dnsmasq shows an error message
"failed sending":
dnsmasq-tftp: sent /run/archiso/bootmnt/arch/boot/syslinux/lpxelinux.0
to 192.168.0.90
dnsmasq-tftp: file /run/archiso/bootmnt/arch/ldlinux.c32 not found
dnsmasq-tftp: file /run/archiso/bootmnt/arch//ldlinux.c32 not found
dnsmasq-tftp: file /run/archiso/bootmnt/arch//boot/isolinux/ldlinux.c32
not found
dnsmasq-tftp: file /run/archiso/bootmnt/arch//boot/isolinux/ldlinux.c32
not found
dnsmasq-tftp: file /run/archiso/bootmnt/arch//isolinux/ldlinux.c32 not
found
dnsmasq-tftp: file /run/archiso/bootmnt/arch//boot/syslinuxldlinux.c32
not found
dnsmasq-tftp: sent /run/archiso/bootmnt/arch//boot/syslinux/ldlinux.c32
to 192.168.0.90
dnsmasq-tftp: failed sending
/run/archiso/bootmnt/arch//boot/syslinux/ldlinux.c32 to 192.168.0.90
Note that there are some still wrong thing here: "/syslinuxldlinux.c32",
and double slash but I guess it does not cause any harm.
Good Luck!
>
> ---
>
>>From a78a64f06c1092c7e0e060be69f8aaf9679a6400 Mon Sep 17 00:00:00 2001
> From: Matt Fleming <matt.fleming at intel.com>
> Date: Wed, 12 Jun 2013 13:04:44 +0100
> Subject: [PATCH] PATH: Change the PATH directive syntax
>
> In retrospect, choosing the colon character as the entry separator for
> the PATH directive was not a smart move, as that character is also used
> in TFTP-style paths. This conflict manifests as PXELINUX being unable to
> find and load files.
>
> An example dnsmasq log looks like,
>
> dnsmasq-tftp: sent /arch/boot/syslinux/lpxelinux.0 to 192.168.0.90
> dnsmasq-tftp: file /arch/ldlinux.c32 not found
> dnsmasq-tftp: file /arch//ldlinux.c32 not found
> dnsmasq-tftp: file /arch//boot/isolinux/ldlinux.c32 not found
> dnsmasq-tftp: file /arch//isolinux/ldlinux.c32 not found
> dnsmasq-tftp: file /arch//boot/syslinuxldlinux.c32 not found
> dnsmasq-tftp: sent /arch//boot/syslinux/ldlinux.c32 to 192.168.0.90
> dnsmasq-tftp: error 0 No error, file close received from 192.168.0.90
> dnsmasq-tftp: failed sending /arch//boot/syslinux/ldlinux.c32 to 192.168.0.90
> dnsmasq-tftp: sent /arch/boot/syslinux/archiso.cfg to 192.168.0.90
> dnsmasq-tftp: sent /arch/boot/syslinux/whichsys.c32 to 192.168.0.90
> dnsmasq-tftp: file /arch/libcom32.c32 not found
> dnsmasq-tftp: file /arch//libcom32.c32 not found
> dnsmasq-tftp: file /arch/libcom32.c32 not found
> dnsmasq-tftp: file /arch//arch//boot/syslinux/libcom32.c32 not found
>
> The last line of the log is the indication that there's a problem.
> Internally, Syslinux adds the location of ldlinux.c32 to PATH by
> querying the current working directory once ldlinux.c32 is successfully
> loaded. Under PXELINUX that means the initial PATH string will be,
>
> "::/arch/boot/syslinux/"
>
> The PATH parsing code doesn't know how to correctly parse the "::"
> string and hence, the file is searched for relative to the 210 dhcp
> option directory - /arch/.
>
> Switch to using the space character to separate PATH entries instead,
> which not only allows TFTP-style paths but also http and ftp urls. If we
> ever support filenames containing spaces in config files, then we'll
> have to introduce some means of quotation, to distinguish filenames
> containing spaces from separate PATH entries.
>
> Internally, PATH is now a linked list which *greatly* simplifies the
> path code, and means we no longer have to parse strings backwards and
> forwards.
>
> Note: This change is not backwards-compatible, which means that anyone
> using the PATH directive with versions before 5.11 will need to change
> their config file when upgrading.
>
> Signed-off-by: Matt Fleming <matt.fleming at intel.com>
> ---
> com32/elflink/ldlinux/readconfig.c | 53 +++++++++++++++++++++++++-------------
> com32/lib/sys/module/common.c | 36 +++++++++-----------------
> core/elflink/load_env32.c | 26 +++----------------
> core/fs/fs.c | 2 --
> core/include/fs.h | 10 ++++++-
> doc/syslinux.txt | 2 +-
> 6 files changed, 61 insertions(+), 68 deletions(-)
>
> diff --git a/com32/elflink/ldlinux/readconfig.c b/com32/elflink/ldlinux/readconfig.c
> index 35b137e..12cb7e3 100644
> --- a/com32/elflink/ldlinux/readconfig.c
> +++ b/com32/elflink/ldlinux/readconfig.c
> @@ -770,6 +770,39 @@ extern void loadkeys(char *);
> extern char syslinux_banner[];
> extern char copyright_str[];
>
> +/*
> + * PATH-based lookup
> + *
> + * Each entry in the PATH directive is separated by a space, e.g.
> + *
> + * PATH /bar /bin/foo /baz/bar/bin
> + */
> +static int parse_path(char *p)
> +{
> + struct path_entry *entry;
> + const char *str;
> +
> + while (*p) {
> + /* Find the next directory */
> + str = refdup_word(&p);
> + if (!str)
> + goto bail;
> +
> + entry = path_add(str);
> + refstr_put(str);
> +
> + if (!entry)
> + goto bail;
> +
> + p = skipspace(p);
> + }
> +
> + return 0;
> +
> +bail:
> + return -1;
> +}
> +
> static void parse_config_file(FILE * f)
> {
> char line[MAX_LINE], *p, *ep, ch;
> @@ -1337,24 +1370,8 @@ do_include:
> } else if (looking_at(p, "say")) {
> printf("%s\n", p+4);
> } else if (looking_at(p, "path")) {
> - /* PATH-based lookup */
> - const char *new_path;
> - char *_p;
> - size_t len, new_len;
> -
> - new_path = refstrdup(skipspace(p + 4));
> - len = strlen(PATH);
> - new_len = strlen(new_path);
> - _p = malloc(len + new_len + 2);
> - if (_p) {
> - strncpy(_p, PATH, len);
> - _p[len++] = ':';
> - strncpy(_p + len, new_path, new_len);
> - _p[len + new_len] = '\0';
> - free(PATH);
> - PATH = _p;
> - } else
> - printf("Failed to realloc PATH\n");
> + if (parse_path(skipspace(p + 4)))
> + printf("Failed to parse PATH\n");
> } else if (looking_at(p, "sendcookies")) {
> const union syslinux_derivative_info *sdi;
>
> diff --git a/com32/lib/sys/module/common.c b/com32/lib/sys/module/common.c
> index 8547036..b763704 100644
> --- a/com32/lib/sys/module/common.c
> +++ b/com32/lib/sys/module/common.c
> @@ -59,40 +59,28 @@ void print_elf_symbols(struct elf_module *module) {
>
> FILE *findpath(char *name)
> {
> + struct path_entry *entry;
> char path[FILENAME_MAX];
> FILE *f;
> - char *p, *n;
> - int i;
>
> f = fopen(name, "rb"); /* for full path */
> if (f)
> return f;
>
> - p = PATH;
> -again:
> - i = 0;
> - while (*p && *p != ':' && i < FILENAME_MAX - 1) {
> - path[i++] = *p++;
> - }
> -
> - if (*p == ':')
> - p++;
> + list_for_each_entry(entry, &PATH, list) {
> + bool slash = false;
>
> - /* Ensure we have a '/' separator */
> - if (path[i] != '/' && i < FILENAME_MAX - 1)
> - path[i++] = '/';
> + /* Ensure we have a '/' separator */
> + if (entry->str[strlen(entry->str) - 1] != '/')
> + slash = true;
>
> - n = name;
> - while (*n && i < FILENAME_MAX - 1)
> - path[i++] = *n++;
> - path[i] = '\0';
> + snprintf(path, sizeof(path), "%s%s%s",
> + entry->str, slash ? "/" : "", name);
>
> - f = fopen(path, "rb");
> - if (f)
> - return f;
> -
> - if (p >= PATH && p < PATH + strlen(PATH))
> - goto again;
> + f = fopen(path, "rb");
> + if (f)
> + return f;
> + }
>
> return NULL;
> }
> diff --git a/core/elflink/load_env32.c b/core/elflink/load_env32.c
> index 0483d86..8551831 100644
> --- a/core/elflink/load_env32.c
> +++ b/core/elflink/load_env32.c
> @@ -124,14 +124,11 @@ void load_env32(com32sys_t * regs __unused)
>
> dprintf("Starting 32 bit elf module subsystem...\n");
>
> - PATH = malloc(strlen(CurrentDirName) + 1);
> - if (!PATH) {
> + if (strlen(CurrentDirName) && !path_add(CurrentDirName)) {
> printf("Couldn't allocate memory for PATH\n");
> goto out;
> }
>
> - strcpy(PATH, CurrentDirName);
> -
> init_module_subsystem(&core_module);
>
> start_ldlinux(1, argv);
> @@ -159,30 +156,15 @@ void load_env32(com32sys_t * regs __unused)
> if (!core_getcwd(path, sizeof(path)))
> goto out;
>
> - if (!strlen(PATH)) {
> - PATH = realloc(PATH, strlen(path) + 1);
> - if (!PATH) {
> - printf("Couldn't allocate memory for PATH\n");
> - goto out;
> - }
> -
> - strcpy(PATH, path);
> - } else {
> - PATH = realloc(PATH, strlen(path) + strlen(PATH) + 2);
> - if (!PATH) {
> - printf("Couldn't allocate memory for PATH\n");
> - goto out;
> - }
> -
> - strcat(PATH, ":");
> - strcat(PATH, path);
> + if (!path_add(path)) {
> + printf("Couldn't allocate memory for PATH\n");
> + goto out;
> }
>
> start_ldlinux(1, argv);
> }
>
> out:
> - free(PATH);
> writestr("\nFailed to load ldlinux.c32");
> }
>
> diff --git a/core/fs/fs.c b/core/fs/fs.c
> index 1cb4b00..b6ee19c 100644
> --- a/core/fs/fs.c
> +++ b/core/fs/fs.c
> @@ -10,8 +10,6 @@
> #include "fs.h"
> #include "cache.h"
>
> -__export char *PATH;
> -
> /* The currently mounted filesystem */
> __export struct fs_info *this_fs = NULL; /* Root filesystem */
>
> diff --git a/core/include/fs.h b/core/include/fs.h
> index c7d0fd7..b5c7f0d 100644
> --- a/core/include/fs.h
> +++ b/core/include/fs.h
> @@ -1,6 +1,7 @@
> #ifndef FS_H
> #define FS_H
>
> +#include <linux/list.h>
> #include <stddef.h>
> #include <stdbool.h>
> #include <string.h>
> @@ -182,7 +183,14 @@ static inline struct file *handle_to_file(uint16_t handle)
> return handle ? &files[handle-1] : NULL;
> }
>
> -extern char *PATH;
> +struct path_entry {
> + struct list_head list;
> + const char *str;
> +};
> +
> +extern struct list_head PATH;
> +
> +extern struct path_entry *path_add(const char *str);
>
> /* fs.c */
> void pm_mangle_name(com32sys_t *);
> diff --git a/doc/syslinux.txt b/doc/syslinux.txt
> index a4b201f..0dfecee 100644
> --- a/doc/syslinux.txt
> +++ b/doc/syslinux.txt
> @@ -551,7 +551,7 @@ F12 filename
> <Ctrl-F>0.
>
> PATH path
> - Specify a colon-separated (':') list of directories to search
> + Specify a list of directories (separated by spaces) to search
> when attempting to load modules. This directive is useful for
> specifying the directories containing the lib*.c32 library
> files as other modules may be dependent on these files, but
>
--
Gerardo Exequiel Pozzi
\cos^2\alpha + \sin^2\alpha = 1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 555 bytes
Desc: OpenPGP digital signature
URL: <http://www.zytor.com/pipermail/syslinux/attachments/20130612/b4e23cac/attachment.sig>
More information about the Syslinux
mailing list