diff --git a/fs/fd.c b/fs/fd.c index 6bc39f5918..d3f0952bb4 100644 --- a/fs/fd.c +++ b/fs/fd.c @@ -143,7 +143,10 @@ static int fdtable_expand(struct fdtable *table, fd_t max) { } struct fd *fdtable_get(struct fdtable *table, fd_t f) { - if (f < 0 || (unsigned) f >= current->files->size) + // Bounds-check the table being indexed, not the calling task's. They are + // not always the same one: /proc//fd reads another task's table, and + // fdtable_release walks a table the caller has already let go of. + if (f < 0 || (unsigned) f >= table->size) return NULL; return table->files[f]; } diff --git a/fs/proc/pid.c b/fs/proc/pid.c index bfd6226f1c..9ae666ba9d 100644 --- a/fs/proc/pid.c +++ b/fs/proc/pid.c @@ -269,7 +269,15 @@ static struct proc_dir_entry proc_pid_fd; static bool proc_pid_fd_readdir(struct proc_entry *entry, unsigned long *index, struct proc_entry *next_entry) { struct task *task = proc_get_task(entry); if (task == NULL) - return _ESRCH; + // This returns "is there another entry", not an error code. _ESRCH is + // nonzero, so it reads as true and proc_readdir goes on to use a + // next_entry that was never filled in. + return false; + if (task->files == NULL) { + // Exited, not yet reaped: do_exit dropped the fd table already. + proc_put_task(task); + return false; + } lock(&task->files->lock); while (*index < task->files->size && task->files->files[*index] == NULL) (*index)++; @@ -289,9 +297,15 @@ static int proc_pid_fd_readlink(struct proc_entry *entry, char *buf) { struct task *task = proc_get_task(entry); if (task == NULL) return _ESRCH; + if (task->files == NULL) { + proc_put_task(task); + return _ESRCH; + } lock(&task->files->lock); struct fd *fd = fdtable_get(task->files, entry->fd); - int err = generic_getpath(fd, buf); + int err = _ENOENT; + if (fd != NULL) + err = generic_getpath(fd, buf); unlock(&task->files->lock); proc_put_task(task); return err; @@ -302,7 +316,9 @@ static int proc_pid_exe_readlink(struct proc_entry *entry, char *buf) { if (task == NULL) return _ESRCH; lock(&task->general_lock); - int err = generic_getpath(task->mm->exefile, buf); + int err = _ESRCH; + if (task->mm != NULL) + err = generic_getpath(task->mm->exefile, buf); unlock(&task->general_lock); proc_put_task(task); return err; @@ -329,6 +345,10 @@ static int proc_pid_cwd_readlink(struct proc_entry *entry, char *buf) { struct task *task = proc_get_task(entry); if (task == NULL) return _ESRCH; + if (task->fs == NULL) { + proc_put_task(task); + return _ESRCH; + } lock(&task->fs->lock); int err = generic_getpath(task->fs->pwd, buf); unlock(&task->fs->lock); diff --git a/kernel/exit.c b/kernel/exit.c index 2a6de25af5..5eed5ae1ab 100644 --- a/kernel/exit.c +++ b/kernel/exit.c @@ -50,12 +50,21 @@ noreturn void do_exit(int status) { } // release all our resources - mm_release(current->mm); + // These are read by other threads through /proc/, which holds + // pids_lock for the whole access, so detach them under pids_lock -- a + // reader then sees either a pointer that is still ours to hand out, or + // NULL. The actual releasing has to happen outside the lock. + lock(&pids_lock); + struct mm *mm = current->mm; + struct fdtable *files = current->files; + struct fs_info *fs = current->fs; current->mm = NULL; - fdtable_release(current->files); current->files = NULL; - fs_info_release(current->fs); current->fs = NULL; + unlock(&pids_lock); + mm_release(mm); + fdtable_release(files); + fs_info_release(fs); // sighand must be released below so it can be protected by pids_lock // since it can be accessed by other threads