Skip to content

Commit

Permalink
debugfs: fix set_field's handling of timestamps
Browse files Browse the repository at this point in the history 
How timestamps are encoded in inodes and superblocks are different.
Unfortunately, commit ca8bc92 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: ca8bc92 ("Add post-2038 timestamp support to e2fsprogs")
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
  • Loading branch information
@tytso
tytso committed Apr 18, 2024
1 parent 5182bd6 commit c30260e
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 36 deletions.
66 changes: 49 additions & 17 deletions debugfs/set_fields.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -99,16 +101,16 @@ 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 */
{ "state", &set_sb.s_state, NULL, 2, parse_uint },
{ "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 },
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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 },
Expand All @@ -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 },
Expand Down Expand Up @@ -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 },
Expand Down Expand Up @@ -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);

Expand All @@ -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)
{
Expand Down Expand Up @@ -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";
Expand Down
45 changes: 29 additions & 16 deletions lib/ext2fs/tst_bitmaps.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)))
{
Expand All @@ -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)))
{
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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]))
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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]))
Expand Down
6 changes: 3 additions & 3 deletions lib/ss/test_ss.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: ");
Expand Down

0 comments on commit c30260e

Please sign in to comment.