[syslinux] PATH directive searches in reverse order with wrong separator

[Phil Pokorny]

On Sun, Mar 5, 2017 at 4:23 PM, Phil Pokorny
<ppokorny at penguincomputing.com> wrote:
> But that will cause the code that attempts to put the CurrentDirName
> at the beginning of the list to break because it will end up at the
> end.  Probably better to move the check for current working directory
> location for a file to findpath() like this:
>
> diff --git a/com32/lib/sys/module/common.c b/com32/lib/sys/module/common.c
> --- a/com32/lib/sys/module/common.c
> +++ b/com32/lib/sys/module/common.c
> @@ -67,6 +67,14 @@ FILE *findpath(char *name)
>         if (f)
>                 return f;
>
> +       /* Look in CurrentDirName */
> +       /* CurrentDirName ends with a / (see efi_setcwd) */
> +       snprintf(path, sizeof(path), "%s%s", CurrentDirName, name);
> +       dprintf("findpath: trying \"%s\"\n", path);
> +       f = fopen(path, "rb"); /* for full path */
> +       if (f)
> +               return f;
> +
>         list_for_each_entry(entry, &PATH, list) {
>                 bool slash = false;
>
>
> but I'm worried that the code in the com32/lib/ tree expects to use
> chdir,(), get_cwd() and friends to manipulate the current working
> directory. (see core/fs/lib/searchconfig.c)  But that leads into a
> maze of twisty passages of virtual functions, default implementations
> and other stuff I haven't figured out yet.  Also, what's the
> difference between CurrentDirName[] and config_cwd[] and
> this_fs->cwd_name?

No, this is a bad idea.

Looking at fopen->open_file->searchdir call path, I think we can
assume that the fopen call above this patch already searched the
"current directory" and therefore there is no reason to search it
again.  We can probably kill CurrentDirName entirely now that we have
fs->ops with chdir() and get_cwd() and realname().  There aren't that
many places it's used.


> While trying to figure all that out, I found this in
> generic_mangle_name() which looks wrong to me.  When collapsing repeat
> slashes (should we also be stripping repeat backslashes here?), we
> decrement i which is the "space left" count, but we haven't actually
> copied a character.  I think that means that later, we'll end up
> padding with the incorrect number of characters.

There's another copy of that incorrect code in the vfat fs
implementation of mangle_name().  There is a strreplace() function in
the SYSLINUX library that could be used to convert backslash to
forward slash and then call the generic_mangle_name implementation
which would fix this bug and eliminate the duplicate code.

Unfortunately, I'm testing with an AMI Aptio-4 based BIOS (just
recently released updated version for this motherboard) and it still
has problems with the network stack.

Network sniffer traces show that when configured with pathprefix set
to "::/efi/" it repeatedly asks for the file "/efi/ldlinux.e64" from
the TFTP server but appears to ignore the replies and never fetches
the data.  After multiple failures, it tries the other
"search_directories" in order.

Changing pathprefix to "http://server/path/to/efi/" and copying the
files into place, it then does repeated TCP connection handshakes (SYN
-> SYN/ACK -> ACK) and then immediately (micro seconds later) issues a
FIN, then replies with RST when the server tries to FIN/ACK  No HTTP
headers are exchanged.

HTTP fails much faster than TFTP and so it appears to exhaust all
search_directories attempts in a few seconds and then aborts back to
the EFI BIOS.

UseDefault might have been part of this, but that appears to be fixed
now (6.04-pre1 + HEAD) and the message "disable UseDefault" is
printed.

I have an EFI version of GRUB (grubx64.efi) which works on this BIOS
and is able to TFTP a config file, kernel, initrd and launch Linux.
So there don't appear to be any network connectivity or other hardware
issues.

Any ideas?

-- 
Philip Pokorny, RHCE
Chief Technology Officer
PENGUIN COMPUTING, Inc
www.penguincomputing.com

Changing the world through technical innovation