From c30260e0eb959344d911652fa7630721848a3549 Mon Sep 17 00:00:00 2001 From: Theodore Ts'o Date: Thu, 18 Apr 2024 10:04:27 -0400 Subject: [PATCH] debugfs: fix set_field's handling of timestamps How timestamps are encoded in inodes and superblocks are different. Unfortunately, commit ca8bc9240a00 which added post-2038 timestamps was (a) overwriting adjacent superblock fields and/or attempting unaligned writes to a 8-bit field from a 32-bit pointer, and (b) using the incorrect encoding for timestamps stored in inodes. Fix both of these issues, which were found thanks to UBSAN. Fixes: ca8bc9240a00 ("Add post-2038 timestamp support to e2fsprogs") Signed-off-by: Theodore Ts'o --- debugfs/set_fields.c | 66 +++++++++++++++++++++++++++++----------- lib/ext2fs/tst_bitmaps.c | 45 +++++++++++++++++---------- lib/ss/test_ss.c | 6 ++-- 3 files changed, 81 insertions(+), 36 deletions(-) diff --git a/debugfs/set_fields.c b/debugfs/set_fields.c index 6755f8802..4980a63d8 100644 --- a/debugfs/set_fields.c +++ b/debugfs/set_fields.c @@ -71,7 +71,9 @@ static errcode_t parse_string(struct field_set_info *info, char *field, char *ar static errcode_t parse_uuid(struct field_set_info *info, char *field, char *arg); static errcode_t parse_hashalg(struct field_set_info *info, char *field, char *arg); static errcode_t parse_encoding(struct field_set_info *info, char *field, char *arg); -static errcode_t parse_time(struct field_set_info *info, char *field, char *arg); +static errcode_t parse_sb_time(struct field_set_info *info, char *field, char *arg); +static errcode_t parse_ino_time(struct field_set_info *info, char *field, char *arg); + static errcode_t parse_bmap(struct field_set_info *info, char *field, char *arg); static errcode_t parse_gd_csum(struct field_set_info *info, char *field, char *arg); static errcode_t parse_inode_csum(struct field_set_info *info, char *field, @@ -99,8 +101,8 @@ static struct field_set_info super_fields[] = { { "blocks_per_group", &set_sb.s_blocks_per_group, NULL, 4, parse_uint }, { "clusters_per_group", &set_sb.s_clusters_per_group, NULL, 4, parse_uint }, { "inodes_per_group", &set_sb.s_inodes_per_group, NULL, 4, parse_uint }, - { "mtime", &set_sb.s_mtime, &set_sb.s_mtime_hi, 5, parse_time }, - { "wtime", &set_sb.s_wtime, &set_sb.s_wtime_hi, 5, parse_time }, + { "mtime", &set_sb.s_mtime, &set_sb.s_mtime_hi, 5, parse_sb_time }, + { "wtime", &set_sb.s_wtime, &set_sb.s_wtime_hi, 5, parse_sb_time }, { "mnt_count", &set_sb.s_mnt_count, NULL, 2, parse_uint }, { "max_mnt_count", &set_sb.s_max_mnt_count, NULL, 2, parse_int }, /* s_magic */ @@ -108,7 +110,7 @@ static struct field_set_info super_fields[] = { { "errors", &set_sb.s_errors, NULL, 2, parse_uint }, { "minor_rev_level", &set_sb.s_minor_rev_level, NULL, 2, parse_uint }, { "lastcheck", &set_sb.s_lastcheck, &set_sb.s_lastcheck_hi, 5, - parse_time }, + parse_sb_time }, { "checkinterval", &set_sb.s_checkinterval, NULL, 4, parse_uint }, { "creator_os", &set_sb.s_creator_os, NULL, 4, parse_uint }, { "rev_level", &set_sb.s_rev_level, NULL, 4, parse_uint }, @@ -141,7 +143,7 @@ static struct field_set_info super_fields[] = { { "default_mount_opts", &set_sb.s_default_mount_opts, NULL, 4, parse_uint }, { "first_meta_bg", &set_sb.s_first_meta_bg, NULL, 4, parse_uint }, { "mkfs_time", &set_sb.s_mkfs_time, &set_sb.s_mkfs_time_hi, 5, - parse_time }, + parse_sb_time }, { "jnl_blocks", &set_sb.s_jnl_blocks[0], NULL, 4, parse_uint, FLAG_ARRAY, 17 }, { "min_extra_isize", &set_sb.s_min_extra_isize, NULL, 2, parse_uint }, @@ -170,13 +172,13 @@ static struct field_set_info super_fields[] = { { "encryption_level", &set_sb.s_encryption_level, NULL, 1, parse_uint }, { "error_count", &set_sb.s_error_count, NULL, 4, parse_uint }, { "first_error_time", &set_sb.s_first_error_time, - &set_sb.s_first_error_time_hi, 5, parse_time }, + &set_sb.s_first_error_time_hi, 5, parse_sb_time }, { "first_error_ino", &set_sb.s_first_error_ino, NULL, 4, parse_uint }, { "first_error_block", &set_sb.s_first_error_block, NULL, 8, parse_uint }, { "first_error_func", &set_sb.s_first_error_func, NULL, 32, parse_string }, { "first_error_line", &set_sb.s_first_error_line, NULL, 4, parse_uint }, { "last_error_time", &set_sb.s_last_error_time, - &set_sb.s_last_error_time_hi, 5, parse_time }, + &set_sb.s_last_error_time_hi, 5, parse_sb_time }, { "last_error_ino", &set_sb.s_last_error_ino, NULL, 4, parse_uint }, { "last_error_block", &set_sb.s_last_error_block, NULL, 8, parse_uint }, { "last_error_func", &set_sb.s_last_error_func, NULL, 32, parse_string }, @@ -197,13 +199,13 @@ static struct field_set_info inode_fields[] = { 2, parse_uint }, { "size", &set_inode.i_size, &set_inode.i_size_high, 4, parse_uint }, { "atime", &set_inode.i_atime, &set_inode.i_atime_extra, - 4, parse_time }, + 4, parse_ino_time }, { "ctime", &set_inode.i_ctime, &set_inode.i_ctime_extra, - 4, parse_time }, + 4, parse_ino_time }, { "mtime", &set_inode.i_mtime, &set_inode.i_mtime_extra, - 4, parse_time }, + 4, parse_ino_time }, { "dtime", &set_inode.i_dtime, NULL, - 4, parse_time }, + 4, parse_ino_time }, { "gid", &set_inode.i_gid, &set_inode.osd2.linux2.l_i_gid_high, 2, parse_uint }, { "links_count", &set_inode.i_links_count, NULL, 2, parse_uint }, @@ -240,7 +242,7 @@ static struct field_set_info inode_fields[] = { { "atime_extra", &set_inode.i_atime_extra, NULL, 4, parse_uint, FLAG_ALIAS }, { "crtime", &set_inode.i_crtime, &set_inode.i_crtime_extra, - 4, parse_time }, + 4, parse_ino_time }, { "crtime_extra", &set_inode.i_crtime_extra, NULL, 4, parse_uint, FLAG_ALIAS }, { "projid", &set_inode.i_projid, NULL, 4, parse_uint }, @@ -581,18 +583,19 @@ static errcode_t parse_string(struct field_set_info *info, return 0; } -static errcode_t parse_time(struct field_set_info *info, - char *field, char *arg) +static errcode_t parse_sb_time(struct field_set_info *info, + char *field, char *arg) { __s64 t; __u32 t_low, t_high; - __u32 *ptr_low, *ptr_high; + __u32 *ptr_low; + __u8 *ptr_high; if (check_suffix(field)) return parse_uint(info, field, arg); ptr_low = (__u32 *) info->ptr; - ptr_high = (__u32 *) info->ptr2; + ptr_high = (__u8 *) info->ptr2; t = string_to_time(arg); @@ -609,6 +612,34 @@ static errcode_t parse_time(struct field_set_info *info, return 0; } +static errcode_t parse_ino_time(struct field_set_info *info, + char *field, char *arg) +{ + __s64 t; + __u32 t_low, t_high; + __u32 *ptr_low, *ptr_high; + + if (check_suffix(field)) + return parse_uint(info, field, arg); + + ptr_low = (__u32 *) info->ptr; + ptr_high = (__u32 *) info->ptr2; + + t = string_to_time(arg); + + if (t == -1) { + fprintf(stderr, "Couldn't parse '%s' for field %s.\n", + arg, info->name); + return EINVAL; + } + t_low = (__u32) t; + t_high = __encode_extra_time(t, 0); + *ptr_low = t_low; + if (ptr_high) + *ptr_high = t_high; + return 0; +} + static errcode_t parse_uuid(struct field_set_info *info, char *field EXT2FS_ATTR((unused)), char *arg) { @@ -790,7 +821,8 @@ static void print_possible_fields(struct field_set_info *fields) type = "UUID"; else if (ss->func == parse_hashalg) type = "hash algorithm"; - else if (ss->func == parse_time) + else if ((ss->func == parse_sb_time) || + (ss->func == parse_ino_time)) type = "date/time"; else if (ss->func == parse_bmap) type = "set physical->logical block map"; diff --git a/lib/ext2fs/tst_bitmaps.c b/lib/ext2fs/tst_bitmaps.c index cb3c70dcd..ec4c91331 100644 --- a/lib/ext2fs/tst_bitmaps.c +++ b/lib/ext2fs/tst_bitmaps.c @@ -140,7 +140,7 @@ unsigned long parse_ulong(const char *str, const char *cmd, } -int check_fs_open(char *name) +int check_fs_open(const char *name) { if (!test_fs) { com_err(name, 0, "Filesystem not open"); @@ -190,7 +190,8 @@ static void setup_filesystem(const char *name, ext2fs_close_free(&test_fs); } -void setup_cmd(int argc, char **argv, int sci_idx EXT2FS_ATTR((unused)), +void setup_cmd(int argc, const char * const *argv, + int sci_idx EXT2FS_ATTR((unused)), void *infop EXT2FS_ATTR((unused))) { int c, err; @@ -271,7 +272,7 @@ void dump_bitmap(ext2fs_generic_bitmap bmap, unsigned int start, unsigned num) free(buf); } -void dump_inode_bitmap_cmd(int argc, char **argv, +void dump_inode_bitmap_cmd(int argc, const char * const *argv, int sci_idx EXT2FS_ATTR((unused)), void *infop EXT2FS_ATTR((unused))) { @@ -282,7 +283,7 @@ void dump_inode_bitmap_cmd(int argc, char **argv, dump_bitmap(test_fs->inode_map, 1, test_fs->super->s_inodes_count); } -void dump_block_bitmap_cmd(int argc, char **argv, +void dump_block_bitmap_cmd(int argc, const char * const *argv, int sci_idx EXT2FS_ATTR((unused)), void *infop EXT2FS_ATTR((unused))) { @@ -294,7 +295,8 @@ void dump_block_bitmap_cmd(int argc, char **argv, test_fs->super->s_blocks_count); } -void do_setb(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), +void do_setb(int argc, const char * const *argv, + int sci_idx EXT2FS_ATTR((unused)), void *infop EXT2FS_ATTR((unused))) { unsigned int block, num; @@ -333,7 +335,8 @@ void do_setb(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), test_result, op_result); } -void do_clearb(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), +void do_clearb(int argc, const char * const *argv, + int sci_idx EXT2FS_ATTR((unused)), void *infop EXT2FS_ATTR((unused))) { unsigned int block, num; @@ -372,7 +375,8 @@ void do_clearb(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), test_result, op_result); } -void do_testb(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), +void do_testb(int argc, const char * const *argv, + int sci_idx EXT2FS_ATTR((unused)), void *infop EXT2FS_ATTR((unused))) { unsigned int block, num; @@ -408,7 +412,8 @@ void do_testb(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), printf("Block %u is %s\n", block, test_result ? "set" : "clear"); } -void do_ffzb(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), +void do_ffzb(int argc, const char * const *argv, + int sci_idx EXT2FS_ATTR((unused)), void *infop EXT2FS_ATTR((unused))) { unsigned int start, end; @@ -442,7 +447,8 @@ void do_ffzb(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), printf("First unmarked block is %llu\n", (unsigned long long) out); } -void do_ffsb(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), +void do_ffsb(int argc, const char * const *argv, + int sci_idx EXT2FS_ATTR((unused)), void *infop EXT2FS_ATTR((unused))) { unsigned int start, end; @@ -477,7 +483,8 @@ void do_ffsb(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), } -void do_zerob(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), +void do_zerob(int argc, const char * const *argv, + int sci_idx EXT2FS_ATTR((unused)), void *infop EXT2FS_ATTR((unused))) { if (check_fs_open(argv[0])) @@ -487,7 +494,8 @@ void do_zerob(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), ext2fs_clear_block_bitmap(test_fs->block_map); } -void do_seti(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), +void do_seti(int argc, const char * const *argv, + int sci_idx EXT2FS_ATTR((unused)), void *infop EXT2FS_ATTR((unused))) { unsigned int inode; @@ -517,7 +525,8 @@ void do_seti(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), } } -void do_cleari(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), +void do_cleari(int argc, const char * const *argv, + int sci_idx EXT2FS_ATTR((unused)), void *infop EXT2FS_ATTR((unused))) { unsigned int inode; @@ -547,7 +556,8 @@ void do_cleari(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), } } -void do_testi(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), +void do_testi(int argc, const char * const *argv, + int sci_idx EXT2FS_ATTR((unused)), void *infop EXT2FS_ATTR((unused))) { unsigned int inode; @@ -570,7 +580,8 @@ void do_testi(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), printf("Inode %u is %s\n", inode, test_result ? "set" : "clear"); } -void do_ffzi(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), +void do_ffzi(int argc, const char * const *argv, + int sci_idx EXT2FS_ATTR((unused)), void *infop EXT2FS_ATTR((unused))) { unsigned int start, end; @@ -604,7 +615,8 @@ void do_ffzi(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), printf("First unmarked inode is %u\n", out); } -void do_ffsi(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), +void do_ffsi(int argc, const char * const *argv, + int sci_idx EXT2FS_ATTR((unused)), void *infop EXT2FS_ATTR((unused))) { unsigned int start, end; @@ -638,7 +650,8 @@ void do_ffsi(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), printf("First marked inode is %u\n", out); } -void do_zeroi(int argc, char *argv[], int sci_idx EXT2FS_ATTR((unused)), +void do_zeroi(int argc, const char * const *argv, + int sci_idx EXT2FS_ATTR((unused)), void *infop EXT2FS_ATTR((unused))) { if (check_fs_open(argv[0])) diff --git a/lib/ss/test_ss.c b/lib/ss/test_ss.c index 53ca99fc4..a382673f5 100644 --- a/lib/ss/test_ss.c +++ b/lib/ss/test_ss.c @@ -136,9 +136,9 @@ int main(int argc, char **argv) } -void test_cmd (argc, argv) - int argc; - char **argv; +void test_cmd(int argc, const char * const *argv, + int sci_idx __SS_ATTR((unused)), + void *infop __SS_ATTR((unused))) { printf("Hello, world!\n"); printf("Args: ");