FS#3807 - Too long paths will result in an assert error during startup

Attached to Project: OpenTTD
Opened by Peter Henschel (Progman) - Sunday, 02 May 2010, 17:58 GMT
Last edited by Remko Bijker (Rubidium) - Monday, 10 May 2010, 09:59 GMT
Type Bug
Category Core
Status Closed
Assigned To No-one
Operating System Linux
Severity Very Low
Priority Normal
Reported Version trunk
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No



If you have very long paths inside ~/.openttd/ OpenTTD can crash if during startup a search path is generated which does not end in a /. This results in the following assertion

Assertion failed at line 102 of /home/progman/openttd/trunk/src/os/unix/unix.cpp: path[strlen(path) - 1] == PATHSEPCHAR

This can be reproduced by creating two symlinks in content_download/ with the names "recursion" and "recursion2" which points to "." (content_download). During startup the content_download/ directory is searched recursively for all files. With these symlinks it creates a path which is greater than the "path" array and so the path is truncated. This also means the trailing slash is cut off, so the assertion is thrown. In the attached strace log you see how the paths are generated and how a directory path is generated which does not end in a slash.

The question of course is why would someone create such symlinks in a directory which is recursively loaded ;). Adding such of symlinks increase the load time during startup, specially if the names are very short (like "a"->. and "b"->.). These would result in an endless recursion lookup of a binary tree (only terminated by ELOOP (Too many levels of symbolic links)). Maybe a realpath() call can help here.
This task depends upon

Closed by  Remko Bijker (Rubidium)
Monday, 10 May 2010, 09:59 GMT
Reason for closing:  Fixed
Additional comments about closing:  Around r19780.
Comment by Remko Bijker (Rubidium) - Saturday, 08 May 2010, 21:30 GMT
Does the attached patch fix the problem?
Comment by Peter Henschel (Progman) - Saturday, 08 May 2010, 21:57 GMT
It does not. Even if the path does not end with a slash as it is cut off from the path variable maximum size the path itself is still valid. As you see in the strace output it runs the lstat() on "/home/progman...../recursion", which is a valid existing path, it just doesn't end with a slash.
Comment by Remko Bijker (Rubidium) - Saturday, 08 May 2010, 22:31 GMT
Okay, so the ELOOP error is not (yet) triggered... too bad.
Also realpath is basically an unusable function, at least without deliberately crashing older libcs and/or not working properly on non-POSIX 2008 but POSIX 2001 platforms.

In other words, this is going to require something more drastical.
Comment by Remko Bijker (Rubidium) - Sunday, 09 May 2010, 22:23 GMT
Does the attached patch fix the problem?
Comment by Peter Henschel (Progman) - Sunday, 09 May 2010, 23:31 GMT
Yes, this works now. I don't get an assert anymore. Its funny to see via strace how the new size limit (4096 on my system) is used to build any possible path of combination of /recursion/ and /recursion2/ ;) (about 2^350)
Comment by Remko Bijker (Rubidium) - Monday, 10 May 2010, 09:59 GMT
I doubt every path combination is tested if it is 2 to the power of 350 combinations. Just assume that every check takes exactly 1 CPU cycle (it'll take many many more), your CPU can do 4 billion ticks per second (2^32) (4 GHz, single core), then it would take 2^318 seconds to complete. One hour is 86400 seconds, so 2^17 if rounded up, which means doing it completely would have taken at least 2^300 times longer.

A year has about 2^25 seconds, the universe is about 15 billion years old so 2^34 years. Which means it would take at least 2^240 times as long as the (current) universe has lasted, unless we assume that the universe is God's creation and is considerably less than 15 billion years old, but that still means at least 2^260 times as long.

As such I highly doubt it would have checked even near 2^350 combinations.