diff --git a/programs/network/ftpd/commands.inc b/programs/network/ftpd/commands.inc index 740559203..09f4c0e4f 100644 --- a/programs/network/ftpd/commands.inc +++ b/programs/network/ftpd/commands.inc @@ -15,6 +15,7 @@ struct thread_data state dd ? ; disconnected/logging in/logged in/.. passivesocknum dd ? ; when in passive mode, this is the listening socket pasv_ports_left dd ? ; ports left to try for the current PASV + pasv_accepts_left dd ? ; accept attempts left for the passive connection datasocketnum dd ? ; socket used for data transfers permissions dd ? ; read/write/execute/.... buffer_ptr dd ? @@ -155,8 +156,8 @@ close_data_sock: align 4 ; Close the passive listening socket if one is still open. It is open only -; while waiting for the client (MODE_PASSIVE_WAIT, MODE_PASSIVE_FAILED) or after -; a failed listen, so no data connection exists and the mode drops to NOTREADY. +; while waiting for the client (MODE_PASSIVE_WAIT) or after a failed listen, +; so no data connection exists and the mode drops to NOTREADY. close_pasv_sock: cmp [ebp + thread_data.passivesocknum], -1 je .done @@ -350,17 +351,18 @@ open_datasock: jne .socketerror .try_now: + mov dword [ebp + thread_data.pasv_accepts_left], 10 ; do not retry accept forever + .try_again: mov ecx, [ebp + thread_data.passivesocknum] lea edx, [ebp + thread_data.datasock] mov esi, sizeof.thread_data.datasock mcall accept cmp eax, -1 jne .pasv_ok - mov [ebp + thread_data.mode], MODE_PASSIVE_FAILED ; assume that we will fail mcall 23, 200 - mcall accept - cmp eax, -1 - je .socketerror + dec dword [ebp + thread_data.pasv_accepts_left] + jnz .try_again + jmp .socketerror .pasv_ok: mov [ebp + thread_data.datasocketnum], eax mov [ebp + thread_data.mode], MODE_PASSIVE_OK diff --git a/programs/network/ftpd/ftpd.asm b/programs/network/ftpd/ftpd.asm index 0f57e9a95..d8ae96a9c 100644 --- a/programs/network/ftpd/ftpd.asm +++ b/programs/network/ftpd/ftpd.asm @@ -38,7 +38,6 @@ MODE_NOTREADY = 0 MODE_ACTIVE = 1 MODE_PASSIVE_WAIT = 2 MODE_PASSIVE_OK = 3 -MODE_PASSIVE_FAILED = 4 PERMISSION_EXEC = 1b ; LIST PERMISSION_READ = 10b @@ -139,7 +138,11 @@ start: add esp, 8 ; open listening socket - mcall socket, AF_INET4, SOCK_STREAM, SO_NONBLOCK ; we don't want to block on accept +; Blocking on purpose: the accept happens in open_datasock, on the transfer +; command. SO_NONBLOCK used to sit here in the protocol slot and did nothing, +; because socket_open reads it from the type, and protocol is overwritten +; with IP_PROTO_TCP anyway. + mcall socket, AF_INET4, SOCK_STREAM, 0 cmp eax, -1 je sock_err mov [socketnum], eax @@ -263,21 +266,9 @@ threadloop: test eax, eax jz threadloop - cmp [ebp + thread_data.mode], MODE_PASSIVE_WAIT - jne .not_passive - mov ecx, [ebp + thread_data.passivesocknum] - lea edx, [ebp + thread_data.datasock] - mov esi, sizeof.thread_data.datasock - mcall accept - cmp eax, -1 - je .not_passive - mov [ebp + thread_data.datasocketnum], eax - mov [ebp + thread_data.mode], MODE_PASSIVE_OK - mcall close, [ebp + thread_data.passivesocknum] - mov [ebp + thread_data.passivesocknum], -1 - - invoke con_write_asciiz, str_datasock - .not_passive: +; The passive data connection is accepted in open_datasock, on the transfer +; command. Accepting it here, before the control socket is read, would park +; the thread in a blocking accept whenever a command arrives first. mov ecx, [ebp + thread_data.socketnum] mov edx, [ebp + thread_data.buffer_ptr]