When a missile explodes on launch, it has a 33% chance to damage its
base.
Unfortunately, damaging the base breaks callers that call msl_launch()
for each member of a list of missiles created by msl_sel() or
perform_mission(). Damage to the base can damage other missiles
there. Any copies of them in the list become stale. When
msl_launch() modifies and writes back such a stale copy, the damage
gets wiped out, triggering a seqno oops.
Affects missile interdiction and interception using missiles with
non-zero load. Stock game's ABMs have zero load, so interception is
safe there. Relatively harmless in practice. Broken in Empire 2.
Instead of fixing the bug, simply disable damage to the base for now.
March code reads the land units into a land unit list, and writes them
back when it changes them, e.g. when a land unit stops. If a land
unit changes in the land unit file while it is in such a land unit
list, the copy in the land unit list becomes stale, and must not be
used.
To that end, do_unit_move() calls lnd_mar() after prompting for path
or destination. lnd_mar() re-reads all the land units.
Unfortunately, it still writes back stale copies in certain
circumstances. Known ways to trigger such writes:
* Deity loads land unit onto a ship or land unit
* Land unit's crew killed just right, e.g. by collateral damage from
interdiction, followed by additional updates, such as shell fire
damage
* Sector no longer owned or allied, e.g. allied sector captured by an
enemy (own sector would kill or retreat the land unit)
Writing a stale copy wipes out the updates that made the copy stale,
and triggers a seqno mismatch oops. For instance, damage that follows
killing of all crew by collateral damage from interdiction is wiped
out. If no damage follows, we still get a generation oops.
Navigation code reads the ships into a ship list, and writes them back
when it changes them, e.g. when a ship stops. If a ship changes in
the ship file while it is in such a ship list, the copy in the ship
list becomes stale, and must not be used.
To that end, do_unit_move() calls shp_nav() after prompting for path
or destination. shp_nav() re-reads all the ships. Unfortunately, it
still writes back stale copies in certain circumstances. Known ways
to trigger such writes:
* Deity sets a sail path
* Ship's crew gone, e.g. killed by shell fire
* Sector no longer navigable, e.g. harbor shelled down, or bridge
built
Writing a stale copy wipes out the updates that made the copy stale,
and triggers a seqno mismatch oops. For instance, ship damage that
kills all crew while the ship is being navigated gets wiped out.
Commit 904822e3 introduced use of SHUT_WR, which Windows calls
SD_SEND. Add the obvious work-around.
Commit 49ae6a7b introduced use of gettimeofday(), which the Microsoft
CRT lacks. Add a replacement based on _ftime_s().
Fairland creates islands with size 1 + random() % (2 * is - 1), where
"is" is either chosen by the user (fourth command line argument) or
defaults to half the continent size (second command line argument).
Negative values are silently replaced by zero.
Not only does value zero make no sense, it also breaks the code: the
island size is always one then (because random() % -1 is zero), but
allocate_memory() provides only space for zero sectors in sectx[],
secty[] and sectc[]. This leads to buffer overruns in try_to_grow(),
find_coast(), elevate_land, set_coastal_flags(). Can smash the heap.
Fix by changing the lower bound from zero to one. Diagnosed with
valgrind. Has always been broken.
elevate_land() tests for capital sector in three places. The third
one is broken: half of the test is done even for islands, subscripting
capx[] and possibly capy[] out of bounds. This could screw up
elevation (unlikely) or crash (even less likely). Diagnosed with
valgrind.
Broken since the test was added in Chainsaw 3.12. Parenthesis were
added blindly 4.0.11 to shut up the compiler. Reindentation (commit
9b7adfbe and ef383c06, v4.2.13) made the bug stand out more, but it
still managed to hide in the general ugliness of fairland's code.
The journal logs a thread name for each event. The player thread name
changes on entry to the playing phase. Connecting old and new name
isn't as easy as it should be:
Sun Apr 29 12:13:39 2012 Conn29 input coun POGO
Sun Apr 29 12:13:39 2012 Conn29 input pass peter
Sun Apr 29 12:13:39 2012 Conn29 input play
Sun Apr 29 12:13:39 2012 Play#0 login 0 127.0.0.1 armbru
Sun Apr 29 12:15:39 2012 Play#0 logout 0
To connect Conn29 with Play#0, you have to know that country#0 is
named POGO.
Fix that by logging login before the thread name change:
Sun Apr 29 12:17:41 2012 Conn29 input coun POGO
Sun Apr 29 12:17:41 2012 Conn29 input pass peter
Sun Apr 29 12:17:41 2012 Conn29 input play
Sun Apr 29 12:17:41 2012 Conn29 login 0 127.0.0.1 armbru
Sun Apr 29 12:19:41 2012 Play#0 logout 0
Now "Conn29 login 0" makes the connection obvious.
This involves moving journal_login() from player_main() before
empth_set_name() in its caller play_cmd(). Move journal_logout() as
well, for symmetry.
If player_main() fails, we now log login/logout instead of nothing in
the journal. That's okay. Note that before commit c9f21c0e (v4.3.8),
we logged just login then.
It happily arms a plane with a remote nuke. The nuke gets teleported
to the plane when the plane moves (a two-way sortie doesn't count as
move). Broken in 4.3.3. Reported by Harald Katzer.
It was renamed to play_lock because it synchronized not just updates
but also shutdown. Since the previous commit, it again only
synchronizes updates. Rename it back.
Also move its initialization next to shutdown_lock's.
shutdwn() sets the EOF indicator, aborts the running command, if any,
forbids sleeping on I/O and wakes up the player thread, for all player
threads in state PS_PLAYING. It takes play_lock to prevent new
commands from running. It then waits up to 3s for player threads to
terminate, by polling player_next(), to let output buffers drain.
Issues:
1. Polling is lame.
2. New player threads can still enter state PS_PLAYING. They'll block
as soon as they try to run a command. Somehwat unclean.
3. We can exit before all player threads left state PS_PLAYING, losing
a treasury update, play time update, and log entries. Could happen
when player threads blocked on output until commit 90b3abc5 fixed
that; its commit message describes the bug's impact in more detail.
Since then, the bug shouldn't bite in practice, because player
threads should leave state PS_PLAYING quickly.
Fix by introducing shutdown_lock: player threads in state PS_PLAYING
hold it shared, shutdwn() takes it exclusive, instead of play_lock.
Takes care of the issues as follows:
3. shutdwn() waits until all player threads left state PS_PLAYING, no
matter how long it takes them.
2. New player threads block before entering state PS_PLAYING.
1. shutdwn() still polls up to 3s for player threads to terminate.
Still lame. Left for another day.
The victim's connection closes without any explanation. Output may be
lost. This is because kill_cmd() kills by calling io_shutdown(),
which shuts down the socket and drains the I/O queues.
How this makes the victim's thread terminate is a bit subtle: shutting
down the socket makes it ready. If the victim's thread is waiting for
I/O, it wakes up. Since all further reads return EOF, and all further
writes fail, the command terminates quickly (short of inifinite loop
bugs), then the command loop, and finally the thread.
To make kill behave more nicely, change kill_cmd() to work exactly
like server shutdown: send a flash message to the victim, set his EOF
indicator, abort the command, forbid sleeping on I/O, wake up the
victim's thread. Just as reliable, but doesn't lose output.
If the victim's client fails to close his connection, the victim's
thread may still linger in state PS_SHUTDOWN for up to
login_grace_time (default 120s). An attacker could try to use that to
make the server run out of file descriptors or memory, but simply
connecting achieves the same effect more cheaply.
max_idle applies in state PS_PLAYING, login_grace_time before (login,
state PS_INIT) and after (logout, state PS_SHUTDOWN).
Cut login_grace_time to two minutes, from max_idle's 15. Two minutes
is plenty to complete login and logout. Makes swamping the server
with connections slightly harder, as they get dropped faster. While
that makes sense all by itself, the real aim is making increasing
max_idle safe. The next commit will complete that job.
shutdwn() disables blocking on I/O for all player threads in state
PS_PLAYING, by setting struct player member may_sleep to
PLAYER_SLEEP_NEVER. This ensures the player threads complete logout
quickly and reliably. A thread may still block on I/O in io_close()
called from player_delete(), since commit 904822e3, but that's okay,
because it happens after all game state updates.
Bug: if shutdwn() aborts a command, the player thread returns through
dispatch(), which resets may_sleep back to PLAYER_SLEEP_FREELY. Input
can't block regardless, because the EOF indicator is set, but output
can. When it happens, the player thread may not complete logout
before shutdwn() terminates the process.
This can make us lose a treasury update (similar to the bug fixed by
commit bdc1c40f; the relevant bug description is in commit note
6f8ca87f), play time update, and log entries.
How? There are two paths from dispatch() to player_delete(). Here's
the first one:
1. command()
Doesn't print since dispatch() returns 0 when it resets may_sleep
2. player_main()
Loop and call status()
3. status()
If the command set dolcost to a non-trivial amount, print it
Charge dolcost
If player went broke or became solvent, notify him
Charge time used
Return 0, because shutdwn() set the EOF indicator
4. player_main()
Break the loop
Charge time used
print Bye-bye
journal.log the logout
5. play_cmd()
server.log the logout
6. player_login()
Loop
Try to flush output
get EOF, break loop
print so long
call player_delete()
Ways the bug can bite:
A. When we block in 4. print Bye-bye, we can fail to log.
B. When we block in 3. print broke/solvent notification, we can
additionally fail to charge time used.
C. When we block in 3. print dolcost, we can additionally fail to
charge dolcost.
Note: B. and C. couldn't happen before commit bdc1c40f. Instead,
something just like C happened always, whether player thread blocked
or not.
The second path:
1. execute()
Loop and call status()
2. status()
As above
3. execute()
break the loop
4. dispatch()
Continue with the first path
No additional ways to bite.
Fix by avoiding the may_sleep reset when the player thread is on its
way to terminate: may not sleep and has its EOF indicator set.
Broken in commit 0a4d77e9, v4.3.23.
The output queue flush can block indefinitely. Permits a client to
hog the thread indefinitely by not reading output.
Broken in commit 08b94556 (v4.3.20) "Reimplement max_idle without a
separate thread". Until then, the idle thread aborted a stuck attempt
to flush output.
Denial of service seems possible.
recvclient() flushes the output queue before receiving input. The
receive obeys max_idle, the flush doesn't.
Broken in commit 08b94556 (v4.3.20) "Reimplement max_idle without a
separate thread". Until then, the idle thread aborted a stuck attempt
to flush output.
We must not block in io_input() after command abortion unblocked
io_output(). Instead of checking player->aborted, compute the
deadline according to player->may_sleep, like we do for io_output().
player_login() flushes the output queue before receiving input. The
receive obeys max_idle, the flush doesn't. Which means a client could
hog the thread indefinitely.
Broken in commit 08b94556 (v4.3.20) "Reimplement max_idle without a
separate thread". Until then, the idle thread aborted a stuck attempt
to flush output.
Denial of service seems possible.
A client can delay thread exit indefinitely by not reading output.
Broken in commit 08b94556 (v4.3.20) "Reimplement max_idle without a
separate thread". Until then, the idle thread aborted a stuck attempt
to flush output.
Denial of service seems possible.
Note that commit 904822e3 moved flushing the output queue from
player_login() to io_close(). It also made io_close() wait for the
client to close the connection. That wait obeys the deadline.
The nightly build uses a gross hack to keep timestamps stable: it
replaces the system's time() by emp_time(). It doesn't replace other
time-related functions such as gettimeofday(). Works as long as we
don't mix hacked time with unhacked time.
The previous commit compares time gotten from gettimeofday() with time
gotten from time(). The nightly build's I/O timeouts become zero,
which makes login impossible.
Replace gettimeofday(), too.
Idle timeout used to expire max_idle minutes after the last
player->curup update. When we got rid of the idle thread in commit
08b94556 (v4.3.20), this got changed to "wait no more than max_idle
minutes for input". Time spent computing and time spent blocked on
output no longer counts. In particular, a connection can block
indefinitely on output since then. Let's fix that.
Start with basing the input timeout on player->curup again. The
missing output timeout will be added shortly.
Aside: since status() updates player->curup, the idle timer gets reset
when the update aborts a command. Left for another day.
Commit 08b94556 (v4.3.20) added io_open() parameter input_timeout. It
applies to io_input() and, since commit 904822e3, to io_close(). Add
timeout parameters to these functions instead.
Return zero when no input is available, regardless of parameter
waitforinput. Before, it returned -1 with errno set to EAGAIN or
EWOULDBLOCK when not waiting for input. Current callers all wait.
Drop errno from the function's contract, for consistency with
io_output().
Remote hole, can smash the stack. Additionally, the confirmation
prompt is misleading when the player supplies conditionals. Redesign
the flawed prompt.
Broken when Chainsaw added the confirmation prompt. Reported by Scott
C. Zielinski.
If execute's argument was read interactively, it prints the argument
instead of the command. Else, it prints uninitialized garbage. The
latter can theoretically crash the server, or leak information.
Broken in commit 3de1e8be, v4.3.28
lnd_take_casualty() uses uninitialized rsect to compute the mobility
cost of retreating a defending land unit. This can charge incorrect
mobility, prevent retreat, or, if the stars align just right, crash
the server when sector_mcost() subscripts dchr[] with it.
Broken in commit 4e7c993a, v4.3.6. Reported by Scott C. Zielinski.
play_cmd() needs to return in a state that makes player_login() break
the login command loop.
play_cmd() assumes player_main() always returns in such a state:
connection's EOF indicator set. Unfortunately, the assumption is
wrong. Fortunately, play_cmd() checks it, oopses and recovers.
player_main() can return with neiter error nor EOF indicator set,
e.g. when the game is down.
player_main() can return with just the error indicator set. For
instance, when the client dies, io_input() detects ECONNRESET, sets
the error indicator and fails.
Broken in commit 8549efbc. Fix by setting the EOF indicator silently
(without oopsing) when necessary.
C_MOD prevents print functions from blocking. The common reason for
wanting non-blocking prints are "read, print, write back" patterns.
There might be other reasons lurking in the code, and that's why
messing with C_MOD is scary.
Nevertheless, drop it from cutoff, headlines, land, lstat, motd,
neweff, payoff, qorder, sorder, and wall. These commands have had
C_MOD "forever", even though they clearly don't modify game state.
They're all pretty simple. Similar commands such as census, news,
ship, plane, nuke, sstat, pstat, flash never had C_MOD.
arm and disarm no longer require money, for consistency with the other
commands to move stuff around.
satellite no longer requires money, for consistency with lookout,
radar, sonar and skywatch.
qorder, sorder, survey and test no longer require a capital, for
consistency with the other commands that report on stuff the player
owns.
start and stop no longer require a capital, for consistency with the
other commands to control production.
fortify now requires a capital, for consistency with mission, morale
and range.
Create new command capability NONVIS. Give it to players in any state
except visitors (and STAT_UNUSED, but those must not exist). This
makes it possible to have commands available to anyone but visitors.
Command change fails when the player is a visitor. Simply make it
unavailable instead, by requiring NONVIS.
Make read unavailable to visitors, because it's useless: visitors
can't receive telegrams (typed_wu() fails).
Make census, commodity and sinfra unavailable to visitors. Visitors
don't normally have sectors.
Make map and nmap unavailable to visitors. Visitors don't have sectors,
so their maps are always empty.
Make them unavailable to new players (between add and newcap) and
players in sanctuary, too. This is consistent with all the other
commands to examine the environment. It also prevents people from
trying multiple unbroken countries in a blitz to find the one with the
nicest vicinity.
Make resource available to new players, for consistency with census
and commodity.
Make country, echo and financial available to anyone.
