Message type and related cleanup

- Intorduce and use a message type for VFS_GETDENTS, VFS_READ,
   VFS_WRITE.

 - Some cleanup to related functions where vir_bytes are replaced (and
   casted to/from, in parameter definition and local variables as well.

   This allow to see more clearly which function receives unsafe
   (pointer) values, or at least values which are not supposed to be
   valid in the address space of VFS. The current patch does so only
   for the minimal amount of functions which are concerned with the
   introduction of the new message type.

Change-Id: I0cdca97409c4016d02fae067b48bf55d37572c5c

include/minix/callnr.h

@@ -232,9 +232,4 @@
 
 #define NR_VFS_CALLS		49	/* highest number from base plus one */
 
-/* Field names for the read(2), write(2), and getdents(2) calls. */
-#define VFS_READWRITE_FD	m1_i1	/* int */
-#define VFS_READWRITE_BUF	m1_p1	/* char * */
-#define VFS_READWRITE_LEN	m1_i2	/* size_t */
-
 #endif /* !_MINIX_CALLNR_H */

include/minix/ipc.h

@@ -225,7 +225,7 @@ _ASSERT_MSG_SIZE(mess_lc_vfs_getvfsstat);
 typedef struct {
 	int fd;
 	unsigned long req;
-	void *arg;
+	vir_bytes arg;
 
 	uint8_t padding[44];
 } mess_lc_vfs_ioctl;
@@ -312,6 +312,15 @@ typedef struct {
 } mess_lc_vfs_readlink;
 _ASSERT_MSG_SIZE(mess_lc_vfs_readlink);
 
+typedef struct {
+	int fd;
+	vir_bytes buf;
+	size_t len;
+
+	uint8_t padding[44];
+} mess_lc_vfs_readwrite;
+_ASSERT_MSG_SIZE(mess_lc_vfs_readwrite);
+
 typedef struct {
 	uint32_t nfds;
 	fd_set *readfds;
@@ -866,6 +875,7 @@ typedef struct {
 		mess_lc_vfs_path	m_lc_vfs_path;
 		mess_lc_vfs_pipe2	m_lc_vfs_pipe2;
 		mess_lc_vfs_readlink	m_lc_vfs_readlink;
+		mess_lc_vfs_readwrite	m_lc_vfs_readwrite;
 		mess_lc_vfs_select	m_lc_vfs_select;
 		mess_lc_vfs_stat	m_lc_vfs_stat;
 		mess_lc_vfs_statvfs1	m_lc_vfs_statvfs1;

lib/libc/sys-minix/getdents.c

@@ -10,9 +10,9 @@ ssize_t getdents(int fd, char *buffer, size_t nbytes)
   message m;
 
   memset(&m, 0, sizeof(m));
-  m.VFS_READWRITE_FD = fd;
+  m.m_lc_vfs_readwrite.fd = fd;
-  m.VFS_READWRITE_LEN = nbytes;
+  m.m_lc_vfs_readwrite.len = nbytes;
-  m.VFS_READWRITE_BUF = (char *) buffer;
+  m.m_lc_vfs_readwrite.buf = (vir_bytes)buffer;
   return _syscall(VFS_PROC_NR, VFS_GETDENTS, &m);
 }
 

lib/libc/sys-minix/ioctl.c

@@ -48,7 +48,7 @@ int     ioctl(int fd, unsigned long request, ...)
 {
   int r, request_save;
   message m;
-  void *addr;
+  vir_bytes addr;
   void *data;
   va_list ap;
 
@@ -66,12 +66,12 @@ int     ioctl(int fd, unsigned long request, ...)
   switch (request) {
 	case I2C_IOCTL_EXEC:
 		rewrite_i2c_netbsd_to_minix(&i2c, data);
-		addr = (void *) &i2c;
+		addr = (vir_bytes) &i2c;
 		request = MINIX_I2C_IOCTL_EXEC;
 		break;
 	default:
 		/* Keep original as-is */
-		addr = (void *) data;
+		addr = data;
 		break;
   }
 

lib/libc/sys-minix/read.c

@@ -14,8 +14,8 @@ ssize_t read(int fd, void *buffer, size_t nbytes)
   message m;
 
   memset(&m, 0, sizeof(m));
-  m.VFS_READWRITE_FD = fd;
+  m.m_lc_vfs_readwrite.fd = fd;
-  m.VFS_READWRITE_LEN = nbytes;
+  m.m_lc_vfs_readwrite.len = nbytes;
-  m.VFS_READWRITE_BUF = (char *) buffer;
+  m.m_lc_vfs_readwrite.buf = (vir_bytes)buffer;
   return(_syscall(VFS_PROC_NR, VFS_READ, &m));
 }

lib/libc/sys-minix/write.c

@@ -10,9 +10,9 @@ ssize_t write(int fd, const void *buffer, size_t nbytes)
   message m;
 
   memset(&m, 0, sizeof(m));
-  m.VFS_READWRITE_FD = fd;
+  m.m_lc_vfs_readwrite.fd = fd;
-  m.VFS_READWRITE_LEN = nbytes;
+  m.m_lc_vfs_readwrite.len = nbytes;
-  m.VFS_READWRITE_BUF = (char *) __UNCONST(buffer);
+  m.m_lc_vfs_readwrite.buf = (vir_bytes)buffer;
   return(_syscall(VFS_PROC_NR, VFS_WRITE, &m));
 }
 

servers/vfs/coredump.c

@@ -175,7 +175,7 @@ static void adjust_offsets(Elf_Phdr phdrs[], int phnum)
  *===========================================================================*/
 static void write_buf(struct filp *f, char *buf, size_t size)
 {
-  read_write(fp, WRITING, f, buf, size, VFS_PROC_NR);
+  read_write(fp, WRITING, f, (vir_bytes)buf, size, VFS_PROC_NR);
 }
 
 /*===========================================================================*

servers/vfs/device.c

@@ -36,7 +36,7 @@
 static int cdev_opcl(int op, dev_t dev, int flags);
 static int block_io(endpoint_t driver_e, message *mess_ptr);
 static cp_grant_id_t make_grant(endpoint_t driver_e, endpoint_t user_e, int op,
-	void *buf, unsigned long size);
+	vir_bytes buf, unsigned long size);
 
 /*===========================================================================*
  *				bdev_open				     *
@@ -104,7 +104,7 @@ int bdev_close(dev_t dev)
  *				bdev_ioctl				     *
  *===========================================================================*/
 static int bdev_ioctl(dev_t dev, endpoint_t proc_e, unsigned long req,
-	void *buf)
+	vir_bytes buf)
 {
 /* Perform an I/O control operation on a block device. */
   struct dmap *dp;
@@ -155,7 +155,7 @@ static int bdev_ioctl(dev_t dev, endpoint_t proc_e, unsigned long req,
  *				make_grant				     *
  *===========================================================================*/
 static cp_grant_id_t make_grant(endpoint_t driver_e, endpoint_t user_e, int op,
-	void *buf, unsigned long bytes)
+	vir_bytes buf, unsigned long bytes)
 {
 /* Create a magic grant for the given operation and buffer. */
   cp_grant_id_t gid;
@@ -165,7 +165,7 @@ static cp_grant_id_t make_grant(endpoint_t driver_e, endpoint_t user_e, int op,
   switch (op) {
   case CDEV_READ:
   case CDEV_WRITE:
-	gid = cpf_grant_magic(driver_e, user_e, (vir_bytes) buf,
+	gid = cpf_grant_magic(driver_e, user_e, buf,
 		(size_t) bytes, op == CDEV_READ ? CPF_WRITE : CPF_READ);
 	break;
 
@@ -186,7 +186,7 @@ static cp_grant_id_t make_grant(endpoint_t driver_e, endpoint_t user_e, int op,
 	 * although now that we no longer identify responses based on grants,
 	 * this is not strictly necessary.
 	 */
-	gid = cpf_grant_magic(driver_e, user_e, (vir_bytes) buf, size, access);
+	gid = cpf_grant_magic(driver_e, user_e, buf, size, access);
 	break;
 
   default:
@@ -249,7 +249,7 @@ int cdev_io(
   int op,			/* CDEV_READ, CDEV_WRITE, or CDEV_IOCTL */
   dev_t dev,			/* major-minor device number */
   endpoint_t proc_e,		/* in whose address space is buf? */
-  void *buf,			/* virtual address of the buffer */
+  vir_bytes buf,		/* virtual address of the buffer */
   off_t pos,			/* byte position */
   unsigned long bytes,		/* how many bytes to transfer, or request */
   int flags			/* special flags, like O_NONBLOCK */
@@ -490,11 +490,11 @@ int do_ioctl(void)
   struct filp *f;
   register struct vnode *vp;
   dev_t dev;
-  void *argx;
+  vir_bytes argx;
 
   scratch(fp).file.fd_nr = job_m_in.m_lc_vfs_ioctl.fd;
   ioctlrequest = job_m_in.m_lc_vfs_ioctl.req;
-  argx = job_m_in.m_lc_vfs_ioctl.arg;
+  argx = (vir_bytes)job_m_in.m_lc_vfs_ioctl.arg;
 
   if ((f = get_filp(scratch(fp).file.fd_nr, VNODE_READ)) == NULL)
 	return(err_code);

servers/vfs/lock.c

@@ -31,7 +31,7 @@ int req;			/* either F_SETLK or F_SETLKW */
   struct file_lock *flp, *flp2, *empty;
 
   /* Fetch the flock structure from user space. */
-  r = sys_datacopy_wrapper(who_e, (vir_bytes)scratch(fp).io.io_buffer, VFS_PROC_NR,
+  r = sys_datacopy_wrapper(who_e, scratch(fp).io.io_buffer, VFS_PROC_NR,
 		   (vir_bytes) &flock, sizeof(flock));
   if (r != OK) return(EINVAL);
 
@@ -142,7 +142,7 @@ int req;			/* either F_SETLK or F_SETLKW */
 
 	/* Copy the flock structure back to the caller. */
 	r = sys_datacopy_wrapper(VFS_PROC_NR, (vir_bytes) &flock, who_e,
-		(vir_bytes)scratch(fp).io.io_buffer, sizeof(flock));
+		scratch(fp).io.io_buffer, sizeof(flock));
 	return(r);
   }
 

servers/vfs/main.c

@@ -822,15 +822,15 @@ struct fproc *rfp;
   case VFS_READ:
   case VFS_WRITE:
 	assert(blocked_on == FP_BLOCKED_ON_PIPE);
-	m_in.VFS_READWRITE_FD = scratch(rfp).file.fd_nr;
+	m_in.m_lc_vfs_readwrite.fd = scratch(rfp).file.fd_nr;
-	m_in.VFS_READWRITE_BUF = scratch(rfp).io.io_buffer;
+	m_in.m_lc_vfs_readwrite.buf = scratch(rfp).io.io_buffer;
-	m_in.VFS_READWRITE_LEN = scratch(rfp).io.io_nbytes;
+	m_in.m_lc_vfs_readwrite.len = scratch(rfp).io.io_nbytes;
 	break;
   case VFS_FCNTL:
 	assert(blocked_on == FP_BLOCKED_ON_LOCK);
 	m_in.m_lc_vfs_fcntl.fd = scratch(rfp).file.fd_nr;
 	m_in.m_lc_vfs_fcntl.cmd = scratch(rfp).io.io_nbytes;
-	m_in.m_lc_vfs_fcntl.arg_ptr = (vir_bytes)scratch(rfp).io.io_buffer;
+	m_in.m_lc_vfs_fcntl.arg_ptr = scratch(rfp).io.io_buffer;
 	assert(m_in.m_lc_vfs_fcntl.cmd == F_SETLKW);
 	break;
   default:

servers/vfs/misc.c

@@ -104,9 +104,7 @@ int do_fcntl(void)
   tll_access_t locktype;
 
   scratch(fp).file.fd_nr = job_m_in.m_lc_vfs_fcntl.fd;
-  /* LSC: io_buffer is used everywhere as a valid VFS memory space pointer.
+  scratch(fp).io.io_buffer = job_m_in.m_lc_vfs_fcntl.arg_ptr;
-   * Seems downright scary to me. */
-  scratch(fp).io.io_buffer = (char *)job_m_in.m_lc_vfs_fcntl.arg_ptr;
   scratch(fp).io.io_nbytes = job_m_in.m_lc_vfs_fcntl.cmd;
   fcntl_req = job_m_in.m_lc_vfs_fcntl.cmd;
   fcntl_argx = job_m_in.m_lc_vfs_fcntl.arg_int;
@@ -172,7 +170,7 @@ int do_fcntl(void)
 	else if (!(f->filp_mode & W_BIT)) r = EBADF;
 	else {
 		/* Copy flock data from userspace. */
-		r = sys_datacopy_wrapper(who_e, (vir_bytes)scratch(fp).io.io_buffer,
+		r = sys_datacopy_wrapper(who_e, scratch(fp).io.io_buffer,
 			SELF, (vir_bytes) &flock_arg, sizeof(flock_arg));
 	}
 
@@ -442,7 +440,7 @@ int do_vm_call(void)
 
 			if(result == OK) {
 				result = actual_read_write_peek(fp, PEEKING,
-					req_fd, NULL, length);
+					req_fd, /* vir_bytes */ 0, length);
 			}
 
 			break;

servers/vfs/path.c

@@ -621,8 +621,8 @@ char ename[NAME_MAX + 1];
   if (!S_ISDIR(dirp->v_mode)) return(EBADF);
 
   do {
-	r = req_getdents(dirp->v_fs_e, dirp->v_inode_nr, pos, buf, sizeof(buf),
+	r = req_getdents(dirp->v_fs_e, dirp->v_inode_nr, pos, (vir_bytes)buf,
-			 &new_pos, 1);
+		sizeof(buf), &new_pos, 1);
 
 	if (r == 0) {
 		return(ENOENT); /* end of entries -- matching inode !found */

servers/vfs/pipe.c

@@ -327,7 +327,7 @@ void wait_for(endpoint_t who)
  *===========================================================================*/
 void pipe_suspend(filp, buf, size)
 struct filp *filp;
-char *buf;
+vir_bytes buf;
 size_t size;
 {
 /* Take measures to suspend the processing of the present system call.

servers/vfs/proto.h

@@ -32,7 +32,7 @@ int vm_vfs_procctl_handlemem(endpoint_t ep, vir_bytes mem, vir_bytes len, int fl
 /* device.c */
 int cdev_open(dev_t dev, int flags);
 int cdev_close(dev_t dev);
-int cdev_io(int op, dev_t dev, endpoint_t proc_e, void *buf, off_t pos,
+int cdev_io(int op, dev_t dev, endpoint_t proc_e, vir_bytes buf, off_t pos,
 	unsigned long bytes, int flags);
 int cdev_select(dev_t dev, int ops);
 int cdev_cancel(dev_t dev);
@@ -168,7 +168,7 @@ int pipe_check(struct filp *filp, int rw_flag, int oflags, int bytes,
 void release(struct vnode *vp, int op, int count);
 void revive(endpoint_t proc_e, int returned);
 void suspend(int why);
-void pipe_suspend(struct filp *rfilp, char *buf, size_t size);
+void pipe_suspend(struct filp *rfilp, vir_bytes buf, size_t size);
 void unsuspend_by_endpt(endpoint_t proc_e);
 void wait_for(endpoint_t proc_e);
 
@@ -187,12 +187,12 @@ int do_getdents(void);
 void lock_bsf(void);
 void unlock_bsf(void);
 void check_bsf_lock(void);
-int do_read_write_peek(int rw_flag, int fd, char *buf, size_t bytes);
+int do_read_write_peek(int rw_flag, int fd, vir_bytes buf, size_t bytes);
-int actual_read_write_peek(struct fproc *rfp, int rw_flag, int fd, char *buf,
+int actual_read_write_peek(struct fproc *rfp, int rw_flag, int fd, vir_bytes buf,
 	size_t bytes);
-int read_write(struct fproc *rfp, int rw_flag, struct filp *f, char *buffer,
+int read_write(struct fproc *rfp, int rw_flag, struct filp *f, vir_bytes buffer,
 	size_t nbytes, endpoint_t for_e);
-int rw_pipe(int rw_flag, endpoint_t usr, struct filp *f, char *buf,
+int rw_pipe(int rw_flag, endpoint_t usr, struct filp *f, vir_bytes buf,
 	size_t req_size);
 
 /* request.c */
@@ -208,7 +208,7 @@ int req_create(endpoint_t fs_e, ino_t inode_nr, int omode, uid_t uid,
 int req_flush(endpoint_t fs_e, dev_t dev);
 int req_statvfs(endpoint_t fs_e, struct statvfs *buf);
 int req_ftrunc(endpoint_t fs_e, ino_t inode_nr, off_t start, off_t end);
-int req_getdents(endpoint_t fs_e, ino_t inode_nr, off_t pos, char *buf,
+int req_getdents(endpoint_t fs_e, ino_t inode_nr, off_t pos, vir_bytes buf,
 	size_t size, off_t *new_pos, int direct);
 int req_inhibread(endpoint_t fs_e, ino_t inode_nr);
 int req_link(endpoint_t fs_e, ino_t link_parent, char *lastc,

servers/vfs/read.c

@@ -30,8 +30,8 @@
  *===========================================================================*/
 int do_read(void)
 {
-  return(do_read_write_peek(READING, job_m_in.VFS_READWRITE_FD,
+  return(do_read_write_peek(READING, job_m_in.m_lc_vfs_readwrite.fd,
-          job_m_in.VFS_READWRITE_BUF, (size_t) job_m_in.VFS_READWRITE_LEN));
+          job_m_in.m_lc_vfs_readwrite.buf, job_m_in.m_lc_vfs_readwrite.len));
 }
 
 
@@ -82,7 +82,7 @@ void check_bsf_lock(void)
  *				actual_read_write_peek			     *
  *===========================================================================*/
 int actual_read_write_peek(struct fproc *rfp, int rw_flag, int io_fd,
-	char *io_buf, size_t io_nbytes)
+	vir_bytes io_buf, size_t io_nbytes)
 {
 /* Perform read(fd, buffer, nbytes) or write(fd, buffer, nbytes) call. */
   struct filp *f;
@@ -121,7 +121,7 @@ int actual_read_write_peek(struct fproc *rfp, int rw_flag, int io_fd,
 /*===========================================================================*
  *				do_read_write_peek			     *
  *===========================================================================*/
-int do_read_write_peek(int rw_flag, int io_fd, char *io_buf, size_t io_nbytes)
+int do_read_write_peek(int rw_flag, int io_fd, vir_bytes io_buf, size_t io_nbytes)
 {
 	return actual_read_write_peek(fp, rw_flag, io_fd, io_buf, io_nbytes);
 }
@@ -130,7 +130,7 @@ int do_read_write_peek(int rw_flag, int io_fd, char *io_buf, size_t io_nbytes)
  *				read_write				     *
  *===========================================================================*/
 int read_write(struct fproc *rfp, int rw_flag, struct filp *f,
-	char *buf, size_t size, endpoint_t for_e)
+	vir_bytes buf, size_t size, endpoint_t for_e)
 {
   register struct vnode *vp;
   off_t position, res_pos;
@@ -207,7 +207,7 @@ int read_write(struct fproc *rfp, int rw_flag, struct filp *f,
 		r = req_bpeek(vp->v_bfs_e, vp->v_sdev, position, size);
 	} else {
 		r = req_breadwrite(vp->v_bfs_e, for_e, vp->v_sdev, position,
-		       size, (vir_bytes) buf, rw_flag, &res_pos, &res_cum_io);
+		       size, buf, rw_flag, &res_pos, &res_cum_io);
 		if (r == OK) {
 			position = res_pos;
 			cum_io += res_cum_io;
@@ -227,7 +227,7 @@ int read_write(struct fproc *rfp, int rw_flag, struct filp *f,
 	} else {
 		off_t new_pos;
 		r = req_readwrite(vp->v_fs_e, vp->v_inode_nr, position,
-			rw_flag, for_e, (vir_bytes) buf, size, &new_pos,
+			rw_flag, for_e, buf, size, &new_pos,
 			&cum_io_incr);
 
 		if (r >= 0) {
@@ -273,9 +273,9 @@ int do_getdents(void)
   off_t new_pos;
   register struct filp *rfilp;
 
-  scratch(fp).file.fd_nr = job_m_in.VFS_READWRITE_FD;
+  scratch(fp).file.fd_nr = job_m_in.m_lc_vfs_readwrite.fd;
-  scratch(fp).io.io_buffer = job_m_in.VFS_READWRITE_BUF;
+  scratch(fp).io.io_buffer = job_m_in.m_lc_vfs_readwrite.buf;
-  scratch(fp).io.io_nbytes = (size_t) job_m_in.VFS_READWRITE_LEN;
+  scratch(fp).io.io_nbytes = job_m_in.m_lc_vfs_readwrite.len;
 
   /* Is the file descriptor valid? */
   if ( (rfilp = get_filp(scratch(fp).file.fd_nr, VNODE_READ)) == NULL)
@@ -306,7 +306,7 @@ int rw_pipe(rw_flag, usr_e, f, buf, req_size)
 int rw_flag;			/* READING or WRITING */
 endpoint_t usr_e;
 struct filp *f;
-char *buf;
+vir_bytes buf;
 size_t req_size;
 {
   int r, oflags, partial_pipe = 0;
@@ -345,7 +345,7 @@ size_t req_size;
 	panic("unmapped pipe");
 
   r = req_readwrite(vp->v_mapfs_e, vp->v_mapinode_nr, position, rw_flag, usr_e,
-		    (vir_bytes) buf, size, &new_pos, &cum_io_incr);
+		    buf, size, &new_pos, &cum_io_incr);
 
   if (r != OK) {
 	return(r);

servers/vfs/request.c

@@ -287,7 +287,7 @@ static int req_getdents_actual(
   endpoint_t fs_e,
   ino_t inode_nr,
   off_t pos,
-  char *buf,
+  vir_bytes buf,
   size_t size,
   off_t *new_pos,
   int direct,
@@ -303,9 +303,9 @@ static int req_getdents_actual(
   assert(vmp != NULL);
 
   if (direct) {
-	grant_id = cpf_grant_direct(fs_e, (vir_bytes) buf, size, CPF_WRITE);
+	grant_id = cpf_grant_direct(fs_e, buf, size, CPF_WRITE);
   } else {
-	grant_id = cpf_grant_magic(fs_e, who_e, (vir_bytes) buf, size,
+	grant_id = cpf_grant_magic(fs_e, who_e, buf, size,
 				   CPF_WRITE | cpflag);
   }
 
@@ -341,7 +341,7 @@ int req_getdents(
   endpoint_t fs_e,
   ino_t inode_nr,
   off_t pos,
-  char *buf,
+  vir_bytes buf,
   size_t size,
   off_t *new_pos,
   int direct)
@@ -352,8 +352,7 @@ int req_getdents(
 		direct, CPF_TRY);
 
 	if(r == EFAULT && !direct) {
-		if((r=vm_vfs_procctl_handlemem(who_e, (vir_bytes) buf,
+		if((r=vm_vfs_procctl_handlemem(who_e, buf, size, 1)) != OK) {
-			size, 1)) != OK) {
 			return r;
 		}
 

servers/vfs/scratchpad.h

@@ -10,7 +10,7 @@ EXTERN struct scratchpad {
 	struct filp *filp;
   } file;
   struct io_cmd {
-	char *io_buffer;
+	vir_bytes io_buffer;
 	size_t io_nbytes;
   } io;
 } scratchpad[NR_PROCS];

servers/vfs/write.c

@@ -15,6 +15,6 @@
 int do_write(void)
 {
 /* Perform the write(fd, buffer, nbytes) system call. */
-  return(do_read_write_peek(WRITING, job_m_in.VFS_READWRITE_FD,
+  return(do_read_write_peek(WRITING, job_m_in.m_lc_vfs_readwrite.fd,
-	job_m_in.VFS_READWRITE_BUF, (size_t) job_m_in.VFS_READWRITE_LEN));
+	job_m_in.m_lc_vfs_readwrite.buf, job_m_in.m_lc_vfs_readwrite.len));
 }