[gfs2-utils] 02/03: libgfs2: Avoid potential gfs/gfs2 superblock update clash

This is an automated email from the git hooks/post-receive script. andyp pushed a commit to branch master in repository gfs2-utils. commit 8cea89ce70357634586acd420e8a9142dead6e59 Author: Andrew Price <anprice(a)redhat.com&gt; AuthorDate: Wed Oct 6 15:30:28 2021 +0100 libgfs2: Avoid potential gfs/gfs2 superblock update clash gfs1's sb_jindex_di field is at the same offset as gfs2's sb_master_dir. Writing both in lgfs2_sb_out() means that one can overwrite the other when the values are different. To avoid that possibility, make them a union and add unit tests that cover this scenario. Signed-off-by: Andrew Price <anprice(a)redhat.com&gt; --- gfs2/libgfs2/check_libgfs2.c | 2 + gfs2/libgfs2/check_ondisk.c | 155 +++++++++++++++++++++++++++++++++++++++++++ gfs2/libgfs2/checks.am | 2 +- gfs2/libgfs2/libgfs2.h | 7 +- 4 files changed, 163 insertions(+), 3 deletions(-) diff --git a/gfs2/libgfs2/check_libgfs2.c b/gfs2/libgfs2/check_libgfs2.c index 0cfe08f2..a5d9df52 100644 --- a/gfs2/libgfs2/check_libgfs2.c +++ b/gfs2/libgfs2/check_libgfs2.c @@ -5,6 +5,7 @@ void print_it(const char *label, const char *fmt, const char *fmt2, ...) {} extern Suite *suite_meta(void); +extern Suite *suite_ondisk(void); extern Suite *suite_rgrp(void); int main(void) @@ -12,6 +13,7 @@ int main(void) int failures; SRunner *runner = srunner_create(suite_meta()); + srunner_add_suite(runner, suite_ondisk()); srunner_add_suite(runner, suite_rgrp()); srunner_run_all(runner, CK_ENV); diff --git a/gfs2/libgfs2/check_ondisk.c b/gfs2/libgfs2/check_ondisk.c new file mode 100644 index 00000000..6c8dd065 --- /dev/null +++ b/gfs2/libgfs2/check_ondisk.c @@ -0,0 +1,155 @@ +#include <check.h> +#include "libgfs2.h" + +Suite *suite_ondisk(void); + +START_TEST(check_sb_in) +{ + char buf[sizeof(struct gfs2_sb)]; + char namechk[GFS2_LOCKNAME_LEN]; + struct gfs2_sbd sbd; + char uuidchk[sizeof(sbd.sd_uuid)]; + + memset(buf, 0x5a, sizeof(buf)); + memset(namechk, 0x5a, GFS2_LOCKNAME_LEN); + memset(uuidchk, 0x5a, sizeof(sbd.sd_uuid)); + memset(&sbd, 0, sizeof(sbd)); + + lgfs2_sb_in(&sbd, buf); + /* Compare each field individually to find exactly where any bugs are */ + ck_assert(sbd.sd_fs_format == 0x5a5a5a5a); + ck_assert(sbd.sd_multihost_format == 0x5a5a5a5a); + ck_assert(sbd.sd_flags == 0x5a5a5a5a); + ck_assert(sbd.sd_bsize == 0x5a5a5a5a); + ck_assert(sbd.sd_bsize_shift == 0x5a5a5a5a); + ck_assert(sbd.sd_seg_size == 0x5a5a5a5a); + ck_assert(sbd.sd_meta_dir.in_formal_ino == 0x5a5a5a5a5a5a5a5a); + ck_assert(sbd.sd_meta_dir.in_addr == 0x5a5a5a5a5a5a5a5a); + ck_assert(sbd.sd_root_dir.in_formal_ino == 0x5a5a5a5a5a5a5a5a); + ck_assert(sbd.sd_root_dir.in_addr == 0x5a5a5a5a5a5a5a5a); + ck_assert(memcmp(sbd.sd_lockproto, namechk, GFS2_LOCKNAME_LEN) == 0); + ck_assert(memcmp(sbd.sd_locktable, namechk, GFS2_LOCKNAME_LEN) == 0); + ck_assert(sbd.sd_jindex_di.in_formal_ino == 0x5a5a5a5a5a5a5a5a); + ck_assert(sbd.sd_jindex_di.in_addr == 0x5a5a5a5a5a5a5a5a); + ck_assert(sbd.sd_rindex_di.in_formal_ino == 0x5a5a5a5a5a5a5a5a); + ck_assert(sbd.sd_rindex_di.in_addr == 0x5a5a5a5a5a5a5a5a); + ck_assert(sbd.sd_quota_di.in_formal_ino == 0x5a5a5a5a5a5a5a5a); + ck_assert(sbd.sd_quota_di.in_addr == 0x5a5a5a5a5a5a5a5a); + ck_assert(sbd.sd_license_di.in_formal_ino == 0x5a5a5a5a5a5a5a5a); + ck_assert(sbd.sd_license_di.in_addr == 0x5a5a5a5a5a5a5a5a); + ck_assert(memcmp(sbd.sd_uuid, uuidchk, sizeof(sbd.sd_uuid)) == 0); +} +END_TEST + +START_TEST(check_sb1_out) +{ + char namechk[GFS2_LOCKNAME_LEN]; + char buf[sizeof(struct gfs_sb)]; + struct gfs2_sbd sbd; + struct gfs_sb *sb; + + memset(namechk, 0x5a, GFS2_LOCKNAME_LEN); + + /* 1. If only the gfs1 fields are set, the sb must be filled */ + memset(buf, 0, sizeof(buf)); + memset(&sbd, 0, sizeof(sbd)); + + sbd.sd_fs_format = 0x5a5a5a51; + sbd.sd_multihost_format = 0x5a5a5a52; + sbd.sd_flags = 0x5a5a5a53; + sbd.sd_bsize = 0x5a5a5a54; + sbd.sd_bsize_shift = 0x5a5a5a55; + sbd.sd_seg_size = 0x5a5a5a56; + sbd.sd_jindex_di.in_formal_ino = 0x5a5a5a5a5a5a5a57; + sbd.sd_jindex_di.in_addr = 0x5a5a5a5a5a5a5a58; + sbd.sd_rindex_di.in_formal_ino = 0x5a5a5a5a5a5a5a59; + sbd.sd_rindex_di.in_addr = 0x5a5a5a5a5a5a5a5a; + sbd.sd_root_dir.in_formal_ino = 0x5a5a5a5a5a5a5a5b; + sbd.sd_root_dir.in_addr = 0x5a5a5a5a5a5a5a5c; + memset(sbd.sd_lockproto, 0x5a, sizeof(sbd.sd_lockproto)); + memset(sbd.sd_locktable, 0x5a, sizeof(sbd.sd_locktable)); + sbd.sd_quota_di.in_formal_ino = 0x5a5a5a5a5a5a5a5d; + sbd.sd_quota_di.in_addr = 0x5a5a5a5a5a5a5a5e; + sbd.sd_license_di.in_formal_ino = 0x5a5a5a5a5a5a5a5f; + sbd.sd_license_di.in_addr = 0x5a5a5a5a5a5a5a50; + + lgfs2_sb_out(&sbd, buf); + + sb = (struct gfs_sb *)buf; + ck_assert(be32_to_cpu(sb->sb_fs_format) == 0x5a5a5a51); + ck_assert(be32_to_cpu(sb->sb_multihost_format) == 0x5a5a5a52); + ck_assert(be32_to_cpu(sb->sb_flags) == 0x5a5a5a53); + ck_assert(be32_to_cpu(sb->sb_bsize) == 0x5a5a5a54); + ck_assert(be32_to_cpu(sb->sb_bsize_shift) == 0x5a5a5a55); + ck_assert(be32_to_cpu(sb->sb_seg_size) == 0x5a5a5a56); + ck_assert(be64_to_cpu(sb->sb_jindex_di.no_formal_ino) == 0x5a5a5a5a5a5a5a57); + ck_assert(be64_to_cpu(sb->sb_jindex_di.no_addr) == 0x5a5a5a5a5a5a5a58); + ck_assert(be64_to_cpu(sb->sb_rindex_di.no_formal_ino) == 0x5a5a5a5a5a5a5a59); + ck_assert(be64_to_cpu(sb->sb_rindex_di.no_addr) == 0x5a5a5a5a5a5a5a5a); + ck_assert(be64_to_cpu(sb->sb_root_di.no_formal_ino) == 0x5a5a5a5a5a5a5a5b); + ck_assert(be64_to_cpu(sb->sb_root_di.no_addr) == 0x5a5a5a5a5a5a5a5c); + ck_assert(memcmp(sb->sb_lockproto, namechk, GFS2_LOCKNAME_LEN) == 0); + ck_assert(memcmp(sb->sb_locktable, namechk, GFS2_LOCKNAME_LEN) == 0); + ck_assert(be64_to_cpu(sb->sb_quota_di.no_formal_ino) == 0x5a5a5a5a5a5a5a5d); + ck_assert(be64_to_cpu(sb->sb_quota_di.no_addr) == 0x5a5a5a5a5a5a5a5e); + ck_assert(be64_to_cpu(sb->sb_license_di.no_formal_ino) == 0x5a5a5a5a5a5a5a5f); + ck_assert(be64_to_cpu(sb->sb_license_di.no_addr) == 0x5a5a5a5a5a5a5a50); +} +END_TEST + +START_TEST(check_sb2_out) +{ + char buf[sizeof(struct gfs2_sb)]; + char namechk[GFS2_LOCKNAME_LEN]; + struct gfs2_sbd sbd; + struct gfs2_sb *sb; + char uuidchk[sizeof(sbd.sd_uuid)]; + + memset(namechk, 0x5a, GFS2_LOCKNAME_LEN); + memset(uuidchk, 0x5a, sizeof(sbd.sd_uuid)); + + /* 2. If only the gfs2 fields are set, the sb must be filled */ + memset(buf, 0, sizeof(buf)); + memset(&sbd, 0, sizeof(sbd)); + + sbd.sd_fs_format = 0x5a5a5a50; + sbd.sd_multihost_format = 0x5a5a5a51; + sbd.sd_bsize = 0x5a5a5a52; + sbd.sd_bsize_shift = 0x5a5a5a53; + sbd.sd_meta_dir.in_formal_ino = 0x5a5a5a5a5a5a5a54; + sbd.sd_meta_dir.in_addr = 0x5a5a5a5a5a5a5a55; + sbd.sd_root_dir.in_formal_ino = 0x5a5a5a5a5a5a5a56; + sbd.sd_root_dir.in_addr = 0x5a5a5a5a5a5a5a57; + memset(sbd.sd_lockproto, 0x5a, sizeof(sbd.sd_lockproto)); + memset(sbd.sd_locktable, 0x5a, sizeof(sbd.sd_locktable)); + memset(sbd.sd_uuid, 0x5a, sizeof(sbd.sd_uuid)); + + lgfs2_sb_out(&sbd, buf); + + sb = (struct gfs2_sb *)buf; + ck_assert(be32_to_cpu(sb->sb_fs_format) == 0x5a5a5a50); + ck_assert(be32_to_cpu(sb->sb_multihost_format) == 0x5a5a5a51); + ck_assert(be32_to_cpu(sb->sb_bsize) == 0x5a5a5a52); + ck_assert(be32_to_cpu(sb->sb_bsize_shift) == 0x5a5a5a53); + ck_assert(be64_to_cpu(sb->sb_master_dir.no_formal_ino) == 0x5a5a5a5a5a5a5a54); + ck_assert(be64_to_cpu(sb->sb_master_dir.no_addr) == 0x5a5a5a5a5a5a5a55); + ck_assert(be64_to_cpu(sb->sb_root_dir.no_formal_ino) == 0x5a5a5a5a5a5a5a56); + ck_assert(be64_to_cpu(sb->sb_root_dir.no_addr) == 0x5a5a5a5a5a5a5a57); + ck_assert(memcmp(sb->sb_lockproto, namechk, GFS2_LOCKNAME_LEN) == 0); + ck_assert(memcmp(sb->sb_locktable, namechk, GFS2_LOCKNAME_LEN) == 0); + ck_assert(memcmp(sb->sb_uuid, uuidchk, sizeof(sb->sb_uuid)) == 0); +} +END_TEST + +Suite *suite_ondisk(void) +{ + Suite *s = suite_create("ondisk.c"); + + TCase *tc_meta = tcase_create("On-disk structure parsing checks"); + tcase_add_test(tc_meta, check_sb_in); + tcase_add_test(tc_meta, check_sb1_out); + tcase_add_test(tc_meta, check_sb2_out); + suite_add_tcase(s, tc_meta); + + return s; +} diff --git a/gfs2/libgfs2/checks.am b/gfs2/libgfs2/checks.am index 2a2b7538..18d719ab 100644 --- a/gfs2/libgfs2/checks.am +++ b/gfs2/libgfs2/checks.am @@ -7,7 +7,7 @@ check_libgfs2_SOURCES = \ rgrp.c check_rgrp.c \ crc32c.c \ gfs2_disk_hash.c \ - ondisk.c \ + ondisk.c check_ondisk.c \ buf.c \ device_geometry.c \ fs_ops.c \ diff --git a/gfs2/libgfs2/libgfs2.h b/gfs2/libgfs2/libgfs2.h index 41f83b94..2b816f3c 100644 --- a/gfs2/libgfs2/libgfs2.h +++ b/gfs2/libgfs2/libgfs2.h @@ -325,10 +325,13 @@ struct gfs2_sbd { uint32_t sd_fs_format; uint32_t sd_multihost_format; uint32_t sd_flags; /* gfs1 */ - struct lgfs2_inum sd_meta_dir; + /* gfs1's sb_jindex_di is gfs2's sb_master_dir */ + union { + struct lgfs2_inum sd_meta_dir; + struct lgfs2_inum sd_jindex_di; /* gfs1 */ + }; struct lgfs2_inum sd_root_dir; struct lgfs2_inum sd_rindex_di; /* gfs1 */ - struct lgfs2_inum sd_jindex_di; /* gfs1 */ struct lgfs2_inum sd_quota_di; /* gfs1 */ struct lgfs2_inum sd_license_di; /* gfs1 */ uint32_t sd_bsize_shift; -- To stop receiving notification emails like this one, please contact the administrator of this repository.