kernel/virt_cpy: error struct, better error handling

6 files changed, 84 insertions, 54 deletions

diff --git a/src/kernel/arch/amd64/driver/fsroot.c b/src/kernel/arch/amd64/driver/fsroot.c
index e651c47..5e48638 100644
--- a/src/kernel/arch/amd64/driver/fsroot.c
+++ b/src/kernel/arch/amd64/driver/fsroot.c
@@ -1,9 +1,10 @@
 #include <kernel/arch/amd64/ata.h>
+#include <kernel/arch/amd64/driver/fsroot.h>
 #include <kernel/mem/virt.h>
 #include <kernel/panic.h>
 #include <kernel/proc.h>
 #include <kernel/util.h>
-#include <kernel/arch/amd64/driver/fsroot.h>
+#include <shared/errno.h>
 #include <shared/mem.h>
 #include <stdbool.h>
 
@@ -42,6 +43,16 @@ static void req_preprocess(struct vfs_request *req, size_t max_len) {
 	assert(req->input.len + req->offset <= max_len);
 }
 
+static int req_readcopy(struct vfs_request *req, const void *buf, size_t len) {
+	assert(req->type == VFSOP_READ);
+	req_preprocess(req, len);
+	virt_cpy_to(
+			req->caller->pages, req->output.buf,
+			buf + req->offset, req->output.len);
+	/* read errors are ignored. TODO write docs */
+	return req->output.len;
+}
+
 
 static int handle(struct vfs_request *req) {
 	assert(req->caller);
@@ -72,18 +83,10 @@ static int handle(struct vfs_request *req) {
 						"com1\0"
 						"ps2\0"
 						"ata/";
-					req_preprocess(req, sizeof src);
-					virt_cpy_to(req->caller->pages, req->output.buf,
-							src + req->offset, req->output.len);
-					return req->output.len;
-				}
-				case HANDLE_VGA: {
-					char *vga = (void*)0xB8000;
-					req_preprocess(req, 80*25*2);
-					virt_cpy_to(req->caller->pages, req->output.buf,
-							vga + req->offset, req->output.len);
-					return req->output.len;
+					return req_readcopy(req, src, sizeof src);
 				}
+				case HANDLE_VGA:
+					return req_readcopy(req, (void*)0xB8000, 80*25*2);
 				case HANDLE_ATA_ROOT: {
 					char list[8] = {};
 					size_t len = 0;
@@ -93,13 +96,10 @@ static int handle(struct vfs_request *req) {
 							len += 2;
 						}
 					}
-					req_preprocess(req, len);
-					virt_cpy_to(req->caller->pages, req->output.buf,
-							list + req->offset, req->output.len);
-					return req->output.len;
+					return req_readcopy(req, list, len);
 				}
 				case HANDLE_ATA:   case HANDLE_ATA+1:
-				case HANDLE_ATA+2: case HANDLE_ATA+3: {
+				case HANDLE_ATA+2: case HANDLE_ATA+3:
 					if (req->offset < 0) return 0;
 					char buf[512];
 					uint32_t sector = req->offset / 512;
@@ -107,7 +107,6 @@ static int handle(struct vfs_request *req) {
 					ata_read(id - HANDLE_ATA, sector, buf);
 					virt_cpy_to(req->caller->pages, req->output.buf, buf, len);
 					return len;
-				}
 				default: panic_invalid_state();
 			}
 
@@ -116,8 +115,11 @@ static int handle(struct vfs_request *req) {
 				case HANDLE_VGA: {
 					void *vga = (void*)0xB8000;
 					req_preprocess(req, 80*25*2);
-					virt_cpy_from(req->caller->pages, vga + req->offset,
-							req->input.buf, req->input.len);
+					if (!virt_cpy_from(req->caller->pages, vga + req->offset,
+							req->input.buf, req->input.len))
+					{
+						return -EFAULT;
+					}
 					return req->input.len;
 				}
 				default: return -1;
diff --git a/src/kernel/mem/virt.c b/src/kernel/mem/virt.c
index f0dca06..bff4b5e 100644
--- a/src/kernel/mem/virt.c
+++ b/src/kernel/mem/virt.c
@@ -1,5 +1,6 @@
 #include <kernel/arch/generic.h>
 #include <kernel/mem/virt.h>
+#include <kernel/panic.h>
 #include <kernel/util.h>
 #include <shared/mem.h>
 
@@ -52,12 +53,13 @@ bool virt_iter_next(struct virt_iter *iter) {
 	return true;
 }
 
-bool virt_cpy(
+size_t virt_cpy(
 		struct pagedir *dest_pages,       void __user *dest,
-		struct pagedir  *src_pages, const void __user *src, size_t length)
+		struct pagedir  *src_pages, const void __user *src,
+		size_t length, struct virt_cpy_error *err)
 {
 	struct virt_iter dest_iter, src_iter;
-	size_t cur_len;
+	size_t total = 0, partial;
 
 	virt_iter_new(&dest_iter,           dest, length, dest_pages, true, true);
 	virt_iter_new( &src_iter, (userptr_t)src, length,  src_pages, true, false);
@@ -65,19 +67,26 @@ bool virt_cpy(
 	src_iter.frag_len  = 0;
 
 	for (;;) {
-		if (dest_iter.frag_len <= 0)
-			if (!virt_iter_next(&dest_iter)) break;
-		if ( src_iter.frag_len <= 0)
-			if (!virt_iter_next( &src_iter)) break;
+		if (dest_iter.frag_len <= 0 && !virt_iter_next(&dest_iter)) break;
+		if ( src_iter.frag_len <= 0 && !virt_iter_next( &src_iter)) break;
 
-		cur_len = min(src_iter.frag_len, dest_iter.frag_len);
-		memcpy(dest_iter.frag, src_iter.frag, cur_len);
+		partial = min(src_iter.frag_len, dest_iter.frag_len);
+		total += partial;
+		memcpy(dest_iter.frag, src_iter.frag, partial);
 
-		dest_iter.frag_len -= cur_len;
-		dest_iter.frag     += cur_len;
-		src_iter.frag_len  -= cur_len;
-		src_iter.frag      += cur_len;
+		dest_iter.frag_len -= partial;
+		dest_iter.frag     += partial;
+		src_iter.frag_len  -= partial;
+		src_iter.frag      += partial;
 	}
 
-	return !(dest_iter.error || src_iter.error);
+	if (err) {
+		err->read_fail  = src_iter.error;
+		err->write_fail = dest_iter.error;
+	}
+	if (src_iter.error || dest_iter.error)
+		assert(total != length);
+	else
+		assert(total == length);
+	return total;
 }
diff --git a/src/kernel/mem/virt.h b/src/kernel/mem/virt.h
index 7d95b3b..cf22a75 100644
--- a/src/kernel/mem/virt.h
+++ b/src/kernel/mem/virt.h
@@ -18,6 +18,10 @@ struct virt_iter {
 	bool _writeable;
 };
 
+struct virt_cpy_error {
+	bool read_fail, write_fail;
+};
+
 /* if pages == NULL, create an iterator over physical memory. */
 void virt_iter_new(
 		struct virt_iter *iter, void __user *virt, size_t length,
@@ -25,17 +29,20 @@ void virt_iter_new(
 
 bool virt_iter_next(struct virt_iter *);
 
-bool virt_cpy(
+size_t virt_cpy(
 		struct pagedir *dest_pages,       void __user *dest,
-		struct pagedir  *src_pages, const void __user *src, size_t length);
+		struct pagedir  *src_pages, const void __user *src,
+		size_t length, struct virt_cpy_error *err);
 
 
-static inline bool virt_cpy_to(struct pagedir *dest_pages, // physical -> virtual
+/* copies to virtual memory, returns true on success */
+static inline bool virt_cpy_to(struct pagedir *dest_pages,
 		void __user *dest, const void *src, size_t length) {
-	return virt_cpy(dest_pages, dest, NULL, (userptr_t)src, length);
+	return length == virt_cpy(dest_pages, dest, NULL, (userptr_t)src, length, NULL);
 }
 
-static inline bool virt_cpy_from(struct pagedir *src_pages, // virtual -> physical
+/* copies from virtual memory, returns true on success */
+static inline bool virt_cpy_from(struct pagedir *src_pages,
 		void *dest, const void __user *src, size_t length) {
-	return virt_cpy(NULL, (userptr_t)dest, src_pages, src, length);
+	return length == virt_cpy(NULL, (userptr_t)dest, src_pages, src, length, NULL);
 }
diff --git a/src/kernel/pipe.c b/src/kernel/pipe.c
index edd1388..2dc98fe 100644
--- a/src/kernel/pipe.c
+++ b/src/kernel/pipe.c
@@ -27,6 +27,7 @@ bool pipe_joinqueue(struct handle *h, bool wants_write,
 
 void pipe_trytransfer(struct handle *h) {
 	struct process *rdr, *wtr;
+	struct virt_cpy_error cpyerr;
 	int len;
 	assert(h);
 	if (!h->pipe.sister) {
@@ -43,12 +44,12 @@ void pipe_trytransfer(struct handle *h) {
 
 	len = min(rdr->waits4pipe.len, wtr->waits4pipe.len);
 
-	if (!virt_cpy(
+	virt_cpy(
 			rdr->pages, rdr->waits4pipe.buf,
-			wtr->pages, wtr->waits4pipe.buf, len))
-	{
+			wtr->pages, wtr->waits4pipe.buf,
+			len, &cpyerr);
+	if (cpyerr.read_fail || cpyerr.write_fail)
 		panic_unimplemented();
-	}
 	h->pipe.queued = h->pipe.queued->waits4pipe.next;
 	h->pipe.sister->pipe.queued = h->pipe.sister->pipe.queued->waits4pipe.next;
 	process_transition(rdr, PS_RUNNING);
diff --git a/src/kernel/syscalls.c b/src/kernel/syscalls.c
index c5e12b4..786578a 100644
--- a/src/kernel/syscalls.c
+++ b/src/kernel/syscalls.c
@@ -292,10 +292,13 @@ long _syscall_fs_respond(void __user *buf, long ret, int flags) {
 		// if this vfsop outputs data and ret is positive, it's the length of the buffer
 		// TODO document
 		ret = min(ret, capped_cast32(req->output.len));
-		if (!virt_cpy(req->caller->pages, req->output.buf,
-					process_current->pages, buf, ret)) {
-			// how should this error even be handled? TODO
-		}
+		struct virt_cpy_error err;
+		virt_cpy(req->caller->pages, req->output.buf,
+				process_current->pages, buf, ret, &err);
+
+		if (err.read_fail)
+			panic_unimplemented();
+		/* write failures are ignored */
 	}
 
 	process_current->handled_req = NULL;
diff --git a/src/kernel/vfs/request.c b/src/kernel/vfs/request.c
index 87d6208..b056a31 100644
--- a/src/kernel/vfs/request.c
+++ b/src/kernel/vfs/request.c
@@ -4,6 +4,7 @@
 #include <kernel/panic.h>
 #include <kernel/proc.h>
 #include <kernel/vfs/request.h>
+#include <shared/errno.h>
 #include <shared/mem.h>
 
 void vfsreq_create(struct vfs_request req_) {
@@ -102,6 +103,7 @@ void vfs_backend_tryaccept(struct vfs_backend *backend) {
 void vfs_backend_user_accept(struct vfs_request *req) {
 	struct process *handler;
 	struct fs_wait_response res = {0};
+	struct virt_cpy_error cpyerr;
 	int len = 0;
 
 	assert(req && req->backend && req->backend->user.handler);
@@ -115,9 +117,15 @@ void vfs_backend_user_accept(struct vfs_request *req) {
 
 	if (req->input.buf) {
 		len = min(req->input.len, handler->awaited_req.max_len);
-		if (!virt_cpy(handler->pages, handler->awaited_req.buf, 
-					req->input.kern ? NULL : req->caller->pages, req->input.buf, len))
-			goto fail; // can't copy buffer
+		virt_cpy(handler->pages, handler->awaited_req.buf,
+				 req->input.kern ? NULL : req->caller->pages, req->input.buf,
+				 len, &cpyerr);
+		if (cpyerr.write_fail)
+			panic_unimplemented();
+		if (cpyerr.read_fail) {
+			vfsreq_finish_short(req, -EFAULT);
+			return;
+		}
 	}
 
 	res.len      = len;
@@ -129,15 +137,15 @@ void vfs_backend_user_accept(struct vfs_request *req) {
 
 	if (!virt_cpy_to(handler->pages,
 				handler->awaited_req.res, &res, sizeof res))
-		goto fail; // can't copy response struct
+	{
+		panic_unimplemented();
+	}
 
 	process_transition(handler, PS_RUNNING);
 	handler->handled_req = req;
 	req->backend->user.handler = NULL;
 	regs_savereturn(&handler->regs, 0);
 	return;
-fail:
-	panic_unimplemented(); // TODO
 }
 
 void vfs_backend_refdown(struct vfs_backend *b) {