KolibriOS/kolibrios
15
24
Fork 61
Code Issues 132 Pull Requests 32 Actions Projects 1 Releases Wiki Activity

ftpd: Add MKD command #342

Merged
Burer merged 12 commits from igorsh/kolibrios:ftpd-add-mkd-command into main 2026-06-07 17:12:23 +00:00
Conversation 12 Commits 12 Files Changed 2
+151 -11
igorsh commented 2026-02-23 14:13:32 +00:00
Contributor
Copy Link
Copy Source

Added MKD command

Added MKD command
mxlgv requested changes 2026-02-23 21:40:32 +00:00
Dismissed
programs/network/ftpd/commands.inc Outdated
@@ -334,2 +334,4 @@
ret
align 4
console_fs_error:
mxlgv commented 2026-02-23 21:38:35 +00:00
Owner
Copy Link
Copy Source

Don't forget the server in floppy disk image. I think here you can write more optimally, for example, get a line with an error by index.

Don't forget the server in floppy disk image. I think here you can write more optimally, for example, get a line with an error by index.
mxlgv marked this conversation as resolved
programs/network/ftpd/commands.inc Outdated
@@ -1251,0 +1322,4 @@
mov byte [edi], 0x00
call create_path
dec edi
mxlgv commented 2026-02-23 21:39:07 +00:00
Owner
Copy Link
Copy Source

Bad ident

Bad ident
mxlgv marked this conversation as resolved
mxlgv added the Category/ApplicationsFASM
Kind
Feature
Priority
Low
PR
Request changes
 labels 2026-02-23 22:16:44 +00:00
mxlgv requested changes 2026-02-25 18:16:13 +00:00
Dismissed
programs/network/ftpd/commands.inc Outdated
@@ -336,0 +337,4 @@
fs_err_table:
dd str_fs_error2 ; 2
dd str_fs_error3 ; 3
dd str_fs_error5 ; 5
mxlgv commented 2026-02-25 18:00:29 +00:00
Owner
Copy Link
Copy Source

I suggest using 4 as str_fs_error

I suggest using 4 as str_fs_error
Burer marked this conversation as resolved
programs/network/ftpd/commands.inc Outdated
@@ -336,0 +346,4 @@
dd str_fs_error11 ; 11
dd str_fs_error12 ; 12
console_fs_error:
mxlgv commented 2026-02-25 18:08:33 +00:00
Owner
Copy Link
Copy Source

I think you can shorten it something like this

console_fs_error:
        mov edx, 4 ;  Unknown filesystem error
       
        cmp     eax, 2
        jb      .print_err
        cmp     eax, 12
        ja       .print_err
       
        mov edx, eax
        
.print_err:
        sub     edx, 2
        mov     edx, [fs_err_table + edx*4]
        invoke  con_write_asciiz, edx
        ret
I think you can shorten it something like this ```asm console_fs_error: mov edx, 4 ; Unknown filesystem error cmp eax, 2 jb .print_err cmp eax, 12 ja .print_err mov edx, eax .print_err: sub edx, 2 mov edx, [fs_err_table + edx*4] invoke con_write_asciiz, edx ret ```
Burer marked this conversation as resolved
mxlgv requested changes 2026-02-25 18:29:38 +00:00
Dismissed
programs/network/ftpd/commands.inc Outdated
@@ -1251,0 +1323,4 @@
; called fs function
push ebx
dec esp
mov byte[esp], 0
mxlgv commented 2026-02-25 18:27:01 +00:00
Owner
Copy Link
Copy Source

May be xor eax, eax and push eax? I also don’t understand why there is mov and push here. push is smaller in size

May be `xor eax, eax` and `push eax`? I also don’t understand why there is `mov` and `push` here. `push` is smaller in size
igorsh commented 2026-02-28 07:43:59 +00:00
Author
Contributor
Copy Link
Copy Source

Эта часть скопирована с cmdDELE. Если не добавить 1 байт в стек - получаю Page Fault.

Эта часть скопирована с **cmdDELE**. Если не добавить 1 байт в стек - получаю Page Fault.
mxlgv marked this conversation as resolved
mxlgv requested changes 2026-03-02 17:41:28 +00:00
Dismissed
programs/network/ftpd/commands.inc Outdated
@@ -1251,0 +1331,4 @@
push dword SSF_CREATE_FOLDER
mov ebx, esp
mcall SF_FILE
add esp, 6*4 + 1
mxlgv commented 2026-03-02 16:51:06 +00:00
Owner
Copy Link
Copy Source

I guess the problem (https://git.kolibrios.org/KolibriOS/kolibrios/pulls/342/files#issuecomment-4485) is here

I guess the problem (https://git.kolibrios.org/KolibriOS/kolibrios/pulls/342/files#issuecomment-4485) is here
igorsh commented 2026-03-02 18:00:17 +00:00
Author
Contributor
Copy Link
Copy Source

No, at that line - mcall SF_FILE

No, at that line - `mcall SF_FILE`
mxlgv commented 2026-03-02 18:21:09 +00:00
Owner
Copy Link
Copy Source

You are right. I read SF 70 and understood why there is one byte.
But I would still recommend storing 0 in the register and then doing a push and mov.

You are right. I read SF 70 and understood why there is one byte. But I would still recommend storing 0 in the register and then doing a `push` and `mov`.
igorsh commented 2026-03-03 18:00:25 +00:00
Author
Contributor
Copy Link
Copy Source

But I would still recommend storing 0 in the register and then doing a push and mov.

Немного не пойму что ты предлагаешь:
push ebx
xor eax, eax
push eax
add esp, 3

вместо
push ebx
dec esp
mov byte[esp], 0
?

> But I would still recommend storing 0 in the register and then doing a push and mov. Немного не пойму что ты предлагаешь: `push ebx` `xor eax, eax` `push eax` `add esp, 3` вместо `push ebx` `dec esp` `mov byte[esp], 0` ?
mxlgv commented 2026-03-04 02:06:08 +00:00
Owner
Copy Link
Copy Source

I suggest leaving it as is. I'm already confused myself.

I suggest leaving it as is. I'm already confused myself.
mxlgv marked this conversation as resolved
mxlgv approved these changes 2026-03-07 20:48:54 +00:00
mxlgv removed the
PR
Request changes
 label 2026-03-07 20:49:08 +00:00
mxlgv requested review from Burer 2026-03-07 20:49:14 +00:00
Burer added 4 commits 2026-03-14 11:18:15 +00:00
ftpd: Add mkd command f7a257e062
ftpd: Fix indents 8c27e13d1b
ftpd: Add function to access filesystem errors by index 8306bd0930
ftpd: Simplify console_fs_error logic
Build system / Check kernel codestyle (pull_request) Successful in 23s
Details
Build system / Build (pull_request) Successful in 9m22s
Details
27c3bcbbd0
Burer force-pushed ftpd-add-mkd-command from e72e908c1a to 27c3bcbbd0 2026-03-14 11:18:15 +00:00 Compare
Burer requested changes 2026-03-14 11:39:47 +00:00
Dismissed
Burer left a comment
Owner
Copy Link
Copy Source

  1. MKD не проверяет, что путь вообще передан, и при пустом аргументе может попытаться создать текущий каталог вместо того, чтобы вернуть ошибку, в programs/network/ftpd/commands.inc (line 1348). В отличие от cmdDELE (line 496), cmdRETR (line 981) и cmdSTOR (line 1079), здесь нет проверки вида sub ecx, 5 / jb .... Для MKD\r\n код соберёт пустое имя, вызовет create_path, и дальше операция пойдёт по пути текущей директории, что выглядит как неверное поведение.

  2. MKD обрабатывает абсолютные пути не так, как остальные файловые команды, в programs/network/ftpd/commands.inc (line 1369). В DELE/RETR/STOR есть специальное пропускание ведущего / перед дописыванием к fpath (см. commands.inc (line 512), commands.inc (line 998), commands.inc (line 1099)). У MKD этого нет, поэтому MKD /foo будет собираться иначе, чем аналогичные операции над файлами. Это как минимум несогласованность поведения, а возможно и неправильная логика.

1. ~~MKD не проверяет, что путь вообще передан, и при пустом аргументе может попытаться создать текущий каталог вместо того, чтобы вернуть ошибку, в programs/network/ftpd/commands.inc (line 1348). В отличие от cmdDELE (line 496), cmdRETR (line 981) и cmdSTOR (line 1079), здесь нет проверки вида `sub ecx, 5 / jb ...`. Для MKD\r\n код соберёт пустое имя, вызовет create_path, и дальше операция пойдёт по пути текущей директории, что выглядит как неверное поведение.~~ 2. ~~MKD обрабатывает абсолютные пути не так, как остальные файловые команды, в programs/network/ftpd/commands.inc (line 1369). В DELE/RETR/STOR есть специальное пропускание ведущего / перед дописыванием к fpath (см. commands.inc (line 512), commands.inc (line 998), commands.inc (line 1099)). У MKD этого нет, поэтому MKD /foo будет собираться иначе, чем аналогичные операции над файлами. Это как минимум несогласованность поведения, а возможно и неправильная логика.~~
igorsh changed title from ftpd: Add mkd command to ftpd: Add MKD command 2026-03-15 06:50:51 +00:00
igorsh marked the pull request as work in progress 2026-03-21 06:27:39 +00:00
igorsh added 2 commits 2026-06-02 18:56:55 +00:00
ftpd: Check empty path 6406f71939
ftpd: Check "/"
Build system / Check kernel codestyle (pull_request) Successful in 26s
Details
Build system / Build (pull_request) Successful in 11m5s
Details
1d20c72bcd
igorsh marked the pull request as ready for review 2026-06-02 19:00:21 +00:00
igorsh added 1 commit 2026-06-02 19:06:31 +00:00
cmd_MKD -> cmdMKD
Build system / Check kernel codestyle (pull_request) Successful in 39s
Details
Build system / Build (pull_request) Successful in 10m39s
Details
41591d9264
Burer requested review from Doczom 2026-06-02 19:13:29 +00:00
Burer requested review from dunkaist 2026-06-02 19:13:29 +00:00
Burer commented 2026-06-05 10:16:42 +00:00
Owner
Copy Link
Copy Source

Hi, @igorsh.
PR is okay overall, but I propose this changes before it got merged.
Please, check them.

ftpd.asm
commands.inc

Hi, @igorsh. PR is okay overall, but I propose this changes before it got merged. Please, check them. [ftpd.asm](/attachments/966d763f-a55b-47e7-a890-19222908410f) [commands.inc](/attachments/8708c854-a594-4ed1-b999-4f7a83a14d0e)
ftpd.asm
15 KiB
commands.inc
37 KiB
Doczom requested changes 2026-06-05 14:12:21 +00:00
Dismissed
programs/network/ftpd/commands.inc
@@ -1313,0 +1355,4 @@
mov ecx, 1024
cmp byte[esi], 0x20
jb .error
Doczom commented 2026-06-05 14:10:23 +00:00
Owner
Copy Link
Copy Source

An undefined value in the eax register

An undefined value in the eax register
Burer marked this conversation as resolved
igorsh added 1 commit 2026-06-06 07:14:55 +00:00
ftpd: Improve code
Build system / Check kernel codestyle (pull_request) Successful in 1m33s
Details
Build system / Build (pull_request) Successful in 11m55s
Details
ad32a51776
Burer requested changes 2026-06-06 08:45:37 +00:00
Dismissed
Burer left a comment
Owner
Copy Link
Copy Source

1: Сломана проверка пустого аргумента

ecx включает завершающий CR/LF, поэтому проверка по длине не отличает пустое MKD\r\n (ecx=5, 5-5=0, не jb) от 1-символьного имени - пустой аргумент проходит, и SF_CREATE_FOLDER вызывается на текущем каталоге. Проверка первого байта аргумента надёжна независимо от CR/LF и разделителя.

Исправление:

-        sub     ecx, 5
+        cmp     byte [ebp + thread_data.buffer + 4], 0x20
         jb      .error

2: Ответ 257 отдаёт абсолютный путь сервера

fpath = home_dir + work_dir + имя (реальный путь на сервере): отдавать его в 257 - это утечка структуры ФС и поломка round-trip (клиент подставит путь в CWD => демон снова допишет home_dir). Эхо-им запрошенный клиентом путь (buffer+4); ответ собираем в fpath (после syscall не нужен), чтобы не было самоперекрытия буфера.

Исправление:

-        lea     edi, [ebp + thread_data.buffer]
+        lea     edi, [ebp + thread_data.fpath]
         mov     eax, '257 '
         stosd
         mov     al, '"'
         stosb
-        lea     esi, [ebp + thread_data.fpath]
+        lea     esi, [ebp + thread_data.buffer + 4]

     .reply_send:
         mov     ecx, [ebp + thread_data.socketnum]
-        lea     edx, [ebp + thread_data.buffer]
+        lea     edx, [ebp + thread_data.fpath]
1: Сломана проверка пустого аргумента `ecx` включает завершающий CR/LF, поэтому проверка по длине не отличает пустое `MKD\r\n` (ecx=5, `5-5=0`, не `jb`) от 1-символьного имени - пустой аргумент проходит, и `SF_CREATE_FOLDER` вызывается на текущем каталоге. Проверка первого байта аргумента надёжна независимо от CR/LF и разделителя. Исправление: ```diff - sub ecx, 5 + cmp byte [ebp + thread_data.buffer + 4], 0x20 jb .error ``` 2: Ответ 257 отдаёт абсолютный путь сервера `fpath` = `home_dir + work_dir + имя` (реальный путь на сервере): отдавать его в 257 - это утечка структуры ФС и поломка round-trip (клиент подставит путь в `CWD` => демон снова допишет `home_dir`). Эхо-им запрошенный клиентом путь (`buffer+4`); ответ собираем в `fpath` (после syscall не нужен), чтобы не было самоперекрытия буфера. Исправление: ```diff - lea edi, [ebp + thread_data.buffer] + lea edi, [ebp + thread_data.fpath] mov eax, '257 ' stosd mov al, '"' stosb - lea esi, [ebp + thread_data.fpath] + lea esi, [ebp + thread_data.buffer + 4] ``` ```diff .reply_send: mov ecx, [ebp + thread_data.socketnum] - lea edx, [ebp + thread_data.buffer] + lea edx, [ebp + thread_data.fpath] ```
👍 1
igorsh added 1 commit 2026-06-06 11:45:16 +00:00
ftpd: Do not show full path
Build system / Check kernel codestyle (pull_request) Successful in 35s
Details
Build system / Build (pull_request) Successful in 14m49s
Details
4f8acf0a7e
Burer added 1 commit 2026-06-06 14:16:50 +00:00
apps/ftpd: full created directory path in MKD reply
Build system / Check kernel codestyle (pull_request) Successful in 27s
Details
Build system / Build (pull_request) Successful in 10m15s
Details
fc56464396
Doczom requested changes 2026-06-06 15:51:03 +00:00
Dismissed
programs/network/ftpd/commands.inc Outdated
@@ -1313,0 +1350,4 @@
test [ebp + thread_data.permissions], PERMISSION_WRITE
jz permission_denied
mov eax, 4 ; Unknown filesystem error
Doczom commented 2026-06-06 15:37:19 +00:00
Owner
Copy Link
Copy Source

костыль
UPD: при обнаружении ошибочного запроса, который не может быть корректно обработан сервером, клиенту должно отправляться сообщение об ошибке(что реализовано), но при этом, так как не возникла ошибка файловой системы в консоль не должно выводится ошибочное сообщение о проблемах с фс

костыль UPD: при обнаружении ошибочного запроса, который не может быть корректно обработан сервером, клиенту должно отправляться сообщение об ошибке(что реализовано), но при этом, так как не возникла ошибка файловой системы в консоль не должно выводится ошибочное сообщение о проблемах с фс
👀 1
Burer commented 2026-06-07 16:06:48 +00:00
Owner
Copy Link
Copy Source

Should be fixed, check, please.

Should be fixed, check, please.
Burer marked this conversation as resolved
Burer added 1 commit 2026-06-07 14:25:50 +00:00
apps/ftpd: don't report FS error for invalid MKD request
Build system / Check kernel codestyle (pull_request) Successful in 23s
Details
Build system / Build (pull_request) Failing after 32s
Details
14df7a1ccf
Burer added 1 commit 2026-06-07 15:44:23 +00:00
Remove accidentally committed kterm submodule
Build system / Check kernel codestyle (pull_request) Successful in 37s
Details
Build system / Build (pull_request) Successful in 17m18s
Details
7ad6cc3e28
Doczom approved these changes 2026-06-07 17:00:49 +00:00
Burer approved these changes 2026-06-07 17:09:54 +00:00
Burer merged commit ae758389f5 into main 2026-06-07 17:12:23 +00:00
Burer deleted branch ftpd-add-mkd-command 2026-06-07 17:12:23 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
mxlgv
dunkaist
Doczom
Burer
Labels
Clear labels
C
Category/Applications
Category/Drivers
Category/General
Category/Kernel
Category/Libraries
Eolite
FASM
FS
GSoC
This issue in GSoC program
 HardwareTested
HLL
Influence/Settings
Influence/Text/TYPO
IRCC
Kernel
Kind
Breaking
Breaking change that won't be backward compatible
Kind
Bug
Something is not working
Kind
Build
Kind
Documentation
Documentation changes
Kind
Enhancement
Improve existing functionality
Kind
Feature
New functionality
Kind
Security
This is security issue
Kind
Testing
Issue or pull request related to testing
Pay for the code
Paid task
PR
Conflicts
PR conflicts with main
PR
Dependent
This PR is dependent on another PR
Priority
Critical
The priority is critical
Priority
High
The priority is high
Priority
Low
The priority is low
Priority
Medium
The priority is medium
PR
Ready to merge
Pull request is ready for merge
PR
Request changes
Changes requested in pull request
PR
Review required
Reviewed
Confirmed
Issue has been confirmed
Reviewed
Duplicate
This issue or pull request already exists
Reviewed
Invalid
Invalid issue
Reviewed
Won't Fix
This issue won't be fixed
Status
Abandoned
Somebody has started to work on this but abandoned work
Status
Blocked
Something is blocking this issue or pull request
Status
Need More Info
Feedback is required to reproduce issue or to continue work
No Label Category/Applications FASM
Kind
Feature
Priority
Low
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
ace-dent (Andrew) Burer Doczom (Mikhail Frolov) dunkaist (Ivan B) Egor00f (Егор) hidnplayr IgorA Leency (Kiril Lipatov) mxlgv (Max Logaev) ramenu rgimad sdongles (Serhii Sakhno) Sweetbread (Gleb Zaharov)
No Assignees
4 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: KolibriOS/kolibrios#342
Delete Branch "igorsh/kolibrios:ftpd-add-mkd-command"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?