From 6c597561bc000a799e9ff45966cccc456c67f96a Mon Sep 17 00:00:00 2001
From: Thomas Veerman <thomas@minix3.org>
Date: Thu, 26 Jul 2012 15:19:08 +0000
Subject: [PATCH] EXT2: various fixes

.enable all compile time warnings and make them errors
.refactor functions with unused parameters
.fix null pointer dereference before checking for null
.proper variable initialization
.use safe string copy functions
.fix massive memory corruption bug in fs_getdents
---
 servers/ext2/Makefile  |  1 +
 servers/ext2/balloc.c  |  2 +-
 servers/ext2/cache.c   |  3 ++-
 servers/ext2/ialloc.c  | 14 +++++-------
 servers/ext2/link.c    |  7 +++---
 servers/ext2/main.c    |  4 ++--
 servers/ext2/path.c    |  2 +-
 servers/ext2/proto.h   |  2 +-
 servers/ext2/read.c    | 50 +++++++++++++++++++++++-------------------
 servers/ext2/super.c   |  2 +-
 servers/ext2/utility.c |  2 +-
 servers/ext2/write.c   |  2 +-
 12 files changed, 48 insertions(+), 43 deletions(-)

diff --git a/servers/ext2/Makefile b/servers/ext2/Makefile
index 31aa46186..090e7cd70 100644
--- a/servers/ext2/Makefile
+++ b/servers/ext2/Makefile
@@ -11,5 +11,6 @@ LDADD+= -lminixfs -lbdev -lsys
 MAN=
 
 BINDIR?= /sbin
+CFLAGS+= -Wall -Wextra -Werror
 
 .include <minix.service.mk>
diff --git a/servers/ext2/balloc.c b/servers/ext2/balloc.c
index 50c43ac56..1a93f5407 100644
--- a/servers/ext2/balloc.c
+++ b/servers/ext2/balloc.c
@@ -210,7 +210,7 @@ struct inode *rip;		/* used for preallocation */
 		/* we preallocate bytes only */
 		ASSERT(EXT2_PREALLOC_BLOCKS == sizeof(char)*CHAR_BIT);
 
-		bit = setbyte(bp->b_bitmap, sp->s_blocks_per_group, word);
+		bit = setbyte(bp->b_bitmap, sp->s_blocks_per_group);
 		if (bit != -1) {
 			block = bit + sp->s_first_data_block +
 					group * sp->s_blocks_per_group;
diff --git a/servers/ext2/cache.c b/servers/ext2/cache.c
index 919b3f56a..149267e32 100644
--- a/servers/ext2/cache.c
+++ b/servers/ext2/cache.c
@@ -351,7 +351,7 @@ void flushall(
 /* Flush all dirty blocks for one device. */
 
   register struct buf *bp;
-  static struct buf **dirty;	/* static so it isn't on stack */
+  static struct buf **dirty = NULL;	/* static so it isn't on stack */
   static int unsigned dirtylistsize = 0;
   int ndirty;
 
@@ -391,6 +391,7 @@ void rw_scattered(
   int j, r;
 
   STATICINIT(iovec, NR_IOREQS);
+  assert(bufq != NULL);
 
   /* (Shell) sort buffers on b_blocknr. */
   gap = 1;
diff --git a/servers/ext2/ialloc.c b/servers/ext2/ialloc.c
index 03b39f2ac..1faca7c52 100644
--- a/servers/ext2/ialloc.c
+++ b/servers/ext2/ialloc.c
@@ -108,12 +108,10 @@ void free_inode(
 }
 
 
-static int find_group_dir(struct super_block *sp, struct inode
-	*parent);
+static int find_group_dir(struct super_block *sp);
 static int find_group_hashalloc(struct super_block *sp, struct inode
 	*parent);
-static int find_group_any(struct super_block *sp, struct inode
-	*parent);
+static int find_group_any(struct super_block *sp);
 static int find_group_orlov(struct super_block *sp, struct inode
 	*parent);
 
@@ -136,13 +134,13 @@ int is_dir;			/* inode will be a directory if it is TRUE */
 	panic("can't alloc inode on read-only filesys.");
 
   if (opt.mfsalloc) {
-	group = find_group_any(sp, parent);
+	group = find_group_any(sp);
   } else {
 	if (is_dir) {
 		if (opt.use_orlov) {
 			group = find_group_orlov(sp, parent);
 		} else {
-			group = find_group_dir(sp, parent);
+			group = find_group_dir(sp);
 		}
 	} else {
 		group = find_group_hashalloc(sp, parent);
@@ -252,7 +250,7 @@ static void free_inode_bit(struct super_block *sp, bit_t bit_returned,
 
 
 /* it's implemented very close to the linux' find_group_dir() */
-static int find_group_dir(struct super_block *sp, struct inode *parent)
+static int find_group_dir(struct super_block *sp)
 {
   int avefreei = sp->s_free_inodes_count / sp->s_groups_count;
   struct group_desc *gd, *best_gd = NULL;
@@ -339,7 +337,7 @@ static int find_group_hashalloc(struct super_block *sp, struct inode *parent)
 /* Find first group which has free inode slot.
  * This is similar to what MFS does.
  */
-static int find_group_any(struct super_block *sp, struct inode *parent)
+static int find_group_any(struct super_block *sp)
 {
   int ngroups = sp->s_groups_count;
   struct group_desc *gd;
diff --git a/servers/ext2/link.c b/servers/ext2/link.c
index b593306b0..88ac687ea 100644
--- a/servers/ext2/link.c
+++ b/servers/ext2/link.c
@@ -200,11 +200,12 @@ int fs_rdlink()
 		r = EIO;
 	} else {
 		bp = get_block(rip->i_dev, b, NORMAL);
-		link_text = bp->b_data;
-		if (bp)
+		if (bp != NULL) {
+			link_text = bp->b_data;
 			r = OK;
-		else
+		} else {
 			r = EIO;
+		}
 	}
   } else {
         /* fast symlink, stored in inode */
diff --git a/servers/ext2/main.c b/servers/ext2/main.c
index 9c330ef9c..09948a8b5 100644
--- a/servers/ext2/main.c
+++ b/servers/ext2/main.c
@@ -32,7 +32,7 @@ static struct optset optset_table[] = {
   { "reserved",		OPT_BOOL,   &opt.use_reserved_blocks,	TRUE    },
   { "prealloc",		OPT_BOOL,   &opt.use_prealloc, 		TRUE	},
   { "noprealloc",	OPT_BOOL,   &opt.use_prealloc, 		FALSE	},
-  { NULL								}
+  { NULL,		0,	    NULL,			0								}
 };
 
 /*===========================================================================*
@@ -125,7 +125,7 @@ static void sef_local_startup()
 /*===========================================================================*
  *		            sef_cb_init_fresh                                *
  *===========================================================================*/
-static int sef_cb_init_fresh(int type, sef_init_info_t *info)
+static int sef_cb_init_fresh(int UNUSED(type), sef_init_info_t *UNUSED(info))
 {
 /* Initialize the Minix file server. */
   int i;
diff --git a/servers/ext2/path.c b/servers/ext2/path.c
index 5b063e247..22410ab42 100644
--- a/servers/ext2/path.c
+++ b/servers/ext2/path.c
@@ -469,7 +469,7 @@ char string[NAME_MAX+1];	/* component extracted from 'old_name' */
 
   /* Special case of the string at cp is empty */
   if (len == 0)
-	strcpy(string, ".");  /* Return "." */
+	strlcpy(string, ".", NAME_MAX + 1);  /* Return "." */
   else {
 	memcpy(string, cp, len);
 	string[len]= '\0';
diff --git a/servers/ext2/proto.h b/servers/ext2/proto.h
index 0c3e87eae..54a75c43e 100644
--- a/servers/ext2/proto.h
+++ b/servers/ext2/proto.h
@@ -114,7 +114,7 @@ void sanitycheck(char *file, int line);
 int ansi_strcmp(register const char* ansi_s, register const char *s2,
 	register size_t ansi_s_length);
 bit_t setbit(bitchunk_t *bitmap, bit_t max_bits, unsigned int word);
-bit_t setbyte(bitchunk_t *bitmap, bit_t max_bits, unsigned int word);
+bit_t setbyte(bitchunk_t *bitmap, bit_t max_bits);
 int unsetbit(bitchunk_t *bitmap, bit_t bit);
 
 /* write.c */
diff --git a/servers/ext2/read.c b/servers/ext2/read.c
index 587239008..de812e680 100644
--- a/servers/ext2/read.c
+++ b/servers/ext2/read.c
@@ -22,8 +22,6 @@ static int rw_chunk(struct inode *rip, u64_t position, unsigned off,
 	size_t chunk, unsigned left, int rw_flag, cp_grant_id_t gid, unsigned
 	buf_off, unsigned int block_size, int *completed);
 
-static char getdents_buf[GETDENTS_BUFSIZ];
-
 static off_t rdahedpos;         /* position to read ahead */
 static struct inode *rdahed_inode;      /* pointer to inode to read ahead */
 
@@ -433,13 +431,19 @@ unsigned bytes_ahead;           /* bytes beyond position for immediate use */
   dev_t dev;
   struct buf *bp = NULL;
   static unsigned int readqsize = 0;
-  static struct buf **read_q;
+  static struct buf **read_q = NULL;
 
   if(readqsize != nr_bufs) {
 	if(readqsize > 0) {
 		assert(read_q != NULL);
 		free(read_q);
-	}
+		read_q = NULL;
+		readqsize = 0;
+	} 
+
+	assert(readqsize == 0);
+	assert(read_q == NULL);
+
 	if(!(read_q = malloc(sizeof(read_q[0])*nr_bufs)))
 		panic("couldn't allocate read_q");
 	readqsize = nr_bufs;
@@ -540,7 +544,10 @@ unsigned bytes_ahead;           /* bytes beyond position for immediate use */
  *===========================================================================*/
 int fs_getdents(void)
 {
-  register struct inode *rip;
+#define GETDENTS_BUFSIZE (sizeof(struct dirent) + EXT2_NAME_MAX + 1)
+#define GETDENTS_ENTRIES	8
+  static char getdents_buf[GETDENTS_BUFSIZE * GETDENTS_ENTRIES];
+  struct inode *rip;
   int o, r, done;
   unsigned int block_size, len, reclen;
   ino_t ino;
@@ -569,7 +576,7 @@ int fs_getdents(void)
   block_pos = pos - off;
   done = FALSE;       /* Stop processing directory blocks when done is set */
 
-  memset(getdents_buf, '\0', GETDENTS_BUFSIZ);  /* Avoid leaking any data */
+  memset(getdents_buf, '\0', sizeof(getdents_buf));  /* Avoid leaking any data */
   tmpbuf_off = 0;       /* Offset in getdents_buf */
   userbuf_off = 0;      /* Offset in the user's buffer */
 
@@ -582,9 +589,6 @@ int fs_getdents(void)
 	b = read_map(rip, block_pos); /* get block number */
 	/* Since directories don't have holes, 'b' cannot be NO_BLOCK. */
 	bp = get_block(rip->i_dev, b, NORMAL);  /* get a dir block */
-
-	if (bp == NO_BLOCK)
-		panic("get_block returned NO_BLOCK");
 	assert(bp != NULL);
 
 	/* Search a directory block. */
@@ -620,20 +624,7 @@ int fs_getdents(void)
 		/* Need the position of this entry in the directory */
 		ent_pos = block_pos + ((char *)d_desc - bp->b_data);
 
-		if (tmpbuf_off + reclen > GETDENTS_BUFSIZ) {
-		r = sys_safecopyto(VFS_PROC_NR, gid,
-				   (vir_bytes) userbuf_off,
-				   (vir_bytes) getdents_buf,
-				   (size_t) tmpbuf_off);
-		if (r != OK) {
-			put_inode(rip);
-			return(r);
-		}
-		userbuf_off += tmpbuf_off;
-		tmpbuf_off = 0;
-		}
-
-		if (userbuf_off + tmpbuf_off + reclen > size) {
+		if (userbuf_off + tmpbuf_off + reclen >= size) {
 			/* The user has no space for one more record */
 			done = TRUE;
 
@@ -645,6 +636,19 @@ int fs_getdents(void)
 			break;
 		}
 
+		if (tmpbuf_off + reclen >= GETDENTS_BUFSIZE*GETDENTS_ENTRIES) {
+			r = sys_safecopyto(VFS_PROC_NR, gid,
+					   (vir_bytes) userbuf_off,
+					   (vir_bytes) getdents_buf,
+					   (size_t) tmpbuf_off);
+			if (r != OK) {
+				put_inode(rip);
+				return(r);
+			}
+			userbuf_off += tmpbuf_off;
+			tmpbuf_off = 0;
+		}
+
 		dep = (struct dirent *) &getdents_buf[tmpbuf_off];
 		dep->d_ino = conv4(le_CPU, d_desc->d_ino);
 		dep->d_off = ent_pos;
diff --git a/servers/ext2/super.c b/servers/ext2/super.c
index 009b73510..317a55a04 100644
--- a/servers/ext2/super.c
+++ b/servers/ext2/super.c
@@ -88,7 +88,7 @@ register struct super_block *sp; /* pointer to a superblock */
 
   STATICINIT(ondisk_superblock, 1);
 
-  if (!sp || !ondisk_superblock)
+  if (!ondisk_superblock)
 	panic("can't allocate memory for super_block buffers");
 
   assert(_MIN_BLOCK_SIZE <= sizeof(*ondisk_superblock));
diff --git a/servers/ext2/utility.c b/servers/ext2/utility.c
index f03785798..776bd152f 100644
--- a/servers/ext2/utility.c
+++ b/servers/ext2/utility.c
@@ -192,7 +192,7 @@ bit_t setbit(bitchunk_t *bitmap, bit_t max_bits, unsigned int word)
 /*===========================================================================*
  *				setbyte   				     *
  *===========================================================================*/
-bit_t setbyte(bitchunk_t *bitmap, bit_t max_bits, unsigned int word)
+bit_t setbyte(bitchunk_t *bitmap, bit_t max_bits)
 {
   /* Find free byte in bitmap and set it. Return number of the starting bit,
    * if failed return -1.
diff --git a/servers/ext2/write.c b/servers/ext2/write.c
index 689ae0086..020282afd 100644
--- a/servers/ext2/write.c
+++ b/servers/ext2/write.c
@@ -41,7 +41,7 @@ int op;				/* special actions */
   long excess, block_pos;
   char new_ind = 0, new_dbl = 0, new_triple = 0;
   int single = 0, triple = 0;
-  register block_t old_block, b1, b2, b3;
+  block_t old_block = NO_BLOCK, b1 = NO_BLOCK, b2 = NO_BLOCK, b3 = NO_BLOCK;
   struct buf *bp = NULL,
              *bp_dindir = NULL,
              *bp_tindir = NULL;