Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

config.c KernelAlloc doesn't check that (UMB) memory is sufficient #203

Open
ecm-pushbx opened this issue Mar 27, 2025 · 5 comments
Open
Labels
bug Something isn't working

Comments

@ecm-pushbx
Copy link
Contributor

I traced a crash that I encountered to the call in

sp = sp->sftt_next = (sfttbl FAR *)
which calls KernelAlloc in
void FAR * KernelAlloc(size_t nBytes, char type, int mode)
which at this point transfers to KernelAllocPara in
void FAR * KernelAllocPara(size_t nPara, char type, char *name, int mode)

The problem is that the latter function never checks that sufficient memory is available in the referenced memory block. In my test case this causes the kernel to overwrite memory not belonging to it, corrupting my little test's int 2Fh handler and likely part of the resident debugger (bootable lDebug) as well.

I'm running in "dosemu2 2.0pre9-dev-20240420-1892-g6a2f4f527 Configured: 2024-04-18 10:57:53 +0000" and booting into a recent lDebug revision. The kernel I use is from the bin/ subdirectory of my most recent build, renamed from kernel.sys to fdkernel.sys, and configured using sys config fdkernel.sys checkdebugger=1. I'm also using testpl.com as a device driver, a build of which I uploaded to our server here. (If you really want I can dig up the exact commands to build it from the lDOS boot repo.)

The full config file used contains:

rem config.sys for DOSEMU + FreeDOS
SWITCHES=/F
DOS=UMB,HIGH
dosdata=umb
lastdrive=Z
files=40
stacks=0
buffers=10
device=c:\testpl.com rc now
device=d:\dosemu\umb.sys
device=d:\dosemu\emufs.sys
device=d:\dosemu\ems.sys
device=d:\dosemu\cdrom.sys
install=d:\dosemu\emufs.com
shell=C:\command.com /e:4096 /p:d:\autoexec.bat
;set ldebugconfig=c:\testdir
;device=c:\bin\ldebug.com /c=d68:0;qcd
;device=c:\bin\ldebug.com /c=qdc

The fred327 alias used in the debugger session is defined as follows:

alias add fred327 boot protocol freedos cmdline=1 ldp/fdkernel.sys . config fdconfig.327

(This adds an override to run a different config file than default.)

The testumb1.sld Script for lDebug file used contains:


r word [0:413] -= 2
r v0 = word [0:413]
r v1 = v0 + 1
r v0 *= #1024 / #16
r v1 *= #1024 / #16

f v0:0 l #1024 26
f v1:0 l #1024 CC
f v1:0 l 10 0
a v1:10
 cmp ax, 4300
 je 20
 cmp ax, 4310
 je 30
 jmp 0:0
 .
r v2 = aao - 4
a v1:20
 mov al, 80
 iret
 .
a v1:30
 push cs
 pop es
 mov bx, 40
 iret
 .
a v1:40
 jmp short 45
 nop
 nop
 nop
 cmp ah, 10
 je 50
 .
r v3 = aao
a
 xor ax, ax		; error
 mov bl, 80		; not implemented
 retf
 .
a v1:50
 cmp byte [cs:10], EB
 je 80
 cmp dx, (#1024 / #16 - v4)
 ja 70
 jb (v3)		; if requested smaller -->
 mov ax, 1		; success
 mov bx, (v0 + v4)	; => UMB, dx = size
 mov word [cs:10], (EB + (v2 - 1 - 12) * 100)
 retf
 .
a v1:70
 xor ax, ax		; error
 mov bl, B0		; only a smaller UMB
 mov dx, (#1024 / #16 - v4)
 retf
 .
a v1:80
 xor ax, ax		; error
 mov bl, B1		; no UMB available
 xor dx, dx		; largest available
 retf
 .
rc.replace r vf = ri2Fp; r dword [v1:v2] = vf; r dword [0:2F * 4] = v1*10000 + 10

This script reserves 2 KiB at the top of the Low Memory Area (below the resident debugger and possibly EBDA), using the upper 1 KiB for code space and the lower 1 KiB as an improvised "UMB" space. The v4 variable indicates how much of a gap (in paragraphs) to leave between the top of the LMA and the UMB. I set this to 1 in the test to avoid any possible confounding factors with UMBs that extend the LMA. (This is the original purpose of the test, which showed me that lRxDOS/lMS-DOS incorrectly treat that case.)

The code is an int 2Fh handler which offers an XMS entrypoint. The only supported function is function 10h, allocate UMB, and then only the size query (eg with dx = 0FFFFh) and exact size allocation are supported. After the UMB is allocated, the int 2Fh handler is changed to chain to its downlink immediately (hiding the XMS entrypoint), and subsequent function 10h calls to the XMS entrypoint say there is no free UMB left.

The use of this script is as follows, once booted into lDebug:

r v4 1
y testumb1.sld
fred327
g
r f CY
g
rc

The Y command runs a Script for lDebug file, which is read using int 13h when booted into the debugger. The fred327 alias was listed above. The first g command runs up to the debugger check, which had to be enabled using sys config as above. r f CY indicates debugger present to the kernel.

The second g command runs up to the next breakpoint, which in this FreeDOS kernel test case is the int3 instruction in testpl.com. The rc command hooks int 2Fh to our handler then. That makes it ready for action, to offer its UMB to the config code after the testpl.com device driver returns. Running a third g command would crash dosemu2, due to the kernel corrupting int 2Fh presumably.

I will dump a longer session output as a subsequent comment.

@ecm-pushbx ecm-pushbx added the bug Something isn't working label Mar 27, 2025
@ecm-pushbx
Copy link
Contributor Author

dosemu2 2.0pre9-dev-20240420-1892-g6a2f4f527 Configured: 2024-04-18 10:57:53 +0000
Get the latest code at http://dosemu2.github.io/dosemu2
Submit Bugs via https://github.com/dosemu2/dosemu2/issues
Ask for help in mail list: linux-msdos@vger.kernel.org
This program comes with ABSOLUTELY NO WARRANTY.
This is free software, GPL v2 (or any later version) distribution conditions.

.................................................................................................................

&; Welcome to lDebug!
Press any key to enter debugger terminal!
Timer stopped
V0=000000AE V1=00000000 V2=00000000 V3=00000000  DCO=00000000 DCS=00000000
V4=00000000 V5=00000000 V6=00000000 V7=00000000  DAO=0000F007 DAS=0000F007
V8=00000000 V9=00000000 VA=00000000 VB=00000000  DIF=0180F008 DPI=0000:0000
VC=00000000 VD=00000000 VE=00000000 VF=00000000  DPR=7681     DPP=0000
Current mode: Virtual 86 Mode
-r v4 1
-y testumb1.sld
-
-r word [0:413] -= 2
-r v0 = word [0:413]
-r v1 = v0 + 1
-r v0 *= #1024 / #16
-r v1 *= #1024 / #16
-
-f v0:0 l #1024 26
-f v1:0 l #1024 CC
-f v1:0 l 10 0
-a v1:10
7640:0010  cmp ax, 4300
7640:0013  je 20
7640:0015  cmp ax, 4310
7640:0018  je 30
7640:001A  jmp 0:0
7640:001F  .
-r v2 = aao - 4
-a v1:20
7640:0020  mov al, 80
7640:0022  iret
7640:0023  .
-a v1:30
7640:0030  push cs
7640:0031  pop es
7640:0032  mov bx, 40
7640:0035  iret
7640:0036  .
-a v1:40
7640:0040  jmp short 45
7640:0042  nop
7640:0043  nop
7640:0044  nop
7640:0045  cmp ah, 10
7640:0048  je 50
7640:004A  .
-r v3 = aao
-a
7640:004A  xor ax, ax           ; error
7640:004C  mov bl, 80           ; not implemented
7640:004E  retf
7640:004F  .
-a v1:50
7640:0050  cmp byte [cs:10], EB
7640:0056  je 80
7640:0058  cmp dx, (#1024 / #16 - v4)
7640:005B  ja 70
7640:005D  jb (v3)              ; if requested smaller -->
7640:005F  mov ax, 1            ; success
7640:0062  mov bx, (v0 + v4)    ; => UMB, dx = size
7640:0065  mov word [cs:10], (EB + (v2 - 1 - 12) * 100)
7640:006C  retf
7640:006D  .
-a v1:70
7640:0070  xor ax, ax           ; error
7640:0072  mov bl, B0           ; only a smaller UMB
7640:0074  mov dx, (#1024 / #16 - v4)
7640:0077  retf
7640:0078  .
-a v1:80
7640:0080  xor ax, ax           ; error
7640:0082  mov bl, B1           ; no UMB available
7640:0084  xor dx, dx           ; largest available
7640:0086  retf
7640:0087  .
-rc.replace r vf = ri2Fp; r dword [v1:v2] = vf; r dword [0:2F * 4] = v1*10000 + 10
-fred327
-g
123Unexpected breakpoint interrupt
AX=7301 BX=0080 CX=0000 DX=0BC3 SP=2678 BP=2678 SI=0019 DI=0154
DS=7397 ES=7397 SS=7397 CS=6EF9 IP=00DF NV UP EI PL ZR NA PE NC
6EF9:00DF 7202              jb      00E3                            not jumping
-r f CY
-g

FreeDOS kernel - GIT (build 2043 OEM:0xfd) [compiled Mar 17 2025]
Kernel compatibility 7.10 - GNUC - 808x compatible - FAT32 support

(C) Copyright 1995-2023 Pasquale J. Villani and The FreeDOS Project.
All Rights Reserved. This is free software and comes with ABSOLUTELY NO
WARRANTY; you can redistribute it and/or modify it under the terms of the
GNU General Public License as published by the Free Software Foundation;
either version 2, or (at your option) any later version.
C: HD1, Pri[ 1], CHS=    0-1-1, start=     0 MB, size=  2000 MB
D: HD2, Pri[ 1], CHS=    0-1-1, start=     0 MB, size=  2000 MB
 SS=7397 BP=2534 SP=2528 CS=02C8 IP=0012 FL=3200
 DS=02C8 SI=0000 ES=7397 DI=256A
 AX=0012 BX=254C CX=1A59 DX=1A4C
 S0=018C S1=6EF9 S2=256A S3=0000 S4=7397 S5=02C8 S6=256A S7=08D0
 S8=254C S9=7397 SA=0012 SB=02C8 SC=1A55 SD=0000 SE=4554 SF=5453
Test payload loaded.
Device command line = "C:\TESTPL.COM RC NOW "
Unexpected breakpoint interrupt
AX=0D00 BX=254C CX=0015 DX=1A4C SP=250C BP=2534 SI=019E DI=0015
DS=02C8 ES=7397 SS=7397 CS=02C8 IP=0426 NV UP EI PL ZR NA PE NC
02C8:0426 2EF606F80102      test    byte [cs:01F8], 02               CS:01F8=02
-rc
&r vf = ri2Fp
&r dword [v1:v2] = vf
&r dword [0:2F * 4] = v1*10000 + 10
-bp new ptr ri21p when ah==3E && bx==0013
-g
dosemu XMS 3.0 & UMB support enabled
EMUFS host file and print access available
dosemu EMS driver rev 0.9 installed.
dosemu CDROM driver installed (V0.2)
Hit permanent breakpoint 00
AX=3E29 BX=0013 CX=0101 DX=F700 SP=25E6 BP=2672 SI=0013 DI=0000
DS=7397 ES=7397 SS=7397 CS=00D9 IP=12EE NV UP DI NG NZ AC PE CY
00D9:12EE EAA6013663        jmp     6336:01A6
-bd all
-g ptr [ss:sp]
AX=0006 BX=0013 CX=0101 DX=F700 SP=25EC BP=2672 SI=0013 DI=0000
DS=7397 ES=7397 SS=7397 CS=6EF9 IP=026C NV UP EI NG NZ AC PE CY
6EF9:026C 7303              jae     0271                            not jumping
-p
AX=0006 BX=0013 CX=0101 DX=F700 SP=25EC BP=2672 SI=0013 DI=0000
DS=7397 ES=7397 SS=7397 CS=6EF9 IP=026E NV UP EI NG NZ AC PE CY
6EF9:026E B8FFFF            mov     ax, FFFF
-
AX=FFFF BX=0013 CX=0101 DX=F700 SP=25EC BP=2672 SI=0013 DI=0000
DS=7397 ES=7397 SS=7397 CS=6EF9 IP=0271 NV UP EI NG NZ AC PE CY
6EF9:0271 C3                retn
-
AX=FFFF BX=0013 CX=0101 DX=F700 SP=25EE BP=2672 SI=0013 DI=0000
DS=7397 ES=7397 SS=7397 CS=6EF9 IP=0D29 NV UP EI NG NZ AC PE CY
6EF9:0D29 46                inc     si
-
AX=FFFF BX=0013 CX=0101 DX=F700 SP=25EE BP=2672 SI=0014 DI=0000
DS=7397 ES=7397 SS=7397 CS=6EF9 IP=0D2A NV UP EI PL NZ NA PE CY
6EF9:0D2A 83FE14            cmp     si, +14
-
AX=FFFF BX=0013 CX=0101 DX=F700 SP=25EE BP=2672 SI=0014 DI=0000
DS=7397 ES=7397 SS=7397 CS=6EF9 IP=0D2D NV UP EI PL ZR NA PE NC
6EF9:0D2D 75F6              jnz     0D25                            not jumping
-
AX=FFFF BX=0013 CX=0101 DX=F700 SP=25EE BP=2672 SI=0014 DI=0000
DS=7397 ES=7397 SS=7397 CS=6EF9 IP=0D2F NV UP EI PL ZR NA PE NC
6EF9:0D2F E8841C            call    29B6
-
Kernel: allocated 32 Diskbuffers = 17024 Bytes in HMA
Terminated with signal 4

The code traced at the end is

kernel/kernel/main.c

Lines 367 to 372 in 192cccc

/* Close all (device) files */
for (i = 0; i < 20; i++)
close(i);
/* and do final buffer allocation. */
PostConfig();
which calls PostConfig in

kernel/kernel/config.c

Lines 420 to 422 in 192cccc

/* Do third pass initialization. */
/* Also, run config.sys to load drivers. */
void PostConfig(void)

I elided the exact crash point but rest assured I did trace it to the KernelAlloc call I linked above.

The commands listed are all explained in the prior post, except for bp new ptr ri21p when ah==3E && bx==0013 and bd all and g ptr [ss:sp]. The bp command sets a conditional breakpoint on the int 21h handler address, only hitting the breakpoint for a certain combination of register values. bd disables the same breakpoint after it was hit, to avoid an error from the DOS CS relocation otherwise. This form of the g command sets a temporary breakpoint on the far interrupt return address read from the iret stack frame, then runs until it is hit.

@ecm-pushbx
Copy link
Contributor Author

By the way, the first UMCB is "protected" (my wording) by FreeDOS. Commenting out dosdata=umb allows me to run my test the intended way and it leaves the 0-paragraph first UMCB allocated. I will add a knob to lDOS to either "protect" the first UMCB or extend the LMA into the consecutive UMBs.

@ecm-pushbx
Copy link
Contributor Author

By the way, this also shows the old FreeDOS defect that subsequent UMBs (by the same or other XMS drivers) aren't allocated after the first time that umb_init runs to completion. This was pointed out by UMBPCI's documentation I believe.

This is the function to allocate UMBs:

STATIC void umb_init(void)
Observe the use of UmbState which makes it so umb_init only runs successfully once.

lDOS is already better in this regard, it scans for any available UMBs after any device driver was loaded and has three different branches to add a new UMB to the UMCB chain no matter where in the chain it is added: https://hg.pushbx.org/ecm/msdos4/file/eba7d38ff1c6/src/BIOS/init.asm#l3961

@ecm-pushbx
Copy link
Contributor Author

See also #202

@ecm-pushbx
Copy link
Contributor Author

I will add a knob to lDOS to either "protect" the first UMCB or extend the LMA into the consecutive UMBs.

Added now: https://hg.pushbx.org/ecm/msdos4/shortlog/584f3d01e25e The default is "Enlarge LMA" but this can be changed by COMPAT=PROTECTFIRSTUMCB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant