exec, dos: assorted bug fixes (#148)

RMortenB · web-flow · commit 9474bbfb687a · 2026-05-12T10:44:00.000+02:00
* dos: buffer-overrun and resource-leak fixes

- fgets: bail early on zero-length buffer, decrement once
- getvar: close file handle on the size-lookup failure path
- runhandler: UnLock dir on the LoadSeg success path
- splitname: bail early on zero-size buffer

* exec: free-list, allocator, task and memory bugfixes

- switch HANDLE_MANAGED_MEM tests from #ifdef to #if
  (the macro is defined as 0 when disabled, so #ifdef was wrong)
- memory: free-list consistency, MEMF_MANAGED tests, pool fixes
- memory_nommu: round byteSize up to MEMCHUNK_TOTAL in AllocMem
- newaddtask: pass MemEntry list head correctly
- prepareexecbase: respect "nomungwall" arg, OR-in stacksnoop
  flag instead of clobbering, fix Preparation debug print
- remtask: Disable around list removal to protect against
  interrupt-level Signal moving the task between lists
- signal: priority comparison uses strict greater-than for
  preemption; corrected debug-print arguments
diff --git a/rom/dos/fgets.c b/rom/dos/fgets.c
@@ -54,7 +54,10 @@
     ULONG len = 0;
     LONG  c;
 
-    buflen--;
+	if(!buflen)
+		return NULL;
+
+	buflen--;
 
     do
     {
diff --git a/rom/dos/getvar.c b/rom/dos/getvar.c
@@ -237,6 +237,7 @@ static LONG getvar_from(const char *name, const char *volume, STRPTR buffer, LON
         {
             D(bug("GetVar: can't find size\n"));
 
+	        Close(file);
             return -1;
         }
 
diff --git a/rom/dos/runhandler.c b/rom/dos/runhandler.c
@@ -73,6 +73,7 @@ struct MsgPort *RunHandler(struct DeviceNode *deviceNode, const char *path, stru
                     D(bug("[RunHandler] LoadSeg(\"L:%s\")\n", cp));
                     deviceNode->dn_SegList = LoadSeg(cp);
                     CurrentDir(olddir);
+					UnLock(dir);
                 }
             }
         }
diff --git a/rom/dos/splitname.c b/rom/dos/splitname.c
@@ -69,6 +69,9 @@
 {
     AROS_LIBFUNC_INIT
 
+	if(!size)
+		return -1;
+
     size --;
 
     name += oldpos;
diff --git a/rom/exec/allocate.c b/rom/exec/allocate.c
@@ -84,7 +84,7 @@
 {
     AROS_LIBFUNC_INIT
 
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
     if ((freeList->mh_Node.ln_Type == NT_MEMORY) && IsManagedMem(freeList))
     {
         struct MemHeaderExt *mhe = (struct MemHeaderExt *)freeList;
diff --git a/rom/exec/allocpooled.c b/rom/exec/allocpooled.c
@@ -75,7 +75,7 @@
     if(!memSize)
         return NULL;
 
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
     if (IsManagedMem(mhe))
     {
         ULONG poolrequirements = (ULONG)(IPTR)mhe->mhe_MemHeader.mh_First;
diff --git a/rom/exec/allocvecpooled.c b/rom/exec/allocvecpooled.c
@@ -56,7 +56,7 @@
     /* 0-sized allocation results in returning NULL (API guarantee) */
     if(!memSize)
         return NULL;
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
     if (IsManagedMem(mhe))
     {
         ULONG poolrequirements = (ULONG)(IPTR)mhe->mhe_MemHeader.mh_First;
diff --git a/rom/exec/createpool.c b/rom/exec/createpool.c
@@ -139,7 +139,7 @@
          * If the pool is in managed memory, don't bother any further setup. The
          * pool should do the rest self.
          */
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
         if (IsManagedMem(firstPuddle))
         {
             D(bug("Managed pool\n");)
diff --git a/rom/exec/deallocate.c b/rom/exec/deallocate.c
@@ -58,7 +58,7 @@
 {
     AROS_LIBFUNC_INIT
 
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
     if ((freeList->mh_Node.ln_Type == NT_MEMORY) && IsManagedMem(freeList))
     {
         struct MemHeaderExt *mhe = (struct MemHeaderExt *)freeList;
diff --git a/rom/exec/deletepool.c b/rom/exec/deletepool.c
@@ -61,7 +61,7 @@
     {
         struct TraceLocation tp = CURRENT_LOCATION("DeletePool");
 
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
         if (IsManagedMem(poolHeader))
         {
             /* Do nothing, everything is handled in FreeMemHeader */
diff --git a/rom/exec/freepooled.c b/rom/exec/freepooled.c
@@ -61,7 +61,7 @@
     if(!memSize || !memory)
         return;
 
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
     if (IsManagedMem(mhe))
     {
         if (mhe->mhe_Free)
diff --git a/rom/exec/freevecpooled.c b/rom/exec/freevecpooled.c
@@ -57,7 +57,7 @@
     if(!memory)
         return;
 
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
     if (IsManagedMem(mhe))
     {
         if (mhe->mhe_FreeVec)
diff --git a/rom/exec/memory.c b/rom/exec/memory.c
@@ -33,7 +33,7 @@ struct MemHeader *FindMem(APTR address, struct ExecBase *SysBase)
 
     while (mh->mh_Node.ln_Succ != NULL)
     {
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
         if (IsManagedMem(mh))
         {
             struct MemHeaderExt *mhe = (struct MemHeaderExt *)mh;
@@ -376,7 +376,8 @@ void mhac_PoolMemHeaderSetup(struct MemHeader * mh, struct ProtectedPool * pool)
 #define mhac_PoolMemHeaderGetCtx(a)     ((struct MemHeaderAllocatorCtx *)(a->mh_Node.ln_Name))
 #define mhac_PoolMemHeaderGetPool(a)    (mhac_PoolMemHeaderGetCtx(a)->mhac_Data1)
 
-#endif
+#endif // defined(NO_ALLOCATOR_CONTEXT)
+
 
 
 #ifdef NO_CONSISTENCY_CHECKS
@@ -540,12 +541,8 @@ APTR stdAllocReverse(struct MemHeader *mh, struct MemHeaderAllocatorCtx *mhac, I
 			struct MemChunk * pp = p1;
 
 			/* Return the last bytes. */
-			p1->mc_Next=p2;
 			mc = (struct MemChunk *)(MK_UBYTEPTR(p2)+p2->mc_Bytes-byteSize);
-
-			p1 = p1->mc_Next;
-			p1->mc_Next  = p2->mc_Next;
-			p1->mc_Bytes = p2->mc_Bytes-byteSize;
+			p2->mc_Bytes -= byteSize;
 
 			mhac_MemChunkCreated(p1, pp, mhac);
 		}
@@ -556,7 +553,7 @@ APTR stdAllocReverse(struct MemHeader *mh, struct MemHeaderAllocatorCtx *mhac, I
 		if (requirements & MEMF_CLEAR)
 			memset(mc, 0, byteSize);
 	}
-
+#if !defined(NO_ALLOCATOR_CONTEXT)
 	else
 	{
 		if (!mhac_IsIndexEmpty(mhac))
@@ -569,6 +566,7 @@ APTR stdAllocReverse(struct MemHeader *mh, struct MemHeaderAllocatorCtx *mhac, I
 			mc = stdAlloc(mh, mhac, size, requirements, tp, SysBase);
 		}
 	}
+#endif
 	return mc;
 }
 
@@ -588,7 +586,7 @@ APTR stdAlloc(struct MemHeader *mh, struct MemHeaderAllocatorCtx *mhac, IPTR siz
      * The check has to be done for the second time. Exec uses stdAlloc on memheader
      * passed upon startup. This is bad, very bad. So here a temporary hack :)
      */
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
     if ((mh->mh_Node.ln_Type == NT_MEMORY) && IsManagedMem(mh))
     {
         struct MemHeaderExt *mhe = (struct MemHeaderExt *)mh;
@@ -654,7 +652,6 @@ APTR stdAlloc(struct MemHeader *mh, struct MemHeaderAllocatorCtx *mhac, IPTR siz
         /* Something found? */
         if (mc != NULL)
         {
-            /* Remember: if MEMF_REVERSE is set p1 and p2 are now invalid. */
             p1 = mc;
             p2 = p1->mc_Next;
 
@@ -688,6 +685,7 @@ APTR stdAlloc(struct MemHeader *mh, struct MemHeaderAllocatorCtx *mhac, IPTR siz
             if (requirements & MEMF_CLEAR)
                 memset(mc, 0, byteSize);
         }
+#if !defined(NO_ALLOCATOR_CONTEXT)
         else
         {
             if (!mhac_IsIndexEmpty(mhac))
@@ -700,7 +698,8 @@ APTR stdAlloc(struct MemHeader *mh, struct MemHeaderAllocatorCtx *mhac, IPTR siz
                 mc = stdAlloc(mh, mhac, size, requirements, tp, SysBase);
             }
         }
-        return mc;
+#endif
+    		return mc;
     }
 }
 
@@ -716,7 +715,7 @@ void stdDealloc(struct MemHeader *freeList, struct MemHeaderAllocatorCtx *mhac,
     struct MemChunk *p1, *p2, *p3;
     UBYTE *p4;
 
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
     if ((freeList->mh_Node.ln_Type == NT_MEMORY) && IsManagedMem(freeList))
     {
         struct MemHeaderExt *mhe = (struct MemHeaderExt *)freeList;
@@ -774,7 +773,7 @@ void stdDealloc(struct MemHeader *freeList, struct MemHeaderAllocatorCtx *mhac,
             /* Found a block with a higher address? */
             if (p2 >= p3)
             {
-#if !defined(NO_CONSISTENCY_CHECKS)
+// #if !defined(NO_CONSISTENCY_CHECKS)
                 /*
                 If the memory to be freed overlaps with the current
                 block something must be wrong.
@@ -788,7 +787,7 @@ void stdDealloc(struct MemHeader *freeList, struct MemHeaderAllocatorCtx *mhac,
                     Alert(AN_FreeTwice);
                     return;
                 }
-#endif
+// #endif
                 /* End the loop with p2 non-zero */
                 break;
             }
@@ -889,7 +888,7 @@ APTR AllocMemHeader(IPTR size, ULONG flags, struct TraceLocation *loc, struct Ex
     {
         struct MemHeader *orig = FindMem(mh, SysBase);
 
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
 		if (IsManagedMem(orig))
         {
             struct MemHeaderExt *mhe_orig = (struct MemHeaderExt *)orig;
@@ -964,7 +963,7 @@ void FreeMemHeader(APTR addr, struct TraceLocation *loc, struct ExecBase *SysBas
 
     IPTR size = (IPTR)mhe->mhe_MemHeader.mh_Upper - (IPTR)addr;
 
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
     if (IsManagedMem(mhe))
     {
         if (mhe->mhe_DestroyPool)
@@ -1038,6 +1037,7 @@ APTR InternalAllocPooled(APTR poolHeader, IPTR memSize, ULONG flags, struct Trac
     if (pool->pool.PoolMagic != POOL_MAGIC)
     {
         PoolManagerAlert(PME_ALLOC_INV_POOL, AT_DeadEnd, memSize, NULL, NULL, poolHeader);
+        return NULL;
     }
 
     if (pool->pool.Requirements & MEMF_SEM_PROTECTED)
@@ -1207,11 +1207,13 @@ void InternalFreePooled(APTR poolHeader, APTR memory, IPTR memSize, struct Trace
         if (pool->pool.PoolMagic != POOL_MAGIC)
         {
             PoolManagerAlert(PME_FREE_INV_POOL, AT_DeadEnd, memSize, memory, poolHeaderMH, NULL);
+            return;
         }
 
         if (poolHeaderMH != poolHeader)
         {
             PoolManagerAlert(PME_FREE_MXD_POOL, 0, memSize, memory, poolHeaderMH, poolHeader);
+            return;
         }
 
         if (pool->pool.Requirements & MEMF_SEM_PROTECTED)
diff --git a/rom/exec/memory_nommu.c b/rom/exec/memory_nommu.c
@@ -25,6 +25,8 @@ APTR nommu_AllocMem(IPTR byteSize, ULONG flags, struct TraceLocation *loc, struc
     struct MemHeader *mh;
     ULONG requirements = flags & MEMF_PHYSICAL_MASK;
 
+	byteSize  = (byteSize + MEMCHUNK_TOTAL-1) & ~(MEMCHUNK_TOTAL-1);
+
     /* Protect memory list against other tasks */
     MEM_LOCK;
 
@@ -39,7 +41,7 @@ APTR nommu_AllocMem(IPTR byteSize, ULONG flags, struct TraceLocation *loc, struc
         if ((requirements & ~mh->mh_Attributes) || mh->mh_Free < byteSize)
             continue;
 
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
         if (IsManagedMem(mh))
         {
             struct MemHeaderExt *mhe = (struct MemHeaderExt *)mh;
@@ -73,15 +75,15 @@ APTR nommu_AllocAbs(APTR location, IPTR byteSize, struct ExecBase *SysBase)
     /* Loop over MemHeader structures */
     ForeachNode(&SysBase->MemList, mh)
     {
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
         if (IsManagedMem(mh))
         {
             struct MemHeaderExt *mhe = (struct MemHeaderExt *)mh;
             if (mhe->mhe_InBounds(mhe, location, endlocation))
             {
                 if (mhe->mhe_AllocAbs)
                 {
-                    APTR ret = mhe->mhe_AllocAbs(mhe, byteSize, location);
+                    ret = mhe->mhe_AllocAbs(mhe, byteSize, location);
 
                     MEM_UNLOCK;
 
@@ -214,7 +216,7 @@ void nommu_FreeMem(APTR memoryBlock, IPTR byteSize, struct TraceLocation *loc, s
 
     ForeachNode(&SysBase->MemList, mh)
     {
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
         if (IsManagedMem(mh))
         {
             struct MemHeaderExt *mhe = (struct MemHeaderExt *)mh;
@@ -280,7 +282,7 @@ IPTR nommu_AvailMem(ULONG attributes, struct ExecBase *SysBase)
             D(bug("[MM] Skipping (mh_Attributes = 0x%08X\n", mh->mh_Attributes);)
             continue;
         }
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
         if (IsManagedMem(mh))
         {
             struct MemHeaderExt *mhe = (struct MemHeaderExt *)mh;
@@ -327,7 +329,7 @@ IPTR nommu_AvailMem(ULONG attributes, struct ExecBase *SysBase)
                         /*  2. The end (+1) of the current MemChunk must be lower than the start of the next one. */
                 if (mc->mc_Next && ((UBYTE *)mc + mc->mc_Bytes >= (UBYTE *)mc->mc_Next))
                 {
-                    bug("[MM] Chunk allocator error in MemHeader 0x%p\n");
+                    bug("[MM] Chunk allocator error in MemHeader 0x%p\n", mh);
                     bug("[MM] Overlapping chunks 0x%p (%u bytes) and 0x%p (%u bytes)\n", mc, mc->mc_Bytes, mc->mc_Next, mc->mc_Next->mc_Bytes);
 
                     Alert(AN_MemoryInsane|AT_DeadEnd);
diff --git a/rom/exec/newaddtask.c b/rom/exec/newaddtask.c
@@ -126,7 +126,7 @@
     if (mlExtra)
         AddTail(&task->tc_MemEntry, &mlExtra->ml_Node);
 
-    DADDTASK("NewAddTask MemEntry head: 0x%p", GetHead(&task->tc_MemEntry.lh_Head));
+    DADDTASK("NewAddTask MemEntry head: 0x%p", GetHead(&task->tc_MemEntry));
 
     /* Set node type to NT_TASK if not set to something else. */
     if (!task->tc_Node.ln_Type)
diff --git a/rom/exec/prepareexecbase.c b/rom/exec/prepareexecbase.c
@@ -120,7 +120,7 @@ static APTR allocmem(struct MemHeader *mh, ULONG size, ULONG attributes)
 {
     APTR ret;
 
-#ifdef HANDLE_MANAGED_MEM
+#if HANDLE_MANAGED_MEM
     if (IsManagedMem(mh))
     {
         struct MemHeaderExt *mhe = (struct MemHeaderExt *)mh;
@@ -334,11 +334,16 @@ struct ExecBase *PrepareExecBase(struct MemHeader *mh, struct TagItem *msg)
 	 */
 	opts = strcasestr(args, "mungwall");
 	if (opts)
-	    PrivExecBase(SysBase)->IntFlags = EXECF_MungWall;
+	{
+		if(!strcasestr(args, "nomungwall"))
+		{
+	    	PrivExecBase(SysBase)->IntFlags = EXECF_MungWall;
+		}
+	}
 
 	opts = strcasestr(args, "stacksnoop");
 	if (opts)
-	    PrivExecBase(SysBase)->IntFlags = EXECF_StackSnoop;
+	    PrivExecBase(SysBase)->IntFlags |= EXECF_StackSnoop;
 
 	/*
 	 * Parse system runtime debug flags.
@@ -372,7 +377,7 @@ struct ExecBase *PrepareExecBase(struct MemHeader *mh, struct TagItem *msg)
     PrivExecBase(SysBase)->ExecLockBase = ExecLock__PrepareBase(mh);
 #endif
 
-    D(bug("[Exec] %s: Preperation complete.\n"));
+    D(bug("[Exec] %s: Preparation complete.\n", __func__));
 
     return SysBase;
 }
diff --git a/rom/exec/remtask.c b/rom/exec/remtask.c
@@ -95,10 +95,15 @@
             /*
              * Remove() here, before freeing the MemEntry list. Because
              * the MemEntry list might contain the task struct itself!
+             *
+             * Use Disable() to protect against interrupt-level Signal()
+             * which can move this task between lists concurrently.
             */
 #if !defined(EXEC_REMTASK_NEEDSSWITCH)
+            Disable();
             task->tc_State = TS_REMOVED;
             Remove(&task->tc_Node);
+            Enable();
 #else
             krnSysCallReschedTask(task, TS_REMOVED);
 #endif
diff --git a/rom/exec/signal.c b/rom/exec/signal.c

Original file line number	Diff line number	Diff line change
`@@ -237,6 +237,7 @@ static LONG getvar_from(const char name, const char volume, STRPTR buffer, LON`
`237`	`237`	`{`
`238`	`238`	`D(bug("GetVar: can't find size\n"));`
`239`	`239`
	`240`	`+ Close(file);`
`240`	`241`	`return -1;`
`241`	`242`	`}`
`242`	`243`
Original file line number	Diff line number	Diff line change
`@@ -73,6 +73,7 @@ struct MsgPort RunHandler(struct DeviceNode deviceNode, const char *path, stru`
`73`	`73`	`D(bug("[RunHandler] LoadSeg(\"L:%s\")\n", cp));`
`74`	`74`	`deviceNode->dn_SegList = LoadSeg(cp);`
`75`	`75`	`CurrentDir(olddir);`
	`76`	`+ UnLock(dir);`
`76`	`77`	`}`
`77`	`78`	`}`
`78`	`79`	`}`
Original file line number	Diff line number	Diff line change
`@@ -69,6 +69,9 @@`
`69`	`69`	`{`
`70`	`70`	`AROS_LIBFUNC_INIT`
`71`	`71`
	`72`	`+ if(!size)`
	`73`	`+ return -1;`
	`74`	`+`
`72`	`75`	`size --;`
`73`	`76`
`74`	`77`	`name += oldpos;`
Original file line number	Diff line number	Diff line change
`@@ -84,7 +84,7 @@`
`84`	`84`	`{`
`85`	`85`	`AROS_LIBFUNC_INIT`
`86`	`86`
`87`		`-#ifdef HANDLE_MANAGED_MEM`
	`87`	`+#if HANDLE_MANAGED_MEM`
`88`	`88`	`if ((freeList->mh_Node.ln_Type == NT_MEMORY) && IsManagedMem(freeList))`
`89`	`89`	`{`
`90`	`90`	`struct MemHeaderExt mhe = (struct MemHeaderExt )freeList;`
Original file line number	Diff line number	Diff line change
`@@ -75,7 +75,7 @@`
`75`	`75`	`if(!memSize)`
`76`	`76`	`return NULL;`
`77`	`77`
`78`		`-#ifdef HANDLE_MANAGED_MEM`
	`78`	`+#if HANDLE_MANAGED_MEM`
`79`	`79`	`if (IsManagedMem(mhe))`
`80`	`80`	`{`
`81`	`81`	`ULONG poolrequirements = (ULONG)(IPTR)mhe->mhe_MemHeader.mh_First;`
Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,7 @@`
`56`	`56`	`/* 0-sized allocation results in returning NULL (API guarantee) */`
`57`	`57`	`if(!memSize)`
`58`	`58`	`return NULL;`
`59`		`-#ifdef HANDLE_MANAGED_MEM`
	`59`	`+#if HANDLE_MANAGED_MEM`
`60`	`60`	`if (IsManagedMem(mhe))`
`61`	`61`	`{`
`62`	`62`	`ULONG poolrequirements = (ULONG)(IPTR)mhe->mhe_MemHeader.mh_First;`
Original file line number	Diff line number	Diff line change
`@@ -139,7 +139,7 @@`
`139`	`139`	`* If the pool is in managed memory, don't bother any further setup. The`
`140`	`140`	`* pool should do the rest self.`
`141`	`141`	`*/`
`142`		`-#ifdef HANDLE_MANAGED_MEM`
	`142`	`+#if HANDLE_MANAGED_MEM`
`143`	`143`	`if (IsManagedMem(firstPuddle))`
`144`	`144`	`{`
`145`	`145`	`D(bug("Managed pool\n");)`
Original file line number	Diff line number	Diff line change
`@@ -58,7 +58,7 @@`
`58`	`58`	`{`
`59`	`59`	`AROS_LIBFUNC_INIT`
`60`	`60`
`61`		`-#ifdef HANDLE_MANAGED_MEM`
	`61`	`+#if HANDLE_MANAGED_MEM`
`62`	`62`	`if ((freeList->mh_Node.ln_Type == NT_MEMORY) && IsManagedMem(freeList))`
`63`	`63`	`{`
`64`	`64`	`struct MemHeaderExt mhe = (struct MemHeaderExt )freeList;`
Original file line number	Diff line number	Diff line change
`@@ -61,7 +61,7 @@`
`61`	`61`	`{`
`62`	`62`	`struct TraceLocation tp = CURRENT_LOCATION("DeletePool");`
`63`	`63`
`64`		`-#ifdef HANDLE_MANAGED_MEM`
	`64`	`+#if HANDLE_MANAGED_MEM`
`65`	`65`	`if (IsManagedMem(poolHeader))`
`66`	`66`	`{`
`67`	`67`	`/* Do nothing, everything is handled in FreeMemHeader */`