After int 24h call the 386-extended stack frame should be restored

[ecm-pushbx]

This is a followup to #210 (comment)

I think after the int 24h maybe we should also use Protect386Registers? Depending on the selected choice the syscall may go on, so we do want the stack to hold this frame again, right?

kernel/kernel/entry.asm

Line 655 in 032523a
int 24h ; DOS Critical error handler

I tested the patched kernel at https://nextcloud.iww.rwth-aachen.de/index.php/s/3Y8CNKiAKzTysks?dir=/2025-08-13 using the following test program.

What this does:

1. Find the NUL header
2. Insert a new device driver that lives in our code segment
3. Open the device
4. Set up known values in 4 high halves of GPRs and in fs and gs
5. Try to read from the device
6. The device will return an error on the first read (device function 04h) attempt
7. The interrupt 24h handler is hardcoded to return code 01h (repeat)
8. In case the repeat gets treated as fail, which can apparently happen due to unknown conditions, the application tries the read again
9. On subsequent reads (device function 04h) the device does return some data
10. The application displays the returned data
11. The application checks that all the registers with known values weren't corrupted
12. The device chain is restored to the original state

; Public Domain

%include "lmacros3.mac"

	cpu 386
	org 256
start:
	mov dx, i24
	mov ax, 2524h
	int 21h

	mov ah, 52h
	int 21h
	cmp dword [es:bx + 22h + 10], "NUL "
	jne error

	push dword [es:bx + 22h]
	pop dword [deviceheader.link]
	mov word [list], bx
	mov word [list + 2], es
	push cs
	push word deviceheader
	pop dword [es:bx + 22h]

	mov dx, i23
	mov ax, 2523h
	int 21h

	mov ax, 3D00h
	mov dx, name
	int 21h
	jc error_undo

	push word 0AAAAh
	push ax
	pop eax
	push word 0BBBBh
	push bx
	pop ebx
	push word 0CCCCh
	push cx
	pop ecx
	push word 0DDDDh
	push dx
	pop edx
	mov dx, 0FFFFh
	mov fs, dx
	mov dx, 5555h
	mov gs, dx

	xchg bx, ax
	mov dx, buffer
	mov cx, data.size - 3
	mov ah, 3Fh
	int 21h
	jnc @F

	mov ah, 09h
	mov dx, msg.failedretrying
	int 21h

	mov dx, buffer
	mov cx, data.size - 3
	mov ah, 3Fh
	int 21h
	jc error_undo_close

@@:
	xchg cx, ax
	mov ah, 09h
	mov dx, msg.read
	int 21h

	mov ah, 3Eh
	int 21h

	mov dx, buffer
	mov bx, 1
	mov ah, 40h
	int 21h

	call undo

	push eax
	pop ax
	pop ax
	mov dx, msg.eaxh
	cmp ax, 0AAAAh
	call mismatch

	push ebx
	pop bx
	pop ax
	mov dx, msg.ebxh
	cmp ax, 0BBBBh
	call mismatch

	push ecx
	pop cx
	pop ax
	mov dx, msg.ecxh
	cmp ax, 0CCCCh
	call mismatch

	push edx
	pop dx
	pop ax
	mov dx, msg.edxh
	cmp ax, 0DDDDh
	call mismatch

	mov ax, fs
	mov dx, msg.fs
	cmp ax, 0FFFFh
	call mismatch

	mov ax, gs
	mov dx, msg.gs
	cmp ax, 5555h
	call mismatch

	mov ax, 4C00h
	int 21h

error_undo_close:
	mov ah, 3Eh
	int 21h
error_undo:
	call undo
error:
	mov ah, 09h
	mov dx, msg.error
	int 21h

	mov ax, 4CFFh
	int 21h

mismatch:
	je .ret
	mov ah, 09h
	int 21h
.ret:
	retn

undo:
	les bx, [list]
	push dword [deviceheader.link]
	pop dword [es:bx + 22h]
	retn


i24:
	mov al, 01h
	iret

i23:
	push es
	push ds
	push bx
	 push cs
	 pop ds
	call undo
	pop bx
	pop ds
	pop es
	stc
	retf


msg:
.error:		ascic "Error!",13,10
.read:		ascic "Read content: "
.eaxh:		ascic "EAXH mismatch!",13,10
.ebxh:		ascic "EBXH mismatch!",13,10
.ecxh:		ascic "ECXH mismatch!",13,10
.edxh:		ascic "EDXH mismatch!",13,10
.fs:		ascic "FS mismatch!",13,10
.gs:		ascic "GS mismatch!",13,10
.failedretrying:ascic "DOS call returned failure, retrying.",13,10

name:
	asciz "TEST$$"

	align 2, nop
buffer:
.size equ 64
	times .size db 0

	align 4, nop
list:	dd 0
deviceheader:
.link:	dd -1
.attr:	dw 8000h
.strat:	dw deviceentry
.int:	dw retf_instruction
.name:	fill 8, 32, db "TEST$$"

deviceentry:
	push ax
	push bx
	push cx
	push dx
	push ds
	push si
	push es
	push di

	xor ax, ax
	mov al, [es:bx + 2]
	add ax, ax
	mov si, ax
	mov dx, deviceerror
	cmp si, devicetable.end - devicetable
	jae .dispatch
	mov dx, [cs:devicetable + si]
.dispatch:
	call dx

	pop di
	pop es
	pop si
	pop ds
	pop dx
	pop cx
	pop bx
	pop ax
retf_instruction:
	retf

	align 2
devicetable:
	dw deviceinit
	dw devicemediacheck
	dw devicegetbpb
	dw devicereadioctl
	dw deviceread
	dw devicereadnd
	dw deviceinputstatus
	dw deviceinputflush
	dw devicewrite
	dw devicewritev
	dw deviceoutputstatus
	dw deviceoutputflush
	dw devicewriteioctl
	dw deviceopen
	dw deviceclose
	dw deviceremovable
.end:

data:
.:	db "Hello world!",13,10
	db "123"
.end:
.size equ .end - .

deviceinit:
devicemediacheck:
devicegetbpb:
devicereadioctl:
deviceinputstatus:
deviceinputflush:
devicewrite:
devicewritev:
deviceoutputstatus:
deviceoutputflush:
devicewriteioctl:
deviceopen:
deviceclose:
deviceremovable:

deviceerror:
	mov word [es:bx + 3], 8103h
	retn

deviceread:
	 push cs
	 pop ds
	rol byte [flag], 1
	jc .read
	not byte [flag]
	jmp deviceerror

.read:
	push es
	mov cx, [es:bx + 12h]
	les di, [es:bx + 0Eh]

	mov si, word [pointer]
	mov ax, data.end
	sub ax, si
	cmp cx, ax
	jb @F
	mov cx, ax
@@:
	mov ax, cx
	rep movsb
	mov word [pointer], si
	pop es
	mov word [es:bx + 12h], ax
deviceok:
	mov word [es:bx + 3], 0100h
	retn

	align 2, nop
pointer:	dw data
flag:		db 0

devicereadnd:
	mov si, word [cs:pointer]
	cmp si, data.end
	jae .busy
	cs lodsb
	mov byte [es:bx + 0Dh], al
	jmp deviceok

.busy:
	mov word [es:bx + 3], 0300h
	retn

Running the test on the kernel linked above does show that both fs and gs were corrupted, just like I predicted.

[ecm-pushbx]

Assembling the test application requires lmacros

[ecm-pushbx]

I incorrectly stated that int 24h return code 01h is ignore. It's repeat. That was actually what I intended as well.

[ecm-pushbx]

The "repeat" response from int 24h is handled as expected for my current FreeDOS kernel:

C:\>testkern
Read content: Hello world!
C:\>callver show
FreeDOS kernel, OEM ID FDh, reports DOS version: 7.10
Internal revision and version: 43 7.10
FreeDOS kernel - GIT (build 2043 OEM:0xfd) [compiled Mar 17 2025]
C:\>

That is, it causes the kernel to internally repeat the operation which then succeeds. On the patched + OpenWatcom kernel the "repeat" response seems to not be accepted, and is turned into "fail" instead.

[ecm-pushbx]

I found another problem with the patch. The lines in

kernel/kernel/entry.asm

Lines 674 to 675 in 032523a

	mov bp, sp
	mov ah, byte [bp+4+4] ; restore old AH from nFlags

read as follows:

                mov     bp, sp
                mov     ah, byte [bp+4+4]       ; restore old AH from nFlags

One of the 4s is the offset from the usual bp. The other 4 is for the additional slots used by the stack, which is here assumed to be only si and di. With your patch the OW build also pushed fs and gs however. This causes the wrong byte to be read, which happened to disallow the "Repeat" reply from int 24h. It was luck that I noticed this, and followed through with my study.

The solution is simple: Add an smacro define to stacks.inc that specifies the size of the 386-extended stack frame (0 for 286/186/8086 build, 4 for 386 OW build, else it appears 6) like what's already encoded in the mmacro RestoreSP:

kernel/hdr/stacks.inc

Lines 154 to 156 in 032523a

	%macro RestoreSP 0
	lea sp, [bp-4]
	%endmacro

Then in the line 675 add this smacro to the bp+4+4 in the instruction that reloads ah. After that, this bug should be fixed and the test program should repeat immediately from the int 24h return code 01h.

[boeckmann]

Wouldn't relocate Restore386Register to the line before

kernel/kernel/entry.asm

Line 674 in 032523a

mov bp, sp

also be a solution? I do not see why this has to be done at the end of the routine, but perhaps I am missing something.

Currently not having my main machine at hand, so cannot test single-step it...

[ecm-pushbx]

Wouldn't relocate Restore386Register to the line before

kernel/kernel/entry.asm

Line 674 in 032523a
mov bp, sp

also be a solution? I do not see why this has to be done at the end of the routine, but perhaps I am missing something.

Currently not having my main machine at hand, so cannot test single-step it...

Yes, that would work, but it would overwrite the fs/gs with the in-kernel copies before running the Abort action. This is unlikely to be a problem but I would prefer it not be done that way.