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

diff --git a/servers/ext2/Makefile b/servers/ext2/Makefile
index 31aa4618626aa08a66eadfcff39b6ba4d99c124e..090e7cd70db30d6f39c1820842678ef89b5988d8 100644 (file)
--- 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 50c43ac5666af7a3ddbcefdb6223c47f391d892f..1a93f54076b5f6440f5ee0fdecb66fb6fca17206 100644 (file)
--- 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 919b3f56ab4756a53b6c9d5becf8314923364049..149267e3200d35610dc04e656f51373ccf2781b5 100644 (file)
--- 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 03b39f2acecc399ee053a61dfdb7c6e7858c19c4..1faca7c529468c307138f5e3ba19b8859535c29f 100644 (file)
--- 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 b593306b0b74c599d9aefc779e60b2747bf16526..88ac687ea0e08f89fdd8077529ec5122cd4bb6b4 100644 (file)
--- 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 9c330ef9c90e4a568b44e80f50534c213390e5d8..09948a8b5b270b0bd68897b98155d37fa70d50ee 100644 (file)
--- 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 5b063e2477b4de6f89c88a16755f2869ab4b7beb..22410ab42839c5615715fa22086acecc8588c17f 100644 (file)
--- 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 0c3e87eaec2716350c92e14fa25c70d2662e817f..54a75c43e6a1325d7c90ffc0a94ce76a19a85225 100644 (file)
--- 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 587239008ffcaf2b4aca115b97fe627b48d81785..de812e6805041bf05ad7e094bcec9c3bf94a89ad 100644 (file)
--- 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 009b7351039ac3d47d21de2714c4df2b560aafe4..317a55a04ffae4f3737f97784790fda077711e24 100644 (file)
--- 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 f037857989de5c2a3871a38111a18cd402253b40..776bd152f171cf824aa5c4a6785988c237534af4 100644 (file)
--- 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 689ae00866f2b785b6e4dc2537a176adbe572b98..020282afd22d318b829f03792e4382e6bcd3ecf8 100644 (file)
--- 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;