From: Jeff Mahoney <jeffm@xxxxxxxx> reiserfs uses sprintf to print error messages and never does any bounds checking. This patch uses snprintf everywhere, does proper length tracking, and gets rid of a few copies to static buffers along the way. We also increase the size of the error buffer to 2k. Reported-by: syzbot+b890b3335a4d8c608963@xxxxxxxxxxxxxxxxxxxxxxxxx Signed-off-by: Jeff Mahoney <jeffm@xxxxxxxx> --- fs/reiserfs/item_ops.c | 17 +--- fs/reiserfs/prints.c | 259 +++++++++++++++++++++++++++++++------------------ 2 files changed, 166 insertions(+), 110 deletions(-) diff --git a/fs/reiserfs/item_ops.c b/fs/reiserfs/item_ops.c index e3c558d1b78c..3f3c96352921 100644 --- a/fs/reiserfs/item_ops.c +++ b/fs/reiserfs/item_ops.c @@ -33,30 +33,21 @@ static int sd_is_left_mergeable(struct reiserfs_key *key, unsigned long bsize) return 0; } -static char *print_time(time_t t) -{ - static char timebuf[256]; - - sprintf(timebuf, "%ld", t); - return timebuf; -} - static void sd_print_item(struct item_head *ih, char *item) { printk("\tmode | size | nlinks | first direct | mtime\n"); if (stat_data_v1(ih)) { struct stat_data_v1 *sd = (struct stat_data_v1 *)item; - printk("\t0%-6o | %6u | %2u | %d | %s\n", sd_v1_mode(sd), + printk("\t0%-6o | %6u | %2u | %d | %u\n", sd_v1_mode(sd), sd_v1_size(sd), sd_v1_nlink(sd), - sd_v1_first_direct_byte(sd), - print_time(sd_v1_mtime(sd))); + sd_v1_first_direct_byte(sd), sd_v1_mtime(sd)); } else { struct stat_data *sd = (struct stat_data *)item; - printk("\t0%-6o | %6llu | %2u | %d | %s\n", sd_v2_mode(sd), + printk("\t0%-6o | %6llu | %2u | %d | %u\n", sd_v2_mode(sd), (unsigned long long)sd_v2_size(sd), sd_v2_nlink(sd), - sd_v2_rdev(sd), print_time(sd_v2_mtime(sd))); + sd_v2_rdev(sd), sd_v2_mtime(sd)); } } diff --git a/fs/reiserfs/prints.c b/fs/reiserfs/prints.c index 7e288d97adcb..e5e66695f17f 100644 --- a/fs/reiserfs/prints.c +++ b/fs/reiserfs/prints.c @@ -10,39 +10,35 @@ #include <stdarg.h> -static char error_buf[1024]; +static char error_buf[2048]; static char fmt_buf[1024]; -static char off_buf[80]; -static char *reiserfs_cpu_offset(struct cpu_key *key) +static size_t snprintf_cpu_offset(char *buf, size_t size, struct cpu_key *key) { + u64 offset = cpu_key_k_offset(key); + if (cpu_key_k_type(key) == TYPE_DIRENTRY) - sprintf(off_buf, "%llu(%llu)", - (unsigned long long) - GET_HASH_VALUE(cpu_key_k_offset(key)), - (unsigned long long) - GET_GENERATION_NUMBER(cpu_key_k_offset(key))); + return snprintf(buf, size, "%llu(%llu)", + (unsigned long long)GET_HASH_VALUE(offset), + (unsigned long long)GET_GENERATION_NUMBER(offset)); else - sprintf(off_buf, "0x%Lx", - (unsigned long long)cpu_key_k_offset(key)); - return off_buf; + return snprintf(buf, size, "0x%Lx", + (unsigned long long)offset); } -static char *le_offset(struct reiserfs_key *key) +static size_t snprintf_le_offset(char *buf, size_t size, + struct reiserfs_key *key) { - int version; + int version = le_key_version(key); + u64 offset = le_key_k_offset(version, key); - version = le_key_version(key); if (le_key_k_type(version, key) == TYPE_DIRENTRY) - sprintf(off_buf, "%llu(%llu)", - (unsigned long long) - GET_HASH_VALUE(le_key_k_offset(version, key)), - (unsigned long long) - GET_GENERATION_NUMBER(le_key_k_offset(version, key))); + return snprintf(buf, size, "%llu(%llu)", + (unsigned long long)GET_HASH_VALUE(offset), + (unsigned long long)GET_GENERATION_NUMBER(offset)); else - sprintf(off_buf, "0x%Lx", - (unsigned long long)le_key_k_offset(version, key)); - return off_buf; + return snprintf(buf, size, "0x%Lx", + (unsigned long long)offset); } static char *cpu_type(struct cpu_key *key) @@ -76,83 +72,130 @@ static char *le_type(struct reiserfs_key *key) } /* %k */ -static void sprintf_le_key(char *buf, struct reiserfs_key *key) +static size_t snprintf_le_key(char *buf, size_t size, struct reiserfs_key *key) { - if (key) - sprintf(buf, "[%d %d %s %s]", le32_to_cpu(key->k_dir_id), - le32_to_cpu(key->k_objectid), le_offset(key), - le_type(key)); - else - sprintf(buf, "[NULL]"); + size_t len; + char *out = buf; + + if (!key) + return snprintf(buf, size, "[NULL]"); + + len = snprintf(out, size, "[%d %d ", le32_to_cpu(key->k_dir_id), + le32_to_cpu(key->k_objectid)); + size -= len; + out += len; + + len = snprintf_le_offset(out, size, key); + size -= len; + out += len; + + len = snprintf(out, size, " %s]", le_type(key)); + size -= len; + out += len; + + return out - buf; } /* %K */ -static void sprintf_cpu_key(char *buf, struct cpu_key *key) +static size_t snprintf_cpu_key(char *buf, size_t size, struct cpu_key *key) { - if (key) - sprintf(buf, "[%d %d %s %s]", key->on_disk_key.k_dir_id, - key->on_disk_key.k_objectid, reiserfs_cpu_offset(key), - cpu_type(key)); - else - sprintf(buf, "[NULL]"); + size_t len; + char *out = buf; + + if (!key) + return snprintf(buf, size, "[NULL]"); + + len = snprintf(out, size, "[%d %d ", key->on_disk_key.k_dir_id, + key->on_disk_key.k_objectid); + size -= len; + out += len; + + len = snprintf_cpu_offset(out, size, key); + size -= len; + out += len; + + len = snprintf(out, size, " %s]", cpu_type(key)); + size -= len; + out += len; + + return out - buf; } -static void sprintf_de_head(char *buf, struct reiserfs_de_head *deh) +static size_t snprintf_de_head(char *buf, size_t size, + struct reiserfs_de_head *deh) { - if (deh) - sprintf(buf, + if (!deh) + return snprintf(buf, size, "[NULL]"); + + return snprintf(buf, size, "[offset=%d dir_id=%d objectid=%d location=%d state=%04x]", deh_offset(deh), deh_dir_id(deh), deh_objectid(deh), deh_location(deh), deh_state(deh)); - else - sprintf(buf, "[NULL]"); - } -static void sprintf_item_head(char *buf, struct item_head *ih) +static size_t snprintf_item_head(char *buf, size_t size, struct item_head *ih) { - if (ih) { - strcpy(buf, - (ih_version(ih) == KEY_FORMAT_3_6) ? "*3.6* " : "*3.5*"); - sprintf_le_key(buf + strlen(buf), &(ih->ih_key)); - sprintf(buf + strlen(buf), ", item_len %d, item_location %d, " - "free_space(entry_count) %d", - ih_item_len(ih), ih_location(ih), ih_free_space(ih)); - } else - sprintf(buf, "[NULL]"); + char *out = buf; + size_t len; + + if (!ih) + return snprintf(buf, size, "[NULL]"); + + len = snprintf(out, size, "*3.%d* ", + ih_version(ih) == KEY_FORMAT_3_6 ? 6 : 5); + size -= len; + out += len; + + len = snprintf_le_key(out, size, &ih->ih_key); + out += len; + size -= len; + + len += snprintf(out, size, + ", item_len %d, item_location %d, free_space(entry_count) %d", + ih_item_len(ih), ih_location(ih), + ih_free_space(ih)); + out += len; + size -= len; + + return out - buf; } -static void sprintf_direntry(char *buf, struct reiserfs_dir_entry *de) +static size_t snprintf_direntry(char *buf, size_t size, + struct reiserfs_dir_entry *de) { char name[20]; memcpy(name, de->de_name, de->de_namelen > 19 ? 19 : de->de_namelen); name[de->de_namelen > 19 ? 19 : de->de_namelen] = 0; - sprintf(buf, "\"%s\"==>[%d %d]", name, de->de_dir_id, de->de_objectid); + return snprintf(buf, size, "\"%s\"==>[%d %d]", name, + de->de_dir_id, de->de_objectid); } -static void sprintf_block_head(char *buf, struct buffer_head *bh) +static int snprintf_block_head(char *buf, size_t size, struct buffer_head *bh) { - sprintf(buf, "level=%d, nr_items=%d, free_space=%d rdkey ", - B_LEVEL(bh), B_NR_ITEMS(bh), B_FREE_SPACE(bh)); + return snprintf(buf, size, + "level=%d, nr_items=%d, free_space=%d rdkey ", + B_LEVEL(bh), B_NR_ITEMS(bh), B_FREE_SPACE(bh)); } -static void sprintf_buffer_head(char *buf, struct buffer_head *bh) +static size_t snprintf_buffer_head(char *buf, size_t size, + struct buffer_head *bh) { - sprintf(buf, - "dev %pg, size %zd, blocknr %llu, count %d, state 0x%lx, page %p, (%s, %s, %s)", - bh->b_bdev, bh->b_size, - (unsigned long long)bh->b_blocknr, atomic_read(&(bh->b_count)), - bh->b_state, bh->b_page, - buffer_uptodate(bh) ? "UPTODATE" : "!UPTODATE", - buffer_dirty(bh) ? "DIRTY" : "CLEAN", - buffer_locked(bh) ? "LOCKED" : "UNLOCKED"); + return snprintf(buf, size, + "dev %pg, size %zd, blocknr %llu, count %d, state 0x%lx, page %p, (%s, %s, %s)", + bh->b_bdev, bh->b_size, + (unsigned long long)bh->b_blocknr, + atomic_read(&(bh->b_count)), + bh->b_state, bh->b_page, + buffer_uptodate(bh) ? "UPTODATE" : "!UPTODATE", + buffer_dirty(bh) ? "DIRTY" : "CLEAN", + buffer_locked(bh) ? "LOCKED" : "UNLOCKED"); } -static void sprintf_disk_child(char *buf, struct disk_child *dc) +static size_t snprintf_disk_child(char *buf, size_t size, struct disk_child *dc) { - sprintf(buf, "[dc_number=%d, dc_size=%u]", dc_block_number(dc), - dc_size(dc)); + return snprintf(buf, size, "[dc_number=%d, dc_size=%u]", + dc_block_number(dc), dc_size(dc)); } static char *is_there_reiserfs_struct(char *fmt, int *what) @@ -190,56 +233,61 @@ static void prepare_error_buf(const char *fmt, va_list args) char *k; char *p = error_buf; int what; + size_t left = sizeof(error_buf); + size_t len; spin_lock(&error_lock); - strcpy(fmt1, fmt); + strncpy(fmt1, fmt, sizeof(fmt_buf)); while ((k = is_there_reiserfs_struct(fmt1, &what)) != NULL) { *k = 0; - p += vsprintf(p, fmt1, args); + len = vsnprintf(p, left, fmt1, args); + p += len; + left -= len; switch (what) { case 'k': - sprintf_le_key(p, va_arg(args, struct reiserfs_key *)); + len = snprintf_le_key(p, left, + va_arg(args, struct reiserfs_key *)); break; case 'K': - sprintf_cpu_key(p, va_arg(args, struct cpu_key *)); + len = snprintf_cpu_key(p, left, + va_arg(args, struct cpu_key *)); break; case 'h': - sprintf_item_head(p, va_arg(args, struct item_head *)); + len = snprintf_item_head(p, left, + va_arg(args, struct item_head *)); break; case 't': - sprintf_direntry(p, - va_arg(args, - struct reiserfs_dir_entry *)); + len = snprintf_direntry(p, left, + va_arg(args, struct reiserfs_dir_entry *)); break; case 'y': - sprintf_disk_child(p, - va_arg(args, struct disk_child *)); + len = snprintf_disk_child(p, left, + va_arg(args, struct disk_child *)); break; case 'z': - sprintf_block_head(p, - va_arg(args, struct buffer_head *)); + len = snprintf_block_head(p, left, + va_arg(args, struct buffer_head *)); break; case 'b': - sprintf_buffer_head(p, - va_arg(args, struct buffer_head *)); + len = snprintf_buffer_head(p, left, + va_arg(args, struct buffer_head *)); break; case 'a': - sprintf_de_head(p, - va_arg(args, - struct reiserfs_de_head *)); + len = snprintf_de_head(p, left, + va_arg(args, struct reiserfs_de_head *)); break; } - p += strlen(p); + p += len; + left -= len; fmt1 = k + 2; } - vsprintf(p, fmt1, args); + vsnprintf(p, left, fmt1, args); spin_unlock(&error_lock); - } /* @@ -621,11 +669,14 @@ void store_print_tb(struct tree_balance *tb) int h = 0; int i; struct buffer_head *tbSh, *tbFh; + size_t left = sizeof(print_tb_buf); + char *out = print_tb_buf; + size_t len; if (!tb) return; - sprintf(print_tb_buf, "\n" + len = snprintf(out, left, "\n" "BALANCING %d\n" "MODE=%c, ITEM_POS=%d POS_IN_ITEM=%d\n" "=====================================================================\n" @@ -634,6 +685,9 @@ void store_print_tb(struct tree_balance *tb) tb->tb_mode, PATH_LAST_POSITION(tb->tb_path), tb->tb_path->pos_in_item); + left -= len; + out += len; + for (h = 0; h < ARRAY_SIZE(tb->insert_size); h++) { if (PATH_H_PATH_OFFSET(tb->tb_path, h) <= tb->tb_path->path_length @@ -645,7 +699,7 @@ void store_print_tb(struct tree_balance *tb) tbSh = NULL; tbFh = NULL; } - sprintf(print_tb_buf + strlen(print_tb_buf), + len = snprintf(out, left, "* %d * %3lld(%2d) * %3lld(%2d) * %3lld(%2d) * %5lld * %5lld * %5lld * %5lld * %5lld *\n", h, (tbSh) ? (long long)(tbSh->b_blocknr) : (-1LL), @@ -663,9 +717,11 @@ void store_print_tb(struct tree_balance *tb) b_blocknr) : (-1LL), (tb->CFR[h]) ? (long long)(tb->CFR[h]-> b_blocknr) : (-1LL)); + left -= len; + out += len; } - sprintf(print_tb_buf + strlen(print_tb_buf), + len = snprintf(out, left, "=====================================================================\n" "* h * size * ln * lb * rn * rb * blkn * s0 * s1 * s1b * s2 * s2b * curb * lk * rk *\n" "* 0 * %4d * %2d * %2d * %2d * %2d * %4d * %2d * %2d * %3d * %2d * %3d * %4d * %2d * %2d *\n", @@ -673,32 +729,41 @@ void store_print_tb(struct tree_balance *tb) tb->rbytes, tb->blknum[0], tb->s0num, tb->snum[0], tb->sbytes[0], tb->snum[1], tb->sbytes[1], tb->cur_blknum, tb->lkey[0], tb->rkey[0]); + left -= len; + out += len; /* this prints balance parameters for non-leaf levels */ h = 0; do { h++; - sprintf(print_tb_buf + strlen(print_tb_buf), + len = snprintf(out, left, "* %d * %4d * %2d * * %2d * * %2d *\n", h, tb->insert_size[h], tb->lnum[h], tb->rnum[h], tb->blknum[h]); + left -= len; + out += len; } while (tb->insert_size[h]); - sprintf(print_tb_buf + strlen(print_tb_buf), + left = snprintf(out, left, "=====================================================================\n" "FEB list: "); + left -= len; + out += len; /* print FEB list (list of buffers in form (bh (b_blocknr, b_count), that will be used for new nodes) */ h = 0; - for (i = 0; i < ARRAY_SIZE(tb->FEB); i++) - sprintf(print_tb_buf + strlen(print_tb_buf), + for (i = 0; i < ARRAY_SIZE(tb->FEB); i++) { + len = snprintf(out, left, "%p (%llu %d)%s", tb->FEB[i], tb->FEB[i] ? (unsigned long long)tb->FEB[i]-> b_blocknr : 0ULL, tb->FEB[i] ? atomic_read(&tb->FEB[i]->b_count) : 0, (i == ARRAY_SIZE(tb->FEB) - 1) ? "\n" : ", "); + left -= len; + out += len; + } - sprintf(print_tb_buf + strlen(print_tb_buf), + snprintf(out, left, "======================== the end ====================================\n"); } -- 2.12.3 -- To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html