Skip to content

Commit 431be02

Browse files
committed
rework update_dcb() to avoid potential access uninitialized memory
1 parent 1c2f541 commit 431be02

File tree

2 files changed

+28
-9
lines changed

2 files changed

+28
-9
lines changed

kernel/kernel.asm

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -519,7 +519,7 @@ _first_mcb dw 0 ;-0002 Start of user memory
519519
global MARK0026H
520520
; A reference seems to indicate that this should start at offset 26h.
521521
MARK0026H equ $
522-
_DPBp dd 0 ; 0000 First drive Parameter Block
522+
_DPBp dd -1 ; 0000 First drive Parameter Block
523523
global _sfthead
524524
_sfthead dd 0 ; 0004 System File Table head
525525
global _clock

kernel/main.c

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -532,20 +532,35 @@ STATIC VOID update_dcb(struct dhdr FAR * dhp)
532532
COUNT nunits = dhp->dh_name[0];
533533
struct dpb FAR *dpb;
534534

535+
/* printf("nblkdev = %i\n", LoL->nblkdev); */
536+
537+
/* if no units, nothing to do, ensure at least 1 unit for rest of logic */
538+
if (nunits == 0) return;
539+
540+
/* allocate memory for new device control blocks, insert into chain [at end], and update our pointer to new end */
541+
dpb = (struct dpb FAR *)KernelAlloc(nunits * sizeof(struct dpb), 'E', Config.cfgDosDataUmb);
542+
543+
/* find end of dpb chain or initialize root if needed */
535544
if (LoL->nblkdev == 0)
536-
dpb = LoL->DPBp;
545+
{
546+
/* update root pointer to new end (our just allocated block) */
547+
LoL->DPBp = dpb;
548+
}
537549
else
538550
{
539-
for (dpb = LoL->DPBp; (ULONG) dpb->dpb_next != 0xffffffffl;
540-
dpb = dpb->dpb_next)
551+
struct dpb FAR *tmp_dpb;
552+
/* find current end of dpb chain by following next pointers to end */
553+
for (tmp_dpb = LoL->DPBp; (ULONG) tmp_dpb->dpb_next != 0xffffffffl; tmp_dpb = dpb->dpb_next)
541554
;
542-
dpb = dpb->dpb_next =
543-
KernelAlloc(nunits * sizeof(struct dpb), 'E', Config.cfgDosDataUmb);
555+
/* insert into chain [at end] */
556+
tmp_dpb->dpb_next = dpb;
544557
}
558+
/* dpb points to last block, one just allocated */
545559

546560
for (Index = 0; Index < nunits; Index++)
547-
{
548-
dpb->dpb_next = dpb + 1;
561+
{
562+
/* printf("processing unit %i of %i nunits\n", Index, nunits); */
563+
dpb->dpb_next = dpb + 1; /* memory allocated as array, so next is just next element */
549564
dpb->dpb_unit = LoL->nblkdev;
550565
dpb->dpb_subunit = Index;
551566
dpb->dpb_device = dhp;
@@ -555,10 +570,14 @@ STATIC VOID update_dcb(struct dhdr FAR * dhp)
555570
LoL->CDSp[LoL->nblkdev].cdsDpb = dpb;
556571
LoL->CDSp[LoL->nblkdev].cdsFlags = CDSPHYSDRV;
557572
}
558-
++dpb;
573+
574+
++dpb; /* dbp = dbp->dpb_next; */
559575
++LoL->nblkdev;
560576
}
577+
/* note that always at least 1 valid dpb due to above early exit if nunits==0 */
561578
(dpb - 1)->dpb_next = (void FAR *)0xFFFFFFFFl;
579+
580+
/* printf("processed %i nunits\n", nunits); */
562581
}
563582

564583
/* If cmdLine is NULL, this is an internal driver */

0 commit comments

Comments
 (0)