From 18a6f0a17a50c25007a8f9b910af02bdf6fbcb78 Mon Sep 17 00:00:00 2001 From: John Hodge Date: Fri, 29 Oct 2010 15:14:19 +0800 Subject: [PATCH] Attempting to fix heap corruption - Added lock field to tVFS_Node - Fixed module makefile --- Kernel/heap.c | 39 ++++++++++++++++++++++++++--------- Kernel/include/acess.h | 4 +++- Kernel/include/vfs.h | 7 +++++++ Kernel/lib.c | 20 ++++++++++++++++-- Modules/Filesystems/FAT/fat.c | 35 ++++++++++++++----------------- Modules/Makefile.tpl | 4 ++-- 6 files changed, 75 insertions(+), 34 deletions(-) diff --git a/Kernel/heap.c b/Kernel/heap.c index 4c0232e0..aa081008 100644 --- a/Kernel/heap.c +++ b/Kernel/heap.c @@ -7,7 +7,8 @@ #include #define WARNINGS 1 -#define DEBUG_TRACE 0 +#define DEBUG_TRACE 1 +#define VERBOSE_DUMP 0 // === CONSTANTS === #define HEAP_INIT_SIZE 0x8000 // 32 KiB @@ -135,19 +136,20 @@ void *Heap_Merge(tHeapHead *Head) * \param Line Source line * \param Bytes Size of region to allocate */ -void *Heap_Allocate(const char *File, int Line, size_t Bytes) +void *Heap_Allocate(const char *File, int Line, size_t __Bytes) { tHeapHead *head, *newhead; tHeapFoot *foot, *newfoot; tHeapHead *best = NULL; Uint bestSize = 0; // Speed hack + size_t Bytes; // Get required size #if POW2_SIZES - Bytes = Bytes + sizeof(tHeapHead) + sizeof(tHeapFoot); - Bytes = 1UUL << LOG2(Bytes); + Bytes = __Bytes + sizeof(tHeapHead) + sizeof(tHeapFoot); + Bytes = 1UUL << LOG2(__Bytes); #else - Bytes = (Bytes + sizeof(tHeapHead) + sizeof(tHeapFoot) + MIN_SIZE-1) & ~(MIN_SIZE-1); + Bytes = (__Bytes + sizeof(tHeapHead) + sizeof(tHeapFoot) + MIN_SIZE-1) & ~(MIN_SIZE-1); #endif // Lock Heap @@ -247,6 +249,7 @@ void *Heap_Allocate(const char *File, int Line, size_t Bytes) newhead->Magic = MAGIC_FREE; foot->Head = newhead; // Update backlink in old footer best->Size = Bytes; // Update size in old header + best->ValidSize = __Bytes; best->Magic = MAGIC_USED; // Mark block as used best->File = File; best->Line = Line; @@ -318,8 +321,8 @@ void Heap_Deallocate(void *Ptr) // Mark as free head->Magic = MAGIC_FREE; - head->File = NULL; - head->Line = 0; + //head->File = NULL; + //head->Line = 0; head->ValidSize = 0; // Merge blocks Heap_Merge( head ); @@ -485,6 +488,7 @@ void Heap_Dump(void) while( (Uint)head < (Uint)gHeapEnd ) { foot = (void*)( (Uint)head + head->Size - sizeof(tHeapFoot) ); + #if VERBOSE_DUMP Log_Log("Heap", "%p (0x%llx): 0x%08lx (%i) %4C", head, MM_GetPhysAddr((Uint)head), head->Size, head->ValidSize, &head->Magic); Log_Log("Heap", "%p %4C", foot->Head, &foot->Magic); @@ -492,7 +496,7 @@ void Heap_Dump(void) Log_Log("Heap", "%sowned by %s:%i", (head->Magic==MAGIC_FREE?"was ":""), head->File, head->Line); } - Log_Log("Heap", ""); + #endif // Sanity Check Header if(head->Size == 0) { @@ -518,6 +522,10 @@ void Heap_Dump(void) break; } + #if VERBOSE_DUMP + Log_Log("Heap", ""); + #endif + // All OK? Go to next head = foot->NextHead; } @@ -525,13 +533,24 @@ void Heap_Dump(void) // Check for a bad return if( (tVAddr)head >= (tVAddr)gHeapEnd ) return ; + + #if !VERBOSE_DUMP + Log_Log("Heap", "%p (0x%llx): 0x%08lx (%i) %4C", + head, MM_GetPhysAddr((Uint)head), head->Size, head->ValidSize, &head->Magic); + Log_Log("Heap", "%p %4C", foot->Head, &foot->Magic); + if(head->File) { + Log_Log("Heap", "%sowned by %s:%i", + (head->Magic==MAGIC_FREE?"was ":""), head->File, head->Line); + } + Log_Log("Heap", ""); + #endif - badHead = head; - Log_Log("Heap", "==== Going Backwards ==== (from %p)", badHead); + badHead = head; // Work backwards foot = (void*)( (tVAddr)gHeapEnd - sizeof(tHeapFoot) ); + Log_Log("Heap", "==== Going Backwards ==== (from %p)", foot); head = foot->Head; while( (tVAddr)head >= (tVAddr)badHead ) { diff --git a/Kernel/include/acess.h b/Kernel/include/acess.h index e6afcf5e..9337567c 100644 --- a/Kernel/include/acess.h +++ b/Kernel/include/acess.h @@ -329,7 +329,9 @@ extern char *strncpy(char *__dest, const char *__src, size_t max); extern int strcmp(const char *__str1, const char *__str2); extern int strncmp(const char *Str1, const char *Str2, size_t num); extern int strucmp(const char *Str1, const char *Str2); -extern char *strdup(const char *Str); +//extern char *strdup(const char *Str); +#define strdup(Str) _strdup(_MODULE_NAME_"/"__FILE__, __LINE__, (Str)) +extern char *_strdup(const char *File, int Line, const char *Str); extern char **str_split(const char *__str, char __ch); extern int strpos(const char *Str, char Ch); extern int strpos8(const char *str, Uint32 search); diff --git a/Kernel/include/vfs.h b/Kernel/include/vfs.h index 4f1fc66e..c2f2e35b 100644 --- a/Kernel/include/vfs.h +++ b/Kernel/include/vfs.h @@ -104,6 +104,13 @@ typedef struct sVFS_Node * this if needed */ void *Data; + + /** + * \brief Node mutex + * \note Provided for the Filesystem driver's use + */ + tMutex Lock; + /** * \} */ diff --git a/Kernel/lib.c b/Kernel/lib.c index 4fa226c1..8a07ea76 100644 --- a/Kernel/lib.c +++ b/Kernel/lib.c @@ -27,7 +27,7 @@ char *strcpy(char *__str1, const char *__str2); char *strncpy(char *__str1, const char *__str2, size_t max); int strcmp(const char *str1, const char *str2); int strncmp(const char *str1, const char *str2, size_t num); -char *strdup(const char *Str); +char *_strdup(const char *File, int Line, const char *Str); char **str_split(const char *__str, char __ch); int strpos8(const char *str, Uint32 Search); int ReadUTF8(Uint8 *str, Uint32 *Val); @@ -54,7 +54,8 @@ EXPORT(strcpy); EXPORT(strncpy); EXPORT(strcmp); EXPORT(strncmp); -EXPORT(strdup); +//EXPORT(strdup); +EXPORT(_strdup); // Takes File/Line too EXPORT(str_split); EXPORT(strpos8); EXPORT(DivUp); @@ -451,6 +452,7 @@ int strncmp(const char *Str1, const char *Str2, size_t num) return *Str1-*Str2; } +#if 0 /** * \fn char *strdup(const char *Str) * \brief Duplicates a string @@ -463,6 +465,20 @@ char *strdup(const char *Str) strcpy(ret, Str); return ret; } +#endif + +/** + * \fn char *_strdup(const char *File, int Line, const char *Str) + * \brief Duplicates a string + */ +char *_strdup(const char *File, int Line, const char *Str) +{ + char *ret; + ret = Heap_Allocate(File, Line, strlen(Str)+1); + if( !ret ) return NULL; + strcpy(ret, Str); + return ret; +} /** * \brief Split a string using the passed character diff --git a/Modules/Filesystems/FAT/fat.c b/Modules/Filesystems/FAT/fat.c index 7eab0937..26b19613 100644 --- a/Modules/Filesystems/FAT/fat.c +++ b/Modules/Filesystems/FAT/fat.c @@ -40,6 +40,7 @@ typedef struct sFAT_LFNCacheEnt { int ID; + // TODO: Handle UTF16 names correctly char Data[256]; } tFAT_LFNCacheEnt; /** @@ -920,6 +921,10 @@ char *FAT_int_CreateName(fat_filetable *ft, char *LongFileName) { #endif ret = (char*) malloc(13); + if( !ret ) { + Log_Warning("FAT", "FAT_int_CreateName: malloc(13) failed"); + return NULL; + } FAT_int_ProperFilename(ret, ft->name); #if USE_LFN } @@ -1099,12 +1104,7 @@ int FAT_int_WriteDirEntry(tVFS_Node *Node, int ID, fat_filetable *Entry) } #endif -#if USE_LFN -// I should probably more tightly associate the LFN cache with the node -// somehow, maybe by adding a field to tVFS_Node before locking it -// Maybe .Cache or something like that (something that is free'd by the -// Inode_UncacheNode function) - +#if USE_LFN /** * \fn char *FAT_int_GetLFN(tVFS_Node *node) * \brief Return pointer to LFN cache entry @@ -1241,32 +1241,29 @@ char *FAT_ReadDir(tVFS_Node *Node, int ID) if(fileinfo[a].attrib == ATTR_LFN) { fat_longfilename *lfnInfo; - int len; lfnInfo = (fat_longfilename *) &fileinfo[a]; // Get cache for corresponding file + // > ID + Index gets the corresponding short node lfn = FAT_int_GetLFN( Node, ID + (lfnInfo->id & 0x3F) ); // Bit 6 indicates the start of an entry if(lfnInfo->id & 0x40) memset(lfn, 0, 256); - // Get the current length - len = strlen(lfn); + a = (lfnInfo->id & 0x3F) * 13; // Sanity Check (FAT implementations should not allow >255 character names) - if(len + 13 > 255) return VFS_SKIP; - // Rebase all bytes - for(a=len+1;a--;) lfn[a+13] = lfn[a]; + if(a > 255) return VFS_SKIP; // Append new bytes - lfn[ 0] = lfnInfo->name1[0]; lfn[ 1] = lfnInfo->name1[1]; - lfn[ 2] = lfnInfo->name1[2]; lfn[ 3] = lfnInfo->name1[3]; - lfn[ 4] = lfnInfo->name1[4]; - lfn[ 5] = lfnInfo->name2[0]; lfn[ 6] = lfnInfo->name2[1]; - lfn[ 7] = lfnInfo->name2[2]; lfn[ 8] = lfnInfo->name2[3]; - lfn[ 9] = lfnInfo->name2[4]; lfn[10] = lfnInfo->name2[5]; - lfn[11] = lfnInfo->name3[0]; lfn[12] = lfnInfo->name3[1]; + lfn[a+ 0] = lfnInfo->name1[0]; lfn[a+ 1] = lfnInfo->name1[1]; + lfn[a+ 2] = lfnInfo->name1[2]; lfn[a+ 3] = lfnInfo->name1[3]; + lfn[a+ 4] = lfnInfo->name1[4]; + lfn[a+ 5] = lfnInfo->name2[0]; lfn[a+ 6] = lfnInfo->name2[1]; + lfn[a+ 7] = lfnInfo->name2[2]; lfn[a+ 8] = lfnInfo->name2[3]; + lfn[a+ 9] = lfnInfo->name2[4]; lfn[a+10] = lfnInfo->name2[5]; + lfn[a+11] = lfnInfo->name3[0]; lfn[a+12] = lfnInfo->name3[1]; LOG("lfn = '%s'", lfn); LEAVE('p', VFS_SKIP); return VFS_SKIP; diff --git a/Modules/Makefile.tpl b/Modules/Makefile.tpl index 1a10e1c5..750f2dc2 100644 --- a/Modules/Makefile.tpl +++ b/Modules/Makefile.tpl @@ -12,7 +12,7 @@ CFGFILES += $(shell test -f Makefile.cfg && echo Makefile.cfg) -include $(CFGFILES) CPPFLAGS := -I$(ACESSDIR)/Kernel/include -I$(ACESSDIR)/Kernel/arch/$(ARCHDIR)/include -DARCH=$(ARCH) $(_CPPFLAGS) -CFLAGS = -Wall -Werror -fno-stack-protector $(CPPFLAGS) -g -O3 -fno-builtin +CFLAGS := -Wall -Werror -fno-stack-protector -g -O3 -fno-builtin ifneq ($(CATEGORY),) FULLNAME := $(CATEGORY)_$(NAME) @@ -64,7 +64,7 @@ endif %.o.$(_SUFFIX): %.c Makefile ../Makefile.tpl $(CFGFILES) @echo --- $(CC) -o $@ - @$(CC) $(CFLAGS) -o $@ -c $< + @$(CC) $(CFLAGS) $(CPPFLAGS) -o $@ -c $< @$(CC) -M $(CPPFLAGS) -MT $@ -o $*.d.$(ARCH) $< -include $(DEPFILES) -- 2.20.1