From bff21fdcad76dfd85ccd14215d64a085b69e3add Mon Sep 17 00:00:00 2001 From: Marian Ohligs Date: Fri, 30 Sep 2011 11:48:13 +0200 Subject: [PATCH 1/3] tidy up syscall.c, add comments, add missing checks, fix bug in open --- kernel/syscall.c | 245 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 183 insertions(+), 62 deletions(-) diff --git a/kernel/syscall.c b/kernel/syscall.c index f5329367..28b660ca 100644 --- a/kernel/syscall.c +++ b/kernel/syscall.c @@ -47,6 +47,11 @@ static int get_fildes(void) task_t* curr_task = per_core(current_task); int fd; + /* + * Seach the process specific file descriptor table for a free + * descriptor. The new descriptor returned by the call is the lowest + * numbered descriptor currently not in use by the process. + */ for (fd = 0; fd < NR_OPEN; fd++) { if (curr_task->fildes_table[fd] == NULL) { /* Init Filedescriptor */ @@ -60,32 +65,55 @@ static int get_fildes(void) return -EMFILE; } +static int sys_write(int fd, const char* buf, size_t len) +{ + task_t* curr_task = per_core(current_task); + + /* fd is negative or greater than the maximum allowable number */ + if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0)) + return -EBADF; + /* fd is not an active, valid file descriptor. */ + if (curr_task->fildes_table[fd] == NULL) + return -EBADF; + + return write_fs(curr_task->fildes_table[fd], (uint8_t*)buf, len); +} + static int sys_open(const char* name, int flags, int mode) { int fd, check; - fildes_t* file = NULL; + task_t* curr_task = per_core(current_task); + /* no name is given */ + if (name == NULL) + return -EINVAL; + + /* Get a free file descriptor */ fd = get_fildes(); - /* validate the fd */ - if (fd < 0) - return fd; - file = per_core(current_task)->fildes_table[fd]; - file->mode = mode; - file->flags = flags; - file->count = 1; - check = open_fs(file, (char*) name); + /* fd is negative or greater than the maximum allowable number */ + if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0)) + return fd; /* in this case fd = errno */ + + /* init the whole file descriptor structure */ + curr_task->fildes_table[fd]->mode = mode; + curr_task->fildes_table[fd]->flags = flags; + curr_task->fildes_table[fd]->count = 1; + check = open_fs(curr_task->fildes_table[fd], (char*) name); + + /* file doesn't exist! */ if (check < 0) { - /* file doesn't exist! */ - kfree(file, sizeof(fildes_t)); - file = NULL; + /* tidy up the fildescriptor */ + kfree(curr_task->fildes_table[fd], sizeof(fildes_t)); + curr_task->fildes_table[fd] = NULL; return check; } return fd; } -static int sys_stat(const char* name, struct stat* st) { +static int sys_stat(const char* name, struct stat* st) +{ vfs_node_t* node; node = findnode_fs(name); @@ -112,43 +140,102 @@ static int sys_stat(const char* name, struct stat* st) { return 0; } +static int sys_read(int fd, const char* buf, size_t len) +{ + task_t* curr_task = per_core(current_task); + + /* fd is negative or greater than the maximum allowable number */ + if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0)) + return -EBADF; + /* fd is not an active, valid file descriptor. */ + if (curr_task->fildes_table[fd] == NULL) + return -EBADF; + + return read_fs(curr_task->fildes_table[fd], (uint8_t*)buf, len); +} + + #if defined(CONFIG_LWIP) && LWIP_SOCKET static int sys_socket(int domain, int type, int protocol) { - int fd; - fildes_t* file = NULL; + int fd, sock; + task_t* curr_task = per_core(current_task); + /* Get a free file descriptor */ fd = get_fildes(); - /* validate the fd */ - if (fd < 0) - return fd; - file = per_core(current_task)->fildes_table[fd]; - file->offset = lwip_socket(domain, type, protocol); - file->node = findnode_fs("/dev/socket"); - file->count = 1; + /* validate the file descriptor */ + if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0)) + return fd; /* in this case fd = errno */ + + /* Get a valid lwip descriptor */ + sock = lwip_socket(domain, type, protocol); + + /* validate the file descriptor */ + if (BUILTIN_EXPECT(sock < 0, 0)) + return sock; /* in this case sock = errno */ + + /* + * init the whole file descriptor structure. + * We use the offset to save the lwip descriptor + * TODO: find another solution or change the name 'offset' + */ + curr_task->fildes_table[fd]->offset = sock; + curr_task->fildes_table[fd]->count = 1; + curr_task->fildes_table[fd]->node = findnode_fs("/dev/socket"); + + /* file doesn't exist! */ + if (curr_task->fildes_table[fd]->node == NULL) { + /* tidy up the fildescriptor */ + kfree(curr_task->fildes_table[fd], sizeof(fildes_t)); + curr_task->fildes_table[fd] = NULL; + return -ENOENT; + } return fd; } static int sys_accept(int s, struct sockaddr* addr, socklen_t* addrlen) { - int fd; - fildes_t* file = NULL; + int fd, sock2; + task_t* curr_task = per_core(current_task); - if (BUILTIN_EXPECT(s >= NR_OPEN, 0)) - return -EINVAL; + /* validate the 'socket'-filedescriptor */ + if (BUILTIN_EXPECT((s >= NR_OPEN) || (s < 0), 0)) + return -EBADF; + /* validate the 'socket'-file descriptor object */ + if (curr_task->fildes_table[s] == NULL) + return -EBADF; + /* Get a free file descriptor */ fd = get_fildes(); - /* validate the fd */ - if (fd < 0) - return fd; - file = per_core(current_task)->fildes_table[fd]; - file->offset = lwip_accept(s, addr, addrlen); - file->node = findnode_fs("/dev/socket"); - file->count = 1; + /* validate the file descriptor */ + if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0)) + return fd; /* in this case fd = errno */ + /* Get a valid lwip descriptor */ + sock2 = lwip_accept(s, addr, addrlen); + /* validate the file descriptor */ + if (BUILTIN_EXPECT(sock2 < 0, 0)) + return sock2; /* in this case sock = errno */ + + /* + * init the whole file descriptor structure. + * We use the offset to save the lwip descriptor + * TODO: find another solution or change the name 'offset' + */ + curr_task->fildes_table[fd]->offset = sock2; + curr_task->fildes_table[fd]->count = 1; + curr_task->fildes_table[fd]->node = findnode_fs("/dev/socket"); + /* file doesn't exist! */ + if (curr_task->fildes_table[fd]->node == NULL) { + /* tidy up the fildescriptor */ + kfree(curr_task->fildes_table[fd], sizeof(fildes_t)); + curr_task->fildes_table[fd] = NULL; + return -ENOENT; + } + return fd; } @@ -157,13 +244,26 @@ static int sys_accept(int s, struct sockaddr* addr, socklen_t* addrlen) static int sys_close(int fd) { + int check; task_t* curr_task = per_core(current_task); - - if (BUILTIN_EXPECT(fd >= NR_OPEN, 0)) - return -EINVAL; + /* fd is negative or greater than the maximum allowable number */ + if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0)) + return -EBADF; + /* fd is not an active, valid file descriptor. */ + if (curr_task->fildes_table[fd] == NULL) + return -EBADF; + + /* + * The close() call deletes a descriptor from the per-process object + * reference table. If this is the last reference to the underlying + * object, the object will be deactivated. + */ if (curr_task->fildes_table[fd]->count == 1) { - close_fs(curr_task->fildes_table[fd]); + check = close_fs(curr_task->fildes_table[fd]); + /* close command failed -> return check = errno */ + if (BUILTIN_EXPECT(check < 0, 0)) + return check; kfree(curr_task->fildes_table[fd], sizeof(fildes_t)); curr_task->fildes_table[fd] = NULL; } else { @@ -178,30 +278,42 @@ static int sys_close(int fd) static int sys_lseek(int fd, off_t pos, int origin) { int ret = -EINVAL; - fildes_t* file; + task_t* curr_task = per_core(current_task); - if (BUILTIN_EXPECT(fd >= NR_OPEN, 0)) - return -EINVAL; + /* fd is negative or greater than the maximum allowable number */ + if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0)) + return -EBADF; + /* fd is not an active, valid file descriptor. */ + if (curr_task->fildes_table[fd] == NULL) + return -EBADF; - file = per_core(current_task)->fildes_table[fd]; - - if (BUILTIN_EXPECT(!((file->node->type == FS_FILE) - || (file->node->type == FS_CHARDEVICE)), 0)) - return -EINVAL; + /* TODO: in case of Fildes is associated with a pipe, socket, or FIFO. return ESPIPE */ + /* TODO: The seek location is too large to be stored in an object of type off_t. return EOVERFLOW*/ switch(origin) - { + { case SEEK_SET: { /* set file offset to offset */ - file->offset = pos; + ret = pos; + /* The seek location is negative. */ + if (ret < 0) + return -EINVAL; + curr_task->fildes_table[fd]->offset = ret; ret = 0; break; } case SEEK_CUR: { /* set file offset to current plus offset */ - ret = pos + file->offset; + ret = pos + curr_task->fildes_table[fd]->offset; + /* The seek location is negative. */ + if (ret < 0) + return -EINVAL; break; } case SEEK_END: { /* set file offset to EOF plus offset */ - file->offset = pos + file->node->block_size; + ret = pos + curr_task->fildes_table[fd]->node->block_size; + /* The seek location is negative. */ + if (ret < 0) + return -EINVAL; + curr_task->fildes_table[fd]->offset = ret; ret = 0; break; } @@ -209,6 +321,7 @@ static int sys_lseek(int fd, off_t pos, int origin) ret = -EINVAL; break; } + return ret; } @@ -217,12 +330,19 @@ static int sys_dup(int fd) task_t* curr_task = per_core(current_task); int new_fd; + /* fd is negative or greater than the maximum allowable number */ if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0)) return -EBADF; + /* fd is not an active, valid file descriptor. */ if (curr_task->fildes_table[fd] == NULL) return -EBADF; + /* Get a free file descriptor */ new_fd = get_fildes(); + /* validate the file descriptor */ + if (BUILTIN_EXPECT((new_fd >= NR_OPEN) || (fd < 0), 0)) + return new_fd; /* in this case fd = errno */ + curr_task->fildes_table[new_fd] = curr_task->fildes_table[fd]; curr_task->fildes_table[fd]->count++; @@ -233,18 +353,26 @@ static int sys_dup2(int fd, int fd2) { task_t* curr_task = per_core(current_task); + /* fd and fd2 is negative or greater than the maximum allowable number */ if (BUILTIN_EXPECT((fd >= NR_OPEN) || (fd < 0), 0)) return -EBADF; if (BUILTIN_EXPECT((fd2 >= NR_OPEN) || (fd2 < 0), 0)) return -EBADF; + /* fd is not an active, valid file descriptor. */ if (curr_task->fildes_table[fd] == NULL) return -EBADF; - if (fd == fd2) + /* If fd and fd2 are equal, then dup2() just returns fd2 */ + if (fd == fd2) return fd2; + + /* + * if descriptor fd2 is already in use, it is first deallocated + * as if a close(2) call had been done first + */ if (curr_task->fildes_table[fd2] != NULL) sys_close(fd2); - + curr_task->fildes_table[fd2] = curr_task->fildes_table[fd]; curr_task->fildes_table[fd]->count++; @@ -296,9 +424,7 @@ int syscall_handler(uint32_t sys_nr, ...) int fd = va_arg(vl, int); const char* buf = va_arg(vl, const char*); size_t len = va_arg(vl, size_t); - if (fd >= 0) - ret = write_fs(per_core(current_task)->fildes_table[fd], - (uint8_t*)buf, len); + ret = sys_write(fd, buf, len); break; } case __NR_open: { @@ -310,8 +436,7 @@ int syscall_handler(uint32_t sys_nr, ...) } case __NR_close: { int fd = va_arg(vl, int); - if (fd >= 0) - ret = sys_close(fd); + ret = sys_close(fd); break; } case __NR_dup: { @@ -328,7 +453,6 @@ int syscall_handler(uint32_t sys_nr, ...) case __NR_stat: { const char* name = va_arg(vl, const char*); struct stat* st = va_arg(vl, struct stat*); - ret = sys_stat(name, st); break; } @@ -336,17 +460,14 @@ int syscall_handler(uint32_t sys_nr, ...) int fd = va_arg(vl, int); const char* buf = va_arg(vl, const char*); size_t len = va_arg(vl, size_t); - if (fd >= 0) - ret = read_fs(per_core(current_task)->fildes_table[fd], - (uint8_t*)buf, len); + ret = sys_read(fd, buf, len); break; } case __NR_lseek: { int fd = va_arg(vl, int); off_t pos = va_arg(vl, off_t); int origin = va_arg(vl, int); - if (fd >= 0) - ret = sys_lseek(fd, pos, origin); + ret = sys_lseek(fd, pos, origin); break; } case __NR_sbrk: { From 84d9058d7a707b126bf065805905a76a61f35c85 Mon Sep 17 00:00:00 2001 From: Marian Ohligs Date: Fri, 30 Sep 2011 11:55:26 +0200 Subject: [PATCH 2/3] fix makefile --- Makefile.example | 2 +- kernel/Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile.example b/Makefile.example index a4b633f7..fd93ee09 100644 --- a/Makefile.example +++ b/Makefile.example @@ -3,7 +3,7 @@ ARCH = x86 NAME = metalsvm LWIPDIRS = lwip/src/arch lwip/src/api lwip/src/core lwip/src/core/ipv4 lwip/src/netif DRIVERDIRS = drivers/net drivers/char -KERNDIRS = libkern kernel mm fs arch/$(ARCH)/kernel arch/$(ARCH)/mm arch/$(ARCH)/scc $(LWIPDIRS) $(DRIVERDIRS) +KERNDIRS = libkern kernel mm fs apps arch/$(ARCH)/kernel arch/$(ARCH)/mm arch/$(ARCH)/scc $(LWIPDIRS) $(DRIVERDIRS) SUBDIRS = $(KERNDIRS) CC_FOR_TARGET=gcc diff --git a/kernel/Makefile b/kernel/Makefile index fabfe28e..0e498e06 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -1,4 +1,4 @@ -C_source := main.c tasks.c syscall.c tests.c echo.c netio.c init.c +C_source := main.c tasks.c syscall.c init.c MODULE := kernel include $(TOPDIR)/Makefile.inc From aecd3b1c8cce419c39bb2c59d561d6b552ecdf30 Mon Sep 17 00:00:00 2001 From: Marian Ohligs Date: Fri, 30 Sep 2011 11:59:45 +0200 Subject: [PATCH 3/3] remove compiler warnings in drivers/char/stdio --- arch/x86/include/asm/kb.h | 4 +++- arch/x86/kernel/kb.c | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/kb.h b/arch/x86/include/asm/kb.h index 1b2dfb40..fcb55196 100644 --- a/arch/x86/include/asm/kb.h +++ b/arch/x86/include/asm/kb.h @@ -53,7 +53,9 @@ typedef struct extern kb_buffer_t kb_buffer; -void kb_flush(); +void kb_init(size_t size, tid_t tid); + +void kb_finish(void); #endif diff --git a/arch/x86/kernel/kb.c b/arch/x86/kernel/kb.c index aabf040c..18a65ee7 100644 --- a/arch/x86/kernel/kb.c +++ b/arch/x86/kernel/kb.c @@ -36,7 +36,7 @@ void kb_init(size_t size, tid_t tid) { return; } -void kb_finish() { +void kb_finish(void) { kfree(kb_buffer.buffer, (kb_buffer.maxsize * sizeof(char))); kb_buffer.buffer = NULL; kb_buffer.size = 0;