Kernel - Fix user handles not being cleared by Threads_Delete
authorJohn Hodge <[email protected]>
Sun, 8 Jun 2014 05:55:32 +0000 (13:55 +0800)
committerJohn Hodge <[email protected]>
Sun, 8 Jun 2014 05:55:32 +0000 (13:55 +0800)
- It would have destroyed the caller's handle list instead

12 files changed:
KernelLand/Kernel/arch/x86/mm_virt.c
KernelLand/Kernel/binary.c
KernelLand/Kernel/include/acess.h
KernelLand/Kernel/include/logdebug.h
KernelLand/Kernel/include/threads.h
KernelLand/Kernel/include/vfs.h
KernelLand/Kernel/include/vfs_threads.h
KernelLand/Kernel/system.c
KernelLand/Kernel/threads.c
KernelLand/Kernel/vfs/handle.c
KernelLand/Kernel/vfs/main.c
KernelLand/Kernel/vfs/open.c

index aec6c60..8087518 100644 (file)
@@ -74,6 +74,7 @@ void  MM_InstallVirtual(void);
 void   MM_PageFault(tVAddr Addr, Uint ErrorCode, tRegs *Regs);
 void   MM_DumpTables_Print(tVAddr Start, Uint32 Orig, size_t Size, void *Node);
 //void MM_DumpTables(tVAddr Start, tVAddr End);
+tPAddr MM_GetPageFromAS(tProcess *Process, volatile const void *Addr);
 //void MM_ClearUser(void);
 tPAddr MM_DuplicatePage(tVAddr VAddr);
 
@@ -538,6 +539,23 @@ tPAddr MM_GetPhysAddr(volatile const void *Addr)
        return (gaPageTable[addr >> 12] & ~0xFFF) | (addr & 0xFFF);
 }
 
+/**
+ * \brief Get the address of a page from another addres space
+ * \return Refenced physical address (or 0 on error)
+ */
+tPAddr MM_GetPageFromAS(tProcess *Process, volatile const void *Addr)
+{
+       tPAddr  ret = 0;
+       GET_TEMP_MAPPING(Process->MemState.CR3);
+       tVAddr  addr = (tVAddr)Addr;
+       if( (gaTmpDir[addr >> 22] & 1) && (gaTmpTable[addr >> 12] & 1) ) {
+               ret = (gaTmpTable[addr >> 12] & ~0xFFF) | (addr & 0xFFF);
+               MM_RefPhys( ret );
+       }
+       REL_TEMP_MAPPING();
+       return ret;
+}
+
 /**
  * \fn void MM_SetCR3(Uint CR3)
  * \brief Sets the current process space
@@ -552,18 +570,17 @@ void MM_SetCR3(Uint CR3)
  */
 void MM_ClearUser(void)
 {
-       Uint    i, j;
-       
-       for( i = 0; i < (MM_USER_MAX>>22); i ++ )
+       ASSERTC(MM_PPD_MIN, ==, MM_USER_MAX);
+       for( unsigned int i = 0; i < (MM_USER_MAX>>22); i ++ )
        {
                // Check if directory is not allocated
                if( !(gaPageDir[i] & PF_PRESENT) ) {
                        gaPageDir[i] = 0;
                        continue;
                }
-               
+       
                // Deallocate tables
-               for( j = 0; j < 1024; j ++ )
+               for( unsigned int j = 0; j < 1024; j ++ )
                {
                        if( gaPageTable[i*1024+j] & 1 )
                                MM_DerefPhys( gaPageTable[i*1024+j] & ~0xFFF );
@@ -583,8 +600,6 @@ void MM_ClearUser(void)
  */
 void MM_ClearSpace(Uint32 CR3)
 {
-        int    i, j;
-       
        if(CR3 == (*gpPageCR3 & ~0xFFF)) {
                Log_Error("MMVirt", "Can't clear current address space");
                return ;
@@ -601,7 +616,7 @@ void MM_ClearSpace(Uint32 CR3)
        GET_TEMP_MAPPING(CR3);
        INVLPG( gaTmpDir );
 
-       for( i = 0; i < 1024; i ++ )
+       for( int i = 0; i < 1024; i ++ )
        {
                Uint32  *table = &gaTmpTable[i*1024];
                if( !(gaTmpDir[i] & PF_PRESENT) )
@@ -611,7 +626,7 @@ void MM_ClearSpace(Uint32 CR3)
 
                if( i < 768 || (i > MM_KERNEL_STACKS >> 22 && i < MM_KERNEL_STACKS_END >> 22) )
                {
-                       for( j = 0; j < 1024; j ++ )
+                       for( int j = 0; j < 1024; j ++ )
                        {
                                if( !(table[j] & 1) )
                                        continue;
@@ -1052,6 +1067,8 @@ void *MM_MapTemp(tPAddr PAddr)
                LOG("%i: %x", i, *pte);
                // Check if page used
                if(*pte & 1)    continue;
+               MM_RefPhys( PAddr );
+               
                // Mark as used
                *pte = PAddr | 3;
                INVLPG( TEMP_MAP_ADDR + (i << 12) );
@@ -1064,6 +1081,15 @@ void *MM_MapTemp(tPAddr PAddr)
        return NULL;
 }
 
+void *MM_MapTempFromProc(tProcess *Process, const void *VAddr)
+{
+       // Get paddr
+       tPAddr  paddr = MM_GetPageFromAS(Process, VAddr);
+       if( paddr == 0 )
+               return NULL;
+       return MM_MapTemp(paddr);
+}
+
 /**
  * \fn void MM_FreeTemp(tVAddr PAddr)
  * \brief Free's a temp mapping
@@ -1073,7 +1099,9 @@ void MM_FreeTemp(void *VAddr)
         int    i = (tVAddr)VAddr >> 12;
        //ENTER("xVAddr", VAddr);
        
-       if(i >= (TEMP_MAP_ADDR >> 12)) {
+       if(i >= (TEMP_MAP_ADDR >> 12))
+       {
+               MM_DerefPhys( gaPageTable[i] & ~0xFFF );
                gaPageTable[ i ] = 0;
                Semaphore_Signal(&gTempMappingsSem, 1);
        }
index 507b45f..5170dac 100644 (file)
@@ -231,23 +231,19 @@ int Proc_int_Execve(const char *File, const char **ArgV, const char **EnvP, int
        }
 
        // --- Get argc 
-       for( argc = 0; ArgV && ArgV[argc]; argc ++ );
+       for( argc = 0; ArgV && ArgV[argc]; argc ++ )
+               ;
        
        // --- Set Process Name
        Threads_SetName(File);
        
        // --- Clear User Address space
-       // NOTE: This is a little roundabout, maybe telling ClearUser to not touch the
-       //       PPD area would be a better idea.
        if( bClearUser )
        {
-                int    nfd = *Threads_GetMaxFD();
-               void    *handles;
-               handles = VFS_SaveHandles(nfd, NULL);
-               VFS_CloseAllUserHandles();
+               // MM_ClearUser should preserve handles
                MM_ClearUser();
-               VFS_RestoreHandles(nfd, handles);
-               VFS_FreeSavedHandles(nfd, handles);
+               // - NOTE: Not a reliable test, but helps for now
+               ASSERTC( VFS_IOCtl(0, 0, NULL), !=, -1 );
        }
        
        // --- Load new binary
index de16818..83611bb 100644 (file)
@@ -27,7 +27,7 @@
 #define DEPRECATED     __attribute__((deprecated))
 //! Mark a parameter as unused
 #define UNUSED(x)      UNUSED_##x __attribute__((unused))
-//! 
+//! Apply alignment to a variable 
 #define ALIGN(x)       __attribute__((aligned(x)))
 
 /**
@@ -242,6 +242,15 @@ extern Uint        MM_GetFlags(volatile const void *VAddr);
  * \note There is only a limited ammount of slots avaliable
  */
 extern void    *MM_MapTemp(tPAddr PAddr);
+/**
+ * \brief Peform a temporary map of a page from another process
+ * \param Process      Source process
+ * \param Address      Source virtual address
+ * \return Virtual address of page in memory
+ * \note Limited slots
+ */
+struct sProcess;
+extern void    *MM_MapTempFromProc(struct sProcess *Process, const void *Address);
 /**
  * \brief Free a temporarily mapped page
  * \param Ptr  Pointer to page base
index 0710c6b..b6b7997 100644 (file)
@@ -49,6 +49,7 @@ extern void   Debug_Leave(const char *FuncName, char RetType, ...);
 extern void    Debug_HexDump(const char *Header, const void *Data, size_t Length);
 
 #define UNIMPLEMENTED()        Warning("'%s' unimplemented", __func__)
+#define TODO(fmt, ...) Panic("TODO: ", fmt ,## __VA_ARGS__)
 #if DEBUG
 # define ENTER(_types...)      Debug_Enter((char*)__func__, _types)
 # define LOG(_fmt...)  Debug_Log((char*)__func__, _fmt)
@@ -72,8 +73,8 @@ extern void   Debug_HexDump(const char *Header, const void *Data, size_t Length);
 #define assert(expr)   ASSERTV(expr, "")
 #define ASSERT(expr)   ASSERTV(expr, "")
 #define ASSERTR(expr,rv)       ASSERTRV(expr, rv, "")
-#define ASSERTC(l,rel,r)       ASSERTV(l rel r, ": 0x%x"#rel"0x%x", l, r)
-#define ASSERTCR(l,rel,r,rv)   ASSERTRV(l rel r, rv, ": 0x%x"#rel"0x%x", l, r)
+#define ASSERTC(l,rel,r)       ASSERTV ((l) rel (r), ": 0x%x"#rel"0x%x", l, r)
+#define ASSERTCR(l,rel,r,rv)   ASSERTRV((l) rel (r), rv, ": 0x%x"#rel"0x%x", l, r)
 /**
  * \}
  */
index ed90d0b..0203932 100644 (file)
@@ -21,6 +21,7 @@ enum eFaultNumbers
 #define GETMSG_IGNORE  ((void*)-1)
 
 typedef struct sThread tThread;
+typedef struct sProcess        tProcess;
 
 // === FUNCTIONS ===
 extern tThread *Proc_GetCurThread(void);
@@ -32,9 +33,9 @@ extern int    Threads_SetGID(tUID ID);
 extern tTID    Threads_WaitTID(int TID, int *Status);
 
 
-extern int     *Threads_GetMaxFD(void);
-extern char    **Threads_GetCWD(void);
-extern char    **Threads_GetChroot(void);
+extern int     *Threads_GetMaxFD(tProcess *Process);
+extern char    **Threads_GetCWD(tProcess *Process);
+extern char    **Threads_GetChroot(tProcess *Process);
 
 extern int     Proc_SendMessage(Uint Dest, int Length, void *Data);
 extern int     Proc_GetMessage(Uint *Source, Uint BufSize, void *Buffer);
index f766613..4be24c3 100644 (file)
@@ -469,7 +469,7 @@ extern tVFS_ACL     *VFS_UnixToAcessACL(Uint Mode, Uint Owner, Uint Group);
  * \param Type Type of wait
  * \param Timeout      Time to wait (NULL for infinite wait)
  * \param Name Name to show in debug output
- * \return Number of nodes that actioned (0 or 1)
+ * \return Bitset of Type flags that applied
  */
 extern int     VFS_SelectNode(tVFS_Node *Node, int Type, tTime *Timeout, const char *Name);
 
index 95ec227..a9829de 100644 (file)
@@ -10,7 +10,7 @@
 
 // === FUNCTIONS ===
 extern void    VFS_ReferenceUserHandles(void);
-extern void    VFS_CloseAllUserHandles(void);
+extern void    VFS_CloseAllUserHandles(struct sProcess *Process);
 
 extern void    *VFS_SaveHandles(int NumFDs, int *FDs);
 extern void    VFS_RestoreHandles(int NumFDs, void *Handles);
index 0c701c3..748c744 100644 (file)
@@ -58,6 +58,15 @@ void System_Init(char *CommandLine)
        // Set the debug to be echoed to the terminal
        Log_Log("Config", "Kernel now echoes to VT7 (Ctrl-Alt-F8)");
        Debug_SetKTerminal("/Devices/pts/vt7");
+       
+       // Run a thread to reap unowned threads
+       for( ;; )
+       {
+                int    status;
+               // TODO: Inform init when a thread dies
+               int tid = Threads_WaitTID(-1, &status);
+               Log_Debug("Thread0", "Thread %i exited with status %i", tid, status);
+       }
 }
 
 /**
index 8762a7c..80b2796 100644 (file)
@@ -198,7 +198,7 @@ void Threads_Delete(tThread *Thread)
                        proc->Next->Prev = proc->Prev;
 
                // VFS Cleanup
-               VFS_CloseAllUserHandles();
+               VFS_CloseAllUserHandles( proc );
                // Architecture cleanup
                Proc_ClearProcess( proc );
                // VFS Configuration strings
@@ -480,18 +480,26 @@ tTID Threads_WaitTID(int TID, int *Status)
                tTID    ret = -1;
                if( ev & THREAD_EVENT_DEADCHILD )
                {
+                       tThread * const us = Proc_GetCurThread();
                        // A child died, get the TID
-                       tThread *us = Proc_GetCurThread();
                        ASSERT(us->LastDeadChild);
-                       ret = us->LastDeadChild->TID;
+                       tThread *dead_thread = us->LastDeadChild;
+                       us->LastDeadChild = dead_thread->Next;
+                       if( us->LastDeadChild )
+                               Threads_PostEvent( us, THREAD_EVENT_DEADCHILD );
+                       else
+                               Threads_ClearEvent( THREAD_EVENT_DEADCHILD );
+                       Mutex_Release(&us->DeadChildLock);
+                       
+                       ret = dead_thread->TID;
                        // - Mark as dead (as opposed to undead)
-                       ASSERT(us->LastDeadChild->Status == THREAD_STAT_ZOMBIE);
-                       us->LastDeadChild->Status = THREAD_STAT_DEAD;
+                       ASSERTC(dead_thread->Status, ==, THREAD_STAT_ZOMBIE);
+                       dead_thread->Status = THREAD_STAT_DEAD;
                        // - Set return status
                        if(Status)
-                               *Status = us->LastDeadChild->RetStatus;
-                       us->LastDeadChild = NULL;
-                       Mutex_Release(&us->DeadChildLock);
+                               *Status = dead_thread->RetStatus;
+                       
+                       Threads_Delete( dead_thread );
                }
                else
                {
@@ -722,9 +730,11 @@ void Threads_Kill(tThread *Thread, int Status)
        SHORTREL( &glThreadListLock );
        // TODO: It's possible that we could be timer-preempted here, should disable that... somehow
        Mutex_Acquire( &Thread->Parent->DeadChildLock );        // released by parent
+       Thread->Next = Thread->Parent->LastDeadChild;
        Thread->Parent->LastDeadChild = Thread;
        Threads_PostEvent( Thread->Parent, THREAD_EVENT_DEADCHILD );
        
+       // Process cleanup happens on reaping
        Log("Thread %i went *hurk* (%i)", Thread->TID, Status);
        
        // And, reschedule
@@ -1246,17 +1256,20 @@ int *Threads_GetErrno(void)
 }
 
 // --- Configuration ---
-int *Threads_GetMaxFD(void)
+int *Threads_GetMaxFD(tProcess *Process)
 {
-       return &Proc_GetCurThread()->Process->MaxFD;
+       if(!Process)    Process = Proc_GetCurThread()->Process;
+       return &Process->MaxFD;
 }
-char **Threads_GetChroot(void)
+char **Threads_GetChroot(tProcess *Process)
 {
-       return &Proc_GetCurThread()->Process->RootDir;
+       if(!Process)    Process = Proc_GetCurThread()->Process;
+       return &Process->RootDir;
 }
-char **Threads_GetCWD(void)
+char **Threads_GetCWD(tProcess *Process)
 {
-       return &Proc_GetCurThread()->Process->CurrentWorkingDir;
+       if(!Process)    Process = Proc_GetCurThread()->Process;
+       return &Process->CurrentWorkingDir;
 }
 // ---
 
index 661ced8..d500aa9 100644 (file)
@@ -39,7 +39,7 @@ tVFS_Handle *VFS_GetHandle(int FD)
                if(FD >= MAX_KERNEL_FILES)      return NULL;
                h = &gaKernelHandles[ FD ];
        } else {
-               if(FD >= *Threads_GetMaxFD())   return NULL;
+               if(FD >= *Threads_GetMaxFD(NULL))       return NULL;
                h = &gaUserHandles[ FD ];
        }
        
@@ -59,7 +59,7 @@ int VFS_SetHandle(int FD, tVFS_Node *Node, int Mode)
                h = &gaKernelHandles[FD];
        }
        else {
-               if( FD >= *Threads_GetMaxFD())  return -1;
+               if( FD >= *Threads_GetMaxFD(NULL))      return -1;
                h = &gaUserHandles[FD];
        }
        h->Node = Node;
@@ -72,7 +72,7 @@ int VFS_AllocHandle(int bIsUser, tVFS_Node *Node, int Mode)
        // Check for a user open
        if(bIsUser)
        {
-                int    max_handles = *Threads_GetMaxFD();
+                int    max_handles = *Threads_GetMaxFD(NULL);
                // Allocate Buffer
                if( MM_GetPhysAddr( gaUserHandles ) == 0 )
                {
@@ -132,14 +132,13 @@ int VFS_AllocHandle(int bIsUser, tVFS_Node *Node, int Mode)
 
 void VFS_ReferenceUserHandles(void)
 {
-        int    i;
-        int    max_handles = *Threads_GetMaxFD();
+       const int       max_handles = *Threads_GetMaxFD(NULL);
 
        // Check if this process has any handles
        if( MM_GetPhysAddr( gaUserHandles ) == 0 )
                return ;
        
-       for( i = 0; i < max_handles; i ++ )
+       for( int i = 0; i < max_handles; i ++ )
        {
                tVFS_Handle     *h;
                h = &gaUserHandles[i];
@@ -150,22 +149,34 @@ void VFS_ReferenceUserHandles(void)
        }
 }
 
-void VFS_CloseAllUserHandles(void)
+void VFS_CloseAllUserHandles(struct sProcess *Process)
 {
-        int    max_handles = *Threads_GetMaxFD();
+       const int       max_handles = *Threads_GetMaxFD(Process);
+       ENTER("pProcess", Process);
 
+       if( max_handles >= PAGE_SIZE / sizeof(tVFS_Handle) )
+               TODO("More than a page of handles");
+
+       tVFS_Handle     *handles = MM_MapTempFromProc(Process, gaUserHandles);
+       LOG("handles=%p", handles);
        // Check if this process has any handles
-       if( MM_GetPhysAddr( gaUserHandles ) == 0 )
+       if( !handles ) {
+               LEAVE('-');
                return ;
+       }
        
        for( int i = 0; i < max_handles; i ++ )
        {
-               tVFS_Handle     *h;
-               h = &gaUserHandles[i];
+               tVFS_Handle     *h = &handles[i];
+               LOG("handles[%i].Node = %p", i, h->Node);
                if( !h->Node )
                        continue ;
                _CloseNode(h->Node);
+               h->Node = NULL;
        }
+       
+       MM_FreeTemp(handles);
+       LEAVE('-');
 }
 
 /**
@@ -174,7 +185,7 @@ void VFS_CloseAllUserHandles(void)
 void *VFS_SaveHandles(int NumFDs, int *FDs)
 {
        tVFS_Handle     *ret;
-        int    max_handles = *Threads_GetMaxFD();
+       const int       max_handles = *Threads_GetMaxFD(NULL);
        
        // Check if this process has any handles
        if( MM_GetPhysAddr( gaUserHandles ) == 0 )
@@ -235,11 +246,16 @@ void *VFS_SaveHandles(int NumFDs, int *FDs)
 void VFS_RestoreHandles(int NumFDs, void *Handles)
 {
        tVFS_Handle     *handles = Handles;
-        int    max_handles = *Threads_GetMaxFD();
+       const int       max_handles = *Threads_GetMaxFD(NULL);
 
        // NULL = nothing to do
        if( !Handles )
-               return ;        
+               return ;
+       
+       if( NumFDs > max_handles ) {
+               Log_Notice("VFS", "RestoreHandles: Capping from %i FDs to %i", NumFDs, max_handles);
+               NumFDs = max_handles;
+       }
 
        // Allocate user handle area (and dereference existing handles)
        for( int i = 0; i < NumFDs; i ++ )
index 0261eca..4ed7829 100644 (file)
@@ -59,9 +59,10 @@ int VFS_Init(void)
        VFS_MkDir("/Devices");
        VFS_MkDir("/Mount");
        VFS_Mount("dev", "/Devices", "devfs", "");
-               
-       Log_Debug("VFS", "Setting max files");
-       *Threads_GetMaxFD() = 32;
+       
+       // Set default max user file count
+       // - Applies to PID0, but propagated to all children
+       *Threads_GetMaxFD(NULL) = 32;
        return 0;
 }
 
index f0b5734..27e9878 100644 (file)
@@ -49,9 +49,9 @@ char *VFS_GetAbsPath(const char *Path)
        char    *tmpStr;
        int             iPos = 0;
        int             iPos2 = 0;
-       const char      *chroot = *Threads_GetChroot();
+       const char      *chroot = *Threads_GetChroot(NULL);
         int    chrootLen;
-       const char      *cwd = *Threads_GetCWD();
+       const char      *cwd = *Threads_GetCWD(NULL);
         int    cwdLen;
        
        ENTER("sPath", Path);
@@ -858,7 +858,7 @@ int VFS_ChDir(const char *Dest)
        VFS_Close(fd);
        
        {
-               char    **cwdptr = Threads_GetCWD();
+               char    **cwdptr = Threads_GetCWD(NULL);
                // Free old working directory
                if( *cwdptr )   free( *cwdptr );
                // Set new
@@ -910,7 +910,7 @@ int VFS_ChRoot(const char *New)
 
        // Update       
        {
-               char    **chroot_ptr = Threads_GetChroot();
+               char    **chroot_ptr = Threads_GetChroot(NULL);
                if( *chroot_ptr )       free( *chroot_ptr );
                *chroot_ptr = buf;
        }

UCC git Repository :: git.ucc.asn.au