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
This commit is contained in:
Thomas Veerman 2012-07-26 15:19:08 +00:00
parent 238a9a057b
commit 6c597561bc
12 changed files with 48 additions and 43 deletions

View file

@ -11,5 +11,6 @@ LDADD+= -lminixfs -lbdev -lsys
MAN= MAN=
BINDIR?= /sbin BINDIR?= /sbin
CFLAGS+= -Wall -Wextra -Werror
.include <minix.service.mk> .include <minix.service.mk>

View file

@ -210,7 +210,7 @@ struct inode *rip; /* used for preallocation */
/* we preallocate bytes only */ /* we preallocate bytes only */
ASSERT(EXT2_PREALLOC_BLOCKS == sizeof(char)*CHAR_BIT); 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) { if (bit != -1) {
block = bit + sp->s_first_data_block + block = bit + sp->s_first_data_block +
group * sp->s_blocks_per_group; group * sp->s_blocks_per_group;

View file

@ -351,7 +351,7 @@ void flushall(
/* Flush all dirty blocks for one device. */ /* Flush all dirty blocks for one device. */
register struct buf *bp; 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; static int unsigned dirtylistsize = 0;
int ndirty; int ndirty;
@ -391,6 +391,7 @@ void rw_scattered(
int j, r; int j, r;
STATICINIT(iovec, NR_IOREQS); STATICINIT(iovec, NR_IOREQS);
assert(bufq != NULL);
/* (Shell) sort buffers on b_blocknr. */ /* (Shell) sort buffers on b_blocknr. */
gap = 1; gap = 1;

View file

@ -108,12 +108,10 @@ void free_inode(
} }
static int find_group_dir(struct super_block *sp, struct inode static int find_group_dir(struct super_block *sp);
*parent);
static int find_group_hashalloc(struct super_block *sp, struct inode static int find_group_hashalloc(struct super_block *sp, struct inode
*parent); *parent);
static int find_group_any(struct super_block *sp, struct inode static int find_group_any(struct super_block *sp);
*parent);
static int find_group_orlov(struct super_block *sp, struct inode static int find_group_orlov(struct super_block *sp, struct inode
*parent); *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."); panic("can't alloc inode on read-only filesys.");
if (opt.mfsalloc) { if (opt.mfsalloc) {
group = find_group_any(sp, parent); group = find_group_any(sp);
} else { } else {
if (is_dir) { if (is_dir) {
if (opt.use_orlov) { if (opt.use_orlov) {
group = find_group_orlov(sp, parent); group = find_group_orlov(sp, parent);
} else { } else {
group = find_group_dir(sp, parent); group = find_group_dir(sp);
} }
} else { } else {
group = find_group_hashalloc(sp, parent); 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() */ /* 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; int avefreei = sp->s_free_inodes_count / sp->s_groups_count;
struct group_desc *gd, *best_gd = NULL; 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. /* Find first group which has free inode slot.
* This is similar to what MFS does. * 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; int ngroups = sp->s_groups_count;
struct group_desc *gd; struct group_desc *gd;

View file

@ -200,11 +200,12 @@ int fs_rdlink()
r = EIO; r = EIO;
} else { } else {
bp = get_block(rip->i_dev, b, NORMAL); bp = get_block(rip->i_dev, b, NORMAL);
link_text = bp->b_data; if (bp != NULL) {
if (bp) link_text = bp->b_data;
r = OK; r = OK;
else } else {
r = EIO; r = EIO;
}
} }
} else { } else {
/* fast symlink, stored in inode */ /* fast symlink, stored in inode */

View file

@ -32,7 +32,7 @@ static struct optset optset_table[] = {
{ "reserved", OPT_BOOL, &opt.use_reserved_blocks, TRUE }, { "reserved", OPT_BOOL, &opt.use_reserved_blocks, TRUE },
{ "prealloc", OPT_BOOL, &opt.use_prealloc, TRUE }, { "prealloc", OPT_BOOL, &opt.use_prealloc, TRUE },
{ "noprealloc", OPT_BOOL, &opt.use_prealloc, FALSE }, { "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 * * 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. */ /* Initialize the Minix file server. */
int i; int i;

View file

@ -469,7 +469,7 @@ char string[NAME_MAX+1]; /* component extracted from 'old_name' */
/* Special case of the string at cp is empty */ /* Special case of the string at cp is empty */
if (len == 0) if (len == 0)
strcpy(string, "."); /* Return "." */ strlcpy(string, ".", NAME_MAX + 1); /* Return "." */
else { else {
memcpy(string, cp, len); memcpy(string, cp, len);
string[len]= '\0'; string[len]= '\0';

View file

@ -114,7 +114,7 @@ void sanitycheck(char *file, int line);
int ansi_strcmp(register const char* ansi_s, register const char *s2, int ansi_strcmp(register const char* ansi_s, register const char *s2,
register size_t ansi_s_length); register size_t ansi_s_length);
bit_t setbit(bitchunk_t *bitmap, bit_t max_bits, unsigned int word); 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); int unsetbit(bitchunk_t *bitmap, bit_t bit);
/* write.c */ /* write.c */

View file

@ -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 size_t chunk, unsigned left, int rw_flag, cp_grant_id_t gid, unsigned
buf_off, unsigned int block_size, int *completed); buf_off, unsigned int block_size, int *completed);
static char getdents_buf[GETDENTS_BUFSIZ];
static off_t rdahedpos; /* position to read ahead */ static off_t rdahedpos; /* position to read ahead */
static struct inode *rdahed_inode; /* pointer to inode 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; dev_t dev;
struct buf *bp = NULL; struct buf *bp = NULL;
static unsigned int readqsize = 0; static unsigned int readqsize = 0;
static struct buf **read_q; static struct buf **read_q = NULL;
if(readqsize != nr_bufs) { if(readqsize != nr_bufs) {
if(readqsize > 0) { if(readqsize > 0) {
assert(read_q != NULL); assert(read_q != NULL);
free(read_q); 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))) if(!(read_q = malloc(sizeof(read_q[0])*nr_bufs)))
panic("couldn't allocate read_q"); panic("couldn't allocate read_q");
readqsize = nr_bufs; readqsize = nr_bufs;
@ -540,7 +544,10 @@ unsigned bytes_ahead; /* bytes beyond position for immediate use */
*===========================================================================*/ *===========================================================================*/
int fs_getdents(void) 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; int o, r, done;
unsigned int block_size, len, reclen; unsigned int block_size, len, reclen;
ino_t ino; ino_t ino;
@ -569,7 +576,7 @@ int fs_getdents(void)
block_pos = pos - off; block_pos = pos - off;
done = FALSE; /* Stop processing directory blocks when done is set */ 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 */ tmpbuf_off = 0; /* Offset in getdents_buf */
userbuf_off = 0; /* Offset in the user's buffer */ 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 */ b = read_map(rip, block_pos); /* get block number */
/* Since directories don't have holes, 'b' cannot be NO_BLOCK. */ /* Since directories don't have holes, 'b' cannot be NO_BLOCK. */
bp = get_block(rip->i_dev, b, NORMAL); /* get a dir 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); assert(bp != NULL);
/* Search a directory block. */ /* Search a directory block. */
@ -620,20 +624,7 @@ int fs_getdents(void)
/* Need the position of this entry in the directory */ /* Need the position of this entry in the directory */
ent_pos = block_pos + ((char *)d_desc - bp->b_data); ent_pos = block_pos + ((char *)d_desc - bp->b_data);
if (tmpbuf_off + reclen > GETDENTS_BUFSIZ) { if (userbuf_off + tmpbuf_off + reclen >= size) {
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) {
/* The user has no space for one more record */ /* The user has no space for one more record */
done = TRUE; done = TRUE;
@ -645,6 +636,19 @@ int fs_getdents(void)
break; 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 = (struct dirent *) &getdents_buf[tmpbuf_off];
dep->d_ino = conv4(le_CPU, d_desc->d_ino); dep->d_ino = conv4(le_CPU, d_desc->d_ino);
dep->d_off = ent_pos; dep->d_off = ent_pos;

View file

@ -88,7 +88,7 @@ register struct super_block *sp; /* pointer to a superblock */
STATICINIT(ondisk_superblock, 1); STATICINIT(ondisk_superblock, 1);
if (!sp || !ondisk_superblock) if (!ondisk_superblock)
panic("can't allocate memory for super_block buffers"); panic("can't allocate memory for super_block buffers");
assert(_MIN_BLOCK_SIZE <= sizeof(*ondisk_superblock)); assert(_MIN_BLOCK_SIZE <= sizeof(*ondisk_superblock));

View file

@ -192,7 +192,7 @@ bit_t setbit(bitchunk_t *bitmap, bit_t max_bits, unsigned int word)
/*===========================================================================* /*===========================================================================*
* setbyte * * 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, /* Find free byte in bitmap and set it. Return number of the starting bit,
* if failed return -1. * if failed return -1.

View file

@ -41,7 +41,7 @@ int op; /* special actions */
long excess, block_pos; long excess, block_pos;
char new_ind = 0, new_dbl = 0, new_triple = 0; char new_ind = 0, new_dbl = 0, new_triple = 0;
int single = 0, 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, struct buf *bp = NULL,
*bp_dindir = NULL, *bp_dindir = NULL,
*bp_tindir = NULL; *bp_tindir = NULL;