apps/ftpd: accept the passive connection on the transfer command, not blindly #720

2026-09-25T20:23:51Z

igorsh commented

2026-09-25 20:23:51 +00:00

threadloop accepted on the PASV listener before reading the control
socket. That listener is blocking, so a command that arrives before the
client connects - PASV followed by anything - parks the thread inside
accept and it never reads the control connection again: the client
hangs. A second PASV did the same, while a client that opens the data
connection first worked, which is why this went unnoticed.

Accept in open_datasock instead, which runs when the transfer command
arrives and the data connection is actually needed. The client's
connection waits in the listen backlog, so connecting first still
works, and a command sent first now gets an answer instead of hanging
the server. Accept retries are bounded (10 x 200 ms) with a counter of
their own, so a persistent accept failure ends in 425 rather than the
single retry the code had; the MODE_PASSIVE_FAILED state, which only
ever marked "assume that we will fail", goes away with it, and so does
its constant.

The listening socket was also created with SO_NONBLOCK in the third
argument, under the comment "we don't want to block on accept". mcall
socket maps its arguments to ecx/edx/esi (domain/type/protocol) and
socket_open looks for SO_NONBLOCK in the type, so the flag sat in the
protocol slot and did nothing; protocol is ignored anyway, the
AF_INET4/SOCK_STREAM branch overwrites it with IP_PROTO_TCP. Pass 0
instead, so that the code and its comment agree that the listener
blocks - making it genuinely non-blocking is a separate change, since
the non-blocking accept path misbehaves on this kernel.

Assisted-by: ZCode:deepseek-flash

threadloop accepted on the PASV listener before reading the control socket. That listener is blocking, so a command that arrives before the client connects - PASV followed by anything - parks the thread inside accept and it never reads the control connection again: the client hangs. A second PASV did the same, while a client that opens the data connection first worked, which is why this went unnoticed. Accept in open_datasock instead, which runs when the transfer command arrives and the data connection is actually needed. The client's connection waits in the listen backlog, so connecting first still works, and a command sent first now gets an answer instead of hanging the server. Accept retries are bounded (10 x 200 ms) with a counter of their own, so a persistent accept failure ends in 425 rather than the single retry the code had; the MODE_PASSIVE_FAILED state, which only ever marked "assume that we will fail", goes away with it, and so does its constant. The listening socket was also created with SO_NONBLOCK in the third argument, under the comment "we don't want to block on accept". mcall socket maps its arguments to ecx/edx/esi (domain/type/protocol) and socket_open looks for SO_NONBLOCK in the type, so the flag sat in the protocol slot and did nothing; protocol is ignored anyway, the AF_INET4/SOCK_STREAM branch overwrites it with IP_PROTO_TCP. Pass 0 instead, so that the code and its comment agree that the listener blocks - making it genuinely non-blocking is a separate change, since the non-blocking accept path misbehaves on this kernel. Assisted-by: ZCode:deepseek-flash

igorsh added 1 commit 2026-09-25 20:23:51 +00:00

ftpd: accept the passive connection on the transfer command, not blindly

Test PR / Build (en_US) (pull_request) Successful in 2m3s

Details

Test PR / Build (es_ES) (pull_request) Successful in 2m5s

Details

Test PR / Build (ru_RU) (pull_request) Successful in 2m14s

Details

64c9b63c5a

threadloop accepted on the PASV listener before reading the control
socket. That listener is blocking, so a command that arrives before the
client connects - PASV followed by anything - parks the thread inside
accept and it never reads the control connection again: the client
hangs. A second PASV did the same, while a client that opens the data
connection first worked, which is why this went unnoticed.

Accept in open_datasock instead, which runs when the transfer command
arrives and the data connection is actually needed. The client's
connection waits in the listen backlog, so connecting first still
works, and a command sent first now gets an answer instead of hanging
the server. Accept retries are bounded (10 x 200 ms) with a counter of
their own, so a persistent accept failure ends in 425 rather than the
single retry the code had; the MODE_PASSIVE_FAILED state, which only
ever marked "assume that we will fail", goes away with it, and so does
its constant.

The listening socket was also created with SO_NONBLOCK in the third
argument, under the comment "we don't want to block on accept". mcall
socket maps its arguments to ecx/edx/esi (domain/type/protocol) and
socket_open looks for SO_NONBLOCK in the type, so the flag sat in the
protocol slot and did nothing; protocol is ignored anyway, the
AF_INET4/SOCK_STREAM branch overwrites it with IP_PROTO_TCP. Pass 0
instead, so that the code and its comment agree that the listener
blocks - making it genuinely non-blocking is a separate change, since
the non-blocking accept path misbehaves on this kernel.

Assisted-by: ZCode:deepseek-flash

CODEOWNERS rules requested review from network 2026-09-25 20:23:51 +00:00

All checks were successful

Test PR / Build (en_US) (pull_request) Successful in 2m3s

Required

Details

Test PR / Build (es_ES) (pull_request) Successful in 2m5s

Required

Details

Test PR / Build (ru_RU) (pull_request) Successful in 2m14s

Required

Details

This pull request doesn't have enough required approvals yet. 0 of 2 official approvals granted.

This pull request is blocked because it's outdated.

You are not authorized to merge this pull request.

This branch is out-of-date with the base branch

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u https://git.kolibrios.org/igorsh/kolibrios ftpd-accept-on-transfer-command:igorsh-ftpd-accept-on-transfer-command

git checkout igorsh-ftpd-accept-on-transfer-command

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KolibriOS/kolibrios#720

Allow edits from maintainers