[syslinux] [5.10] PXE + dhcp opts 209, 210 and path issues in tftp/http

Matt Fleming matt at console-pimps.org
Wed Jun 12 08:31:01 PDT 2013


>From f48d79be8c79241dd4635165e393683809edd823 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 ++++++-
 core/path.c                        | 42 ++++++++++++++++++++++++++++++
 doc/syslinux.txt                   |  2 +-
 7 files changed, 103 insertions(+), 68 deletions(-)
 create mode 100644 core/path.c

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/core/path.c b/core/path.c
new file mode 100644
index 0000000..8e517ca
--- /dev/null
+++ b/core/path.c
@@ -0,0 +1,42 @@
+/* ----------------------------------------------------------------------- *
+ *
+ *   Copyright 2013 Intel Corporation; author: Matt Fleming
+ *
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of the GNU General Public License as published by
+ *   the Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
+ *   Boston MA 02110-1301, USA; either version 2 of the License, or
+ *   (at your option) any later version; incorporated herein by reference.
+ *
+ * ----------------------------------------------------------------------- */
+
+#include <klibc/compiler.h>
+#include <linux/list.h>
+#include <fs.h>
+#include <string.h>
+
+__export LIST_HEAD(PATH);
+
+__export struct path_entry *path_add(const char *str)
+{
+    struct path_entry *entry;
+
+    if (!strlen(str))
+	return NULL;
+
+    entry = malloc(sizeof(*entry));
+    if (!entry)
+	return NULL;
+
+    entry->str = strdup(str);
+    if (!entry->str)
+	goto bail;
+
+    list_add(&entry->list, &PATH);
+
+    return entry;
+
+bail:
+    free(entry);
+    return NULL;
+}
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
-- 
1.8.1.4


-- 
Matt Fleming, Intel Open Source Technology Center


More information about the Syslinux mailing list