| Message ID | 20231108163316.493089-1-hch@lst.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] libxfs: check the size of on-disk data structures | expand |
On Wed, Nov 08, 2023 at 05:33:16PM +0100, Christoph Hellwig wrote: > Provide a dumb BUILD_BUG_ON_MSG and import the kernel xfs_ondisk.h file > so that libxfs_init can check the size of all relevant on-disk > structures. > > This seems like a better way to verify the struct size in userspace > compared to the xfs/122 test in xfstests that needs constant updates. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > > Note that xfs_ondisk.h should move to libxfs if this gets applied. > > include/xfs.h | 4 + > libxfs/init.c | 7 ++ > libxfs/xfs_ondisk.h | 195 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 206 insertions(+) > create mode 100644 libxfs/xfs_ondisk.h > > diff --git a/include/xfs.h b/include/xfs.h > index e97158c8d..796af1f37 100644 > --- a/include/xfs.h > +++ b/include/xfs.h > @@ -38,6 +38,10 @@ extern int xfs_assert_largefile[sizeof(off_t)-8]; > #define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)])) > #endif > > +#ifndef BUILD_BUG_ON_MSG > +#define BUILD_BUG_ON_MSG(a, b) BUILD_BUG_ON(a) How difficult would it be to port the complex kernel macros that actually result in the message being emitted in the gcc error output? It's helpful that when the kernel build breaks, the robots will report exactly which field/struct/whatever tripped, which makes it easier to start figuring out where things went wrong on some weird architecture. Next would be redefining BUILD_BUG_ON as #define BUILD_BUG_ON(c) BUILD_BUG_ON_MSG(condition, condition) so that userspace build failures actually tell you what condition failed without you having to look in the source code. Otherwise I'm all for porting xfs_ondisk.h to xfsprogs. IIRC I tried that a long time ago and Dave or someone said xfs/122 was the answer. --D > +#endif > + > #define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER)) > > #include <xfs/xfs_types.h> > diff --git a/libxfs/init.c b/libxfs/init.c > index ce6e62cde..aa37ef651 100644 > --- a/libxfs/init.c > +++ b/libxfs/init.c > @@ -23,6 +23,11 @@ > #include "xfs_refcount_btree.h" > #include "libfrog/platform.h" > > +#include "xfs_format.h" > +#include "xfs_da_format.h" > +#include "xfs_log_format.h" > +#include "xfs_ondisk.h" > + > #include "libxfs.h" /* for now */ > > #ifndef HAVE_LIBURCU_ATOMIC64 > @@ -317,6 +322,8 @@ libxfs_init(libxfs_init_t *a) > int rval = 0; > int flags; > > + xfs_check_ondisk_structs(); > + > dpath[0] = logpath[0] = rtpath[0] = '\0'; > dname = a->dname; > logname = a->logname; > diff --git a/libxfs/xfs_ondisk.h b/libxfs/xfs_ondisk.h > new file mode 100644 > index 000000000..c4cc99b70 > --- /dev/null > +++ b/libxfs/xfs_ondisk.h > @@ -0,0 +1,195 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2016 Oracle. > + * All Rights Reserved. > + */ > +#ifndef __XFS_ONDISK_H > +#define __XFS_ONDISK_H > + > +#define XFS_CHECK_STRUCT_SIZE(structname, size) \ > + BUILD_BUG_ON_MSG(sizeof(structname) != (size), "XFS: sizeof(" \ > + #structname ") is wrong, expected " #size) > + > +#define XFS_CHECK_OFFSET(structname, member, off) \ > + BUILD_BUG_ON_MSG(offsetof(structname, member) != (off), \ > + "XFS: offsetof(" #structname ", " #member ") is wrong, " \ > + "expected " #off) > + > +#define XFS_CHECK_VALUE(value, expected) \ > + BUILD_BUG_ON_MSG((value) != (expected), \ > + "XFS: value of " #value " is wrong, expected " #expected) > + > +static inline void __init > +xfs_check_ondisk_structs(void) > +{ > + /* ag/file structures */ > + XFS_CHECK_STRUCT_SIZE(struct xfs_acl, 4); > + XFS_CHECK_STRUCT_SIZE(struct xfs_acl_entry, 12); > + XFS_CHECK_STRUCT_SIZE(struct xfs_agf, 224); > + XFS_CHECK_STRUCT_SIZE(struct xfs_agfl, 36); > + XFS_CHECK_STRUCT_SIZE(struct xfs_agi, 344); > + XFS_CHECK_STRUCT_SIZE(struct xfs_bmbt_key, 8); > + XFS_CHECK_STRUCT_SIZE(struct xfs_bmbt_rec, 16); > + XFS_CHECK_STRUCT_SIZE(struct xfs_bmdr_block, 4); > + XFS_CHECK_STRUCT_SIZE(struct xfs_btree_block_shdr, 48); > + XFS_CHECK_STRUCT_SIZE(struct xfs_btree_block_lhdr, 64); > + XFS_CHECK_STRUCT_SIZE(struct xfs_btree_block, 72); > + XFS_CHECK_STRUCT_SIZE(struct xfs_dinode, 176); > + XFS_CHECK_STRUCT_SIZE(struct xfs_disk_dquot, 104); > + XFS_CHECK_STRUCT_SIZE(struct xfs_dqblk, 136); > + XFS_CHECK_STRUCT_SIZE(struct xfs_dsb, 264); > + XFS_CHECK_STRUCT_SIZE(struct xfs_dsymlink_hdr, 56); > + XFS_CHECK_STRUCT_SIZE(struct xfs_inobt_key, 4); > + XFS_CHECK_STRUCT_SIZE(struct xfs_inobt_rec, 16); > + XFS_CHECK_STRUCT_SIZE(struct xfs_refcount_key, 4); > + XFS_CHECK_STRUCT_SIZE(struct xfs_refcount_rec, 12); > + XFS_CHECK_STRUCT_SIZE(struct xfs_rmap_key, 20); > + XFS_CHECK_STRUCT_SIZE(struct xfs_rmap_rec, 24); > + XFS_CHECK_STRUCT_SIZE(xfs_timestamp_t, 8); > + XFS_CHECK_STRUCT_SIZE(struct xfs_legacy_timestamp, 8); > + XFS_CHECK_STRUCT_SIZE(xfs_alloc_key_t, 8); > + XFS_CHECK_STRUCT_SIZE(xfs_alloc_ptr_t, 4); > + XFS_CHECK_STRUCT_SIZE(xfs_alloc_rec_t, 8); > + XFS_CHECK_STRUCT_SIZE(xfs_inobt_ptr_t, 4); > + XFS_CHECK_STRUCT_SIZE(xfs_refcount_ptr_t, 4); > + XFS_CHECK_STRUCT_SIZE(xfs_rmap_ptr_t, 4); > + > + /* dir/attr trees */ > + XFS_CHECK_STRUCT_SIZE(struct xfs_attr3_leaf_hdr, 80); > + XFS_CHECK_STRUCT_SIZE(struct xfs_attr3_leafblock, 80); > + XFS_CHECK_STRUCT_SIZE(struct xfs_attr3_rmt_hdr, 56); > + XFS_CHECK_STRUCT_SIZE(struct xfs_da3_blkinfo, 56); > + XFS_CHECK_STRUCT_SIZE(struct xfs_da3_intnode, 64); > + XFS_CHECK_STRUCT_SIZE(struct xfs_da3_node_hdr, 64); > + XFS_CHECK_STRUCT_SIZE(struct xfs_dir3_blk_hdr, 48); > + XFS_CHECK_STRUCT_SIZE(struct xfs_dir3_data_hdr, 64); > + XFS_CHECK_STRUCT_SIZE(struct xfs_dir3_free, 64); > + XFS_CHECK_STRUCT_SIZE(struct xfs_dir3_free_hdr, 64); > + XFS_CHECK_STRUCT_SIZE(struct xfs_dir3_leaf, 64); > + XFS_CHECK_STRUCT_SIZE(struct xfs_dir3_leaf_hdr, 64); > + XFS_CHECK_STRUCT_SIZE(xfs_attr_leaf_entry_t, 8); > + XFS_CHECK_STRUCT_SIZE(xfs_attr_leaf_hdr_t, 32); > + XFS_CHECK_STRUCT_SIZE(xfs_attr_leaf_map_t, 4); > + XFS_CHECK_STRUCT_SIZE(xfs_attr_leaf_name_local_t, 4); > + > + /* > + * m68k has problems with xfs_attr_leaf_name_remote_t, but we pad it to > + * 4 bytes anyway so it's not obviously a problem. Hence for the moment > + * we don't check this structure. This can be re-instated when the attr > + * definitions are updated to use c99 VLA definitions. > + * > + XFS_CHECK_STRUCT_SIZE(xfs_attr_leaf_name_remote_t, 12); > + */ > + > + XFS_CHECK_OFFSET(xfs_attr_leaf_name_local_t, valuelen, 0); > + XFS_CHECK_OFFSET(xfs_attr_leaf_name_local_t, namelen, 2); > + XFS_CHECK_OFFSET(xfs_attr_leaf_name_local_t, nameval, 3); > + XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, valueblk, 0); > + XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, valuelen, 4); > + XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, namelen, 8); > + XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, name, 9); > + XFS_CHECK_STRUCT_SIZE(xfs_attr_leafblock_t, 32); > + XFS_CHECK_STRUCT_SIZE(struct xfs_attr_shortform, 4); > + XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.totsize, 0); > + XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.count, 2); > + XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].namelen, 4); > + XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].valuelen, 5); > + XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].flags, 6); > + XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].nameval, 7); > + XFS_CHECK_STRUCT_SIZE(xfs_da_blkinfo_t, 12); > + XFS_CHECK_STRUCT_SIZE(xfs_da_intnode_t, 16); > + XFS_CHECK_STRUCT_SIZE(xfs_da_node_entry_t, 8); > + XFS_CHECK_STRUCT_SIZE(xfs_da_node_hdr_t, 16); > + XFS_CHECK_STRUCT_SIZE(xfs_dir2_data_free_t, 4); > + XFS_CHECK_STRUCT_SIZE(xfs_dir2_data_hdr_t, 16); > + XFS_CHECK_OFFSET(xfs_dir2_data_unused_t, freetag, 0); > + XFS_CHECK_OFFSET(xfs_dir2_data_unused_t, length, 2); > + XFS_CHECK_STRUCT_SIZE(xfs_dir2_free_hdr_t, 16); > + XFS_CHECK_STRUCT_SIZE(xfs_dir2_free_t, 16); > + XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_entry_t, 8); > + XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_hdr_t, 16); > + XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_t, 16); > + XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_tail_t, 4); > + XFS_CHECK_STRUCT_SIZE(xfs_dir2_sf_entry_t, 3); > + XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, namelen, 0); > + XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, offset, 1); > + XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, name, 3); > + XFS_CHECK_STRUCT_SIZE(xfs_dir2_sf_hdr_t, 10); > + > + /* log structures */ > + XFS_CHECK_STRUCT_SIZE(struct xfs_buf_log_format, 88); > + XFS_CHECK_STRUCT_SIZE(struct xfs_dq_logformat, 24); > + XFS_CHECK_STRUCT_SIZE(struct xfs_efd_log_format_32, 16); > + XFS_CHECK_STRUCT_SIZE(struct xfs_efd_log_format_64, 16); > + XFS_CHECK_STRUCT_SIZE(struct xfs_efi_log_format_32, 16); > + XFS_CHECK_STRUCT_SIZE(struct xfs_efi_log_format_64, 16); > + XFS_CHECK_STRUCT_SIZE(struct xfs_extent_32, 12); > + XFS_CHECK_STRUCT_SIZE(struct xfs_extent_64, 16); > + XFS_CHECK_STRUCT_SIZE(struct xfs_log_dinode, 176); > + XFS_CHECK_STRUCT_SIZE(struct xfs_icreate_log, 28); > + XFS_CHECK_STRUCT_SIZE(xfs_log_timestamp_t, 8); > + XFS_CHECK_STRUCT_SIZE(struct xfs_log_legacy_timestamp, 8); > + XFS_CHECK_STRUCT_SIZE(struct xfs_inode_log_format_32, 52); > + XFS_CHECK_STRUCT_SIZE(struct xfs_inode_log_format, 56); > + XFS_CHECK_STRUCT_SIZE(struct xfs_qoff_logformat, 20); > + XFS_CHECK_STRUCT_SIZE(struct xfs_trans_header, 16); > + XFS_CHECK_STRUCT_SIZE(struct xfs_attri_log_format, 40); > + XFS_CHECK_STRUCT_SIZE(struct xfs_attrd_log_format, 16); > + XFS_CHECK_STRUCT_SIZE(struct xfs_bui_log_format, 16); > + XFS_CHECK_STRUCT_SIZE(struct xfs_bud_log_format, 16); > + XFS_CHECK_STRUCT_SIZE(struct xfs_cui_log_format, 16); > + XFS_CHECK_STRUCT_SIZE(struct xfs_cud_log_format, 16); > + XFS_CHECK_STRUCT_SIZE(struct xfs_rui_log_format, 16); > + XFS_CHECK_STRUCT_SIZE(struct xfs_rud_log_format, 16); > + XFS_CHECK_STRUCT_SIZE(struct xfs_map_extent, 32); > + XFS_CHECK_STRUCT_SIZE(struct xfs_phys_extent, 16); > + > + XFS_CHECK_OFFSET(struct xfs_bui_log_format, bui_extents, 16); > + XFS_CHECK_OFFSET(struct xfs_cui_log_format, cui_extents, 16); > + XFS_CHECK_OFFSET(struct xfs_rui_log_format, rui_extents, 16); > + XFS_CHECK_OFFSET(struct xfs_efi_log_format, efi_extents, 16); > + XFS_CHECK_OFFSET(struct xfs_efi_log_format_32, efi_extents, 16); > + XFS_CHECK_OFFSET(struct xfs_efi_log_format_64, efi_extents, 16); > + > + /* > + * The v5 superblock format extended several v4 header structures with > + * additional data. While new fields are only accessible on v5 > + * superblocks, it's important that the v5 structures place original v4 > + * fields/headers in the correct location on-disk. For example, we must > + * be able to find magic values at the same location in certain blocks > + * regardless of superblock version. > + * > + * The following checks ensure that various v5 data structures place the > + * subset of v4 metadata associated with the same type of block at the > + * start of the on-disk block. If there is no data structure definition > + * for certain types of v4 blocks, traverse down to the first field of > + * common metadata (e.g., magic value) and make sure it is at offset > + * zero. > + */ > + XFS_CHECK_OFFSET(struct xfs_dir3_leaf, hdr.info.hdr, 0); > + XFS_CHECK_OFFSET(struct xfs_da3_intnode, hdr.info.hdr, 0); > + XFS_CHECK_OFFSET(struct xfs_dir3_data_hdr, hdr.magic, 0); > + XFS_CHECK_OFFSET(struct xfs_dir3_free, hdr.hdr.magic, 0); > + XFS_CHECK_OFFSET(struct xfs_attr3_leafblock, hdr.info.hdr, 0); > + > + XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat, 192); > + XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers, 24); > + XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat_req, 64); > + XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers_req, 64); > + > + /* > + * Make sure the incore inode timestamp range corresponds to hand > + * converted values based on the ondisk format specification. > + */ > + XFS_CHECK_VALUE(XFS_BIGTIME_TIME_MIN - XFS_BIGTIME_EPOCH_OFFSET, > + XFS_LEGACY_TIME_MIN); > + XFS_CHECK_VALUE(XFS_BIGTIME_TIME_MAX - XFS_BIGTIME_EPOCH_OFFSET, > + 16299260424LL); > + > + /* Do the same with the incore quota expiration range. */ > + XFS_CHECK_VALUE(XFS_DQ_BIGTIME_EXPIRY_MIN << XFS_DQ_BIGTIME_SHIFT, 4); > + XFS_CHECK_VALUE(XFS_DQ_BIGTIME_EXPIRY_MAX << XFS_DQ_BIGTIME_SHIFT, > + 16299260424LL); > +} > + > +#endif /* __XFS_ONDISK_H */ > -- > 2.39.2 >
On Thu, Nov 09, 2023 at 11:52:33AM -0800, Darrick J. Wong wrote: > > +#ifndef BUILD_BUG_ON_MSG > > +#define BUILD_BUG_ON_MSG(a, b) BUILD_BUG_ON(a) > > How difficult would it be to port the complex kernel macros that > actually result in the message being emitted in the gcc error output? > > It's helpful that when the kernel build breaks, the robots will report > exactly which field/struct/whatever tripped, which makes it easier to > start figuring out where things went wrong on some weird architecture. I did try to pull the entire compile time assert machinery from the kernels compiler_types.h in, especially as atomic.h already uses a differnet part of it. After it pulled in two more depdendencies I gave up, but in principle it should be entirely doable. > Otherwise I'm all for porting xfs_ondisk.h to xfsprogs. IIRC I tried > that a long time ago and Dave or someone said xfs/122 was the answer. I'd much prefer to do it in C code and inside the libxfs we build. If we can agree on that and on killing off xfs/122 I'll look into porting the more complex compile time assert. The other option would be to switch to using static_assert from C11, which doesn't allow a custom message, but at least the default message isn't confusing as hell.
On Fri, Nov 10, 2023 at 06:08:46AM +0100, Christoph Hellwig wrote: > On Thu, Nov 09, 2023 at 11:52:33AM -0800, Darrick J. Wong wrote: > > > +#ifndef BUILD_BUG_ON_MSG > > > +#define BUILD_BUG_ON_MSG(a, b) BUILD_BUG_ON(a) > > > > How difficult would it be to port the complex kernel macros that > > actually result in the message being emitted in the gcc error output? > > > > It's helpful that when the kernel build breaks, the robots will report > > exactly which field/struct/whatever tripped, which makes it easier to > > start figuring out where things went wrong on some weird architecture. > > I did try to pull the entire compile time assert machinery from > the kernels compiler_types.h in, especially as atomic.h already uses > a differnet part of it. After it pulled in two more depdendencies > I gave up, but in principle it should be entirely doable. > > > Otherwise I'm all for porting xfs_ondisk.h to xfsprogs. IIRC I tried > > that a long time ago and Dave or someone said xfs/122 was the answer. > > I'd much prefer to do it in C code and inside the libxfs we build. > If we can agree on that and on killing off xfs/122 I'll look into > porting the more complex compile time assert. > > The other option would be to switch to using static_assert from C11, > which doesn't allow a custom message, but at least the default message > isn't confusing as hell. I copy-pasta'd the whole mess from compiler_types.h and build_bug.h into include/xfs.h. It works, but it might be kinda egregious though. --D
On Thu, Nov 30, 2023 at 06:06:58PM -0800, Darrick J. Wong wrote: > I copy-pasta'd the whole mess from compiler_types.h and build_bug.h into > include/xfs.h. It works, but it might be kinda egregious though. Oh. I actually have a local patch to simply switch to static_assert as that completly relies on the compiler and gives better output. I haven't even written a proper commit log, but this is it: diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h index 2f24bd42ac1dd7..3a5581ecb36d4c 100644 --- a/fs/xfs/xfs_ondisk.h +++ b/fs/xfs/xfs_ondisk.h @@ -7,16 +7,16 @@ #define __XFS_ONDISK_H #define XFS_CHECK_STRUCT_SIZE(structname, size) \ - BUILD_BUG_ON_MSG(sizeof(structname) != (size), "XFS: sizeof(" \ - #structname ") is wrong, expected " #size) + static_assert(sizeof(structname) == (size), \ + "XFS: sizeof(" #structname ") is wrong, expected " #size) #define XFS_CHECK_OFFSET(structname, member, off) \ - BUILD_BUG_ON_MSG(offsetof(structname, member) != (off), \ + static_assert(offsetof(structname, member) == (off), \ "XFS: offsetof(" #structname ", " #member ") is wrong, " \ "expected " #off) #define XFS_CHECK_VALUE(value, expected) \ - BUILD_BUG_ON_MSG((value) != (expected), \ + static_assert((value) == (expected), \ "XFS: value of " #value " is wrong, expected " #expected) static inline void __init
On Mon, Dec 04, 2023 at 05:37:18AM +0100, Christoph Hellwig wrote: > On Thu, Nov 30, 2023 at 06:06:58PM -0800, Darrick J. Wong wrote: > > I copy-pasta'd the whole mess from compiler_types.h and build_bug.h into > > include/xfs.h. It works, but it might be kinda egregious though. > > Oh. I actually have a local patch to simply switch to static_assert > as that completly relies on the compiler and gives better output. I > haven't even written a proper commit log, but this is it: > > diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h > index 2f24bd42ac1dd7..3a5581ecb36d4c 100644 > --- a/fs/xfs/xfs_ondisk.h > +++ b/fs/xfs/xfs_ondisk.h > @@ -7,16 +7,16 @@ > #define __XFS_ONDISK_H > > #define XFS_CHECK_STRUCT_SIZE(structname, size) \ > - BUILD_BUG_ON_MSG(sizeof(structname) != (size), "XFS: sizeof(" \ > - #structname ") is wrong, expected " #size) > + static_assert(sizeof(structname) == (size), \ > + "XFS: sizeof(" #structname ") is wrong, expected " #size) > > #define XFS_CHECK_OFFSET(structname, member, off) \ > - BUILD_BUG_ON_MSG(offsetof(structname, member) != (off), \ > + static_assert(offsetof(structname, member) == (off), \ > "XFS: offsetof(" #structname ", " #member ") is wrong, " \ > "expected " #off) > > #define XFS_CHECK_VALUE(value, expected) \ > - BUILD_BUG_ON_MSG((value) != (expected), \ > + static_assert((value) == (expected), \ HAH LOL that's much better. I think I even see that kernel code is using it now, and wonder why BUG_BUILD_ON still exists. Going back for another cup of koolaid now, Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > "XFS: value of " #value " is wrong, expected " #expected) > > static inline void __init >
On Mon, Dec 04, 2023 at 11:53:06AM -0800, Darrick J. Wong wrote: > > #define XFS_CHECK_VALUE(value, expected) \ > > - BUILD_BUG_ON_MSG((value) != (expected), \ > > + static_assert((value) == (expected), \ > > HAH LOL that's much better. I think I even see that kernel code is > using it now, and wonder why BUG_BUILD_ON still exists. Yup: hch@brick:~/work/linux$ git-grep static_assert | wc -l 840 hch@brick:~/work/linux$ git-grep BUILD_BUG_ON_MSG | wc -l 92 > Going back for another cup of koolaid now, Haha. > Reviewed-by: Darrick J. Wong <djwong@kernel.org> Thanks. I was just getting ready to send out a series with this and the xfs_ondisk.h move.
diff --git a/include/xfs.h b/include/xfs.h index e97158c8d..796af1f37 100644 --- a/include/xfs.h +++ b/include/xfs.h @@ -38,6 +38,10 @@ extern int xfs_assert_largefile[sizeof(off_t)-8]; #define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)])) #endif +#ifndef BUILD_BUG_ON_MSG +#define BUILD_BUG_ON_MSG(a, b) BUILD_BUG_ON(a) +#endif + #define sizeof_field(TYPE, MEMBER) sizeof((((TYPE *)0)->MEMBER)) #include <xfs/xfs_types.h> diff --git a/libxfs/init.c b/libxfs/init.c index ce6e62cde..aa37ef651 100644 --- a/libxfs/init.c +++ b/libxfs/init.c @@ -23,6 +23,11 @@ #include "xfs_refcount_btree.h" #include "libfrog/platform.h" +#include "xfs_format.h" +#include "xfs_da_format.h" +#include "xfs_log_format.h" +#include "xfs_ondisk.h" + #include "libxfs.h" /* for now */ #ifndef HAVE_LIBURCU_ATOMIC64 @@ -317,6 +322,8 @@ libxfs_init(libxfs_init_t *a) int rval = 0; int flags; + xfs_check_ondisk_structs(); + dpath[0] = logpath[0] = rtpath[0] = '\0'; dname = a->dname; logname = a->logname; diff --git a/libxfs/xfs_ondisk.h b/libxfs/xfs_ondisk.h new file mode 100644 index 000000000..c4cc99b70 --- /dev/null +++ b/libxfs/xfs_ondisk.h @@ -0,0 +1,195 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2016 Oracle. + * All Rights Reserved. + */ +#ifndef __XFS_ONDISK_H +#define __XFS_ONDISK_H + +#define XFS_CHECK_STRUCT_SIZE(structname, size) \ + BUILD_BUG_ON_MSG(sizeof(structname) != (size), "XFS: sizeof(" \ + #structname ") is wrong, expected " #size) + +#define XFS_CHECK_OFFSET(structname, member, off) \ + BUILD_BUG_ON_MSG(offsetof(structname, member) != (off), \ + "XFS: offsetof(" #structname ", " #member ") is wrong, " \ + "expected " #off) + +#define XFS_CHECK_VALUE(value, expected) \ + BUILD_BUG_ON_MSG((value) != (expected), \ + "XFS: value of " #value " is wrong, expected " #expected) + +static inline void __init +xfs_check_ondisk_structs(void) +{ + /* ag/file structures */ + XFS_CHECK_STRUCT_SIZE(struct xfs_acl, 4); + XFS_CHECK_STRUCT_SIZE(struct xfs_acl_entry, 12); + XFS_CHECK_STRUCT_SIZE(struct xfs_agf, 224); + XFS_CHECK_STRUCT_SIZE(struct xfs_agfl, 36); + XFS_CHECK_STRUCT_SIZE(struct xfs_agi, 344); + XFS_CHECK_STRUCT_SIZE(struct xfs_bmbt_key, 8); + XFS_CHECK_STRUCT_SIZE(struct xfs_bmbt_rec, 16); + XFS_CHECK_STRUCT_SIZE(struct xfs_bmdr_block, 4); + XFS_CHECK_STRUCT_SIZE(struct xfs_btree_block_shdr, 48); + XFS_CHECK_STRUCT_SIZE(struct xfs_btree_block_lhdr, 64); + XFS_CHECK_STRUCT_SIZE(struct xfs_btree_block, 72); + XFS_CHECK_STRUCT_SIZE(struct xfs_dinode, 176); + XFS_CHECK_STRUCT_SIZE(struct xfs_disk_dquot, 104); + XFS_CHECK_STRUCT_SIZE(struct xfs_dqblk, 136); + XFS_CHECK_STRUCT_SIZE(struct xfs_dsb, 264); + XFS_CHECK_STRUCT_SIZE(struct xfs_dsymlink_hdr, 56); + XFS_CHECK_STRUCT_SIZE(struct xfs_inobt_key, 4); + XFS_CHECK_STRUCT_SIZE(struct xfs_inobt_rec, 16); + XFS_CHECK_STRUCT_SIZE(struct xfs_refcount_key, 4); + XFS_CHECK_STRUCT_SIZE(struct xfs_refcount_rec, 12); + XFS_CHECK_STRUCT_SIZE(struct xfs_rmap_key, 20); + XFS_CHECK_STRUCT_SIZE(struct xfs_rmap_rec, 24); + XFS_CHECK_STRUCT_SIZE(xfs_timestamp_t, 8); + XFS_CHECK_STRUCT_SIZE(struct xfs_legacy_timestamp, 8); + XFS_CHECK_STRUCT_SIZE(xfs_alloc_key_t, 8); + XFS_CHECK_STRUCT_SIZE(xfs_alloc_ptr_t, 4); + XFS_CHECK_STRUCT_SIZE(xfs_alloc_rec_t, 8); + XFS_CHECK_STRUCT_SIZE(xfs_inobt_ptr_t, 4); + XFS_CHECK_STRUCT_SIZE(xfs_refcount_ptr_t, 4); + XFS_CHECK_STRUCT_SIZE(xfs_rmap_ptr_t, 4); + + /* dir/attr trees */ + XFS_CHECK_STRUCT_SIZE(struct xfs_attr3_leaf_hdr, 80); + XFS_CHECK_STRUCT_SIZE(struct xfs_attr3_leafblock, 80); + XFS_CHECK_STRUCT_SIZE(struct xfs_attr3_rmt_hdr, 56); + XFS_CHECK_STRUCT_SIZE(struct xfs_da3_blkinfo, 56); + XFS_CHECK_STRUCT_SIZE(struct xfs_da3_intnode, 64); + XFS_CHECK_STRUCT_SIZE(struct xfs_da3_node_hdr, 64); + XFS_CHECK_STRUCT_SIZE(struct xfs_dir3_blk_hdr, 48); + XFS_CHECK_STRUCT_SIZE(struct xfs_dir3_data_hdr, 64); + XFS_CHECK_STRUCT_SIZE(struct xfs_dir3_free, 64); + XFS_CHECK_STRUCT_SIZE(struct xfs_dir3_free_hdr, 64); + XFS_CHECK_STRUCT_SIZE(struct xfs_dir3_leaf, 64); + XFS_CHECK_STRUCT_SIZE(struct xfs_dir3_leaf_hdr, 64); + XFS_CHECK_STRUCT_SIZE(xfs_attr_leaf_entry_t, 8); + XFS_CHECK_STRUCT_SIZE(xfs_attr_leaf_hdr_t, 32); + XFS_CHECK_STRUCT_SIZE(xfs_attr_leaf_map_t, 4); + XFS_CHECK_STRUCT_SIZE(xfs_attr_leaf_name_local_t, 4); + + /* + * m68k has problems with xfs_attr_leaf_name_remote_t, but we pad it to + * 4 bytes anyway so it's not obviously a problem. Hence for the moment + * we don't check this structure. This can be re-instated when the attr + * definitions are updated to use c99 VLA definitions. + * + XFS_CHECK_STRUCT_SIZE(xfs_attr_leaf_name_remote_t, 12); + */ + + XFS_CHECK_OFFSET(xfs_attr_leaf_name_local_t, valuelen, 0); + XFS_CHECK_OFFSET(xfs_attr_leaf_name_local_t, namelen, 2); + XFS_CHECK_OFFSET(xfs_attr_leaf_name_local_t, nameval, 3); + XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, valueblk, 0); + XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, valuelen, 4); + XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, namelen, 8); + XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, name, 9); + XFS_CHECK_STRUCT_SIZE(xfs_attr_leafblock_t, 32); + XFS_CHECK_STRUCT_SIZE(struct xfs_attr_shortform, 4); + XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.totsize, 0); + XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.count, 2); + XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].namelen, 4); + XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].valuelen, 5); + XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].flags, 6); + XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].nameval, 7); + XFS_CHECK_STRUCT_SIZE(xfs_da_blkinfo_t, 12); + XFS_CHECK_STRUCT_SIZE(xfs_da_intnode_t, 16); + XFS_CHECK_STRUCT_SIZE(xfs_da_node_entry_t, 8); + XFS_CHECK_STRUCT_SIZE(xfs_da_node_hdr_t, 16); + XFS_CHECK_STRUCT_SIZE(xfs_dir2_data_free_t, 4); + XFS_CHECK_STRUCT_SIZE(xfs_dir2_data_hdr_t, 16); + XFS_CHECK_OFFSET(xfs_dir2_data_unused_t, freetag, 0); + XFS_CHECK_OFFSET(xfs_dir2_data_unused_t, length, 2); + XFS_CHECK_STRUCT_SIZE(xfs_dir2_free_hdr_t, 16); + XFS_CHECK_STRUCT_SIZE(xfs_dir2_free_t, 16); + XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_entry_t, 8); + XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_hdr_t, 16); + XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_t, 16); + XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_tail_t, 4); + XFS_CHECK_STRUCT_SIZE(xfs_dir2_sf_entry_t, 3); + XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, namelen, 0); + XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, offset, 1); + XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, name, 3); + XFS_CHECK_STRUCT_SIZE(xfs_dir2_sf_hdr_t, 10); + + /* log structures */ + XFS_CHECK_STRUCT_SIZE(struct xfs_buf_log_format, 88); + XFS_CHECK_STRUCT_SIZE(struct xfs_dq_logformat, 24); + XFS_CHECK_STRUCT_SIZE(struct xfs_efd_log_format_32, 16); + XFS_CHECK_STRUCT_SIZE(struct xfs_efd_log_format_64, 16); + XFS_CHECK_STRUCT_SIZE(struct xfs_efi_log_format_32, 16); + XFS_CHECK_STRUCT_SIZE(struct xfs_efi_log_format_64, 16); + XFS_CHECK_STRUCT_SIZE(struct xfs_extent_32, 12); + XFS_CHECK_STRUCT_SIZE(struct xfs_extent_64, 16); + XFS_CHECK_STRUCT_SIZE(struct xfs_log_dinode, 176); + XFS_CHECK_STRUCT_SIZE(struct xfs_icreate_log, 28); + XFS_CHECK_STRUCT_SIZE(xfs_log_timestamp_t, 8); + XFS_CHECK_STRUCT_SIZE(struct xfs_log_legacy_timestamp, 8); + XFS_CHECK_STRUCT_SIZE(struct xfs_inode_log_format_32, 52); + XFS_CHECK_STRUCT_SIZE(struct xfs_inode_log_format, 56); + XFS_CHECK_STRUCT_SIZE(struct xfs_qoff_logformat, 20); + XFS_CHECK_STRUCT_SIZE(struct xfs_trans_header, 16); + XFS_CHECK_STRUCT_SIZE(struct xfs_attri_log_format, 40); + XFS_CHECK_STRUCT_SIZE(struct xfs_attrd_log_format, 16); + XFS_CHECK_STRUCT_SIZE(struct xfs_bui_log_format, 16); + XFS_CHECK_STRUCT_SIZE(struct xfs_bud_log_format, 16); + XFS_CHECK_STRUCT_SIZE(struct xfs_cui_log_format, 16); + XFS_CHECK_STRUCT_SIZE(struct xfs_cud_log_format, 16); + XFS_CHECK_STRUCT_SIZE(struct xfs_rui_log_format, 16); + XFS_CHECK_STRUCT_SIZE(struct xfs_rud_log_format, 16); + XFS_CHECK_STRUCT_SIZE(struct xfs_map_extent, 32); + XFS_CHECK_STRUCT_SIZE(struct xfs_phys_extent, 16); + + XFS_CHECK_OFFSET(struct xfs_bui_log_format, bui_extents, 16); + XFS_CHECK_OFFSET(struct xfs_cui_log_format, cui_extents, 16); + XFS_CHECK_OFFSET(struct xfs_rui_log_format, rui_extents, 16); + XFS_CHECK_OFFSET(struct xfs_efi_log_format, efi_extents, 16); + XFS_CHECK_OFFSET(struct xfs_efi_log_format_32, efi_extents, 16); + XFS_CHECK_OFFSET(struct xfs_efi_log_format_64, efi_extents, 16); + + /* + * The v5 superblock format extended several v4 header structures with + * additional data. While new fields are only accessible on v5 + * superblocks, it's important that the v5 structures place original v4 + * fields/headers in the correct location on-disk. For example, we must + * be able to find magic values at the same location in certain blocks + * regardless of superblock version. + * + * The following checks ensure that various v5 data structures place the + * subset of v4 metadata associated with the same type of block at the + * start of the on-disk block. If there is no data structure definition + * for certain types of v4 blocks, traverse down to the first field of + * common metadata (e.g., magic value) and make sure it is at offset + * zero. + */ + XFS_CHECK_OFFSET(struct xfs_dir3_leaf, hdr.info.hdr, 0); + XFS_CHECK_OFFSET(struct xfs_da3_intnode, hdr.info.hdr, 0); + XFS_CHECK_OFFSET(struct xfs_dir3_data_hdr, hdr.magic, 0); + XFS_CHECK_OFFSET(struct xfs_dir3_free, hdr.hdr.magic, 0); + XFS_CHECK_OFFSET(struct xfs_attr3_leafblock, hdr.info.hdr, 0); + + XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat, 192); + XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers, 24); + XFS_CHECK_STRUCT_SIZE(struct xfs_bulkstat_req, 64); + XFS_CHECK_STRUCT_SIZE(struct xfs_inumbers_req, 64); + + /* + * Make sure the incore inode timestamp range corresponds to hand + * converted values based on the ondisk format specification. + */ + XFS_CHECK_VALUE(XFS_BIGTIME_TIME_MIN - XFS_BIGTIME_EPOCH_OFFSET, + XFS_LEGACY_TIME_MIN); + XFS_CHECK_VALUE(XFS_BIGTIME_TIME_MAX - XFS_BIGTIME_EPOCH_OFFSET, + 16299260424LL); + + /* Do the same with the incore quota expiration range. */ + XFS_CHECK_VALUE(XFS_DQ_BIGTIME_EXPIRY_MIN << XFS_DQ_BIGTIME_SHIFT, 4); + XFS_CHECK_VALUE(XFS_DQ_BIGTIME_EXPIRY_MAX << XFS_DQ_BIGTIME_SHIFT, + 16299260424LL); +} + +#endif /* __XFS_ONDISK_H */
Provide a dumb BUILD_BUG_ON_MSG and import the kernel xfs_ondisk.h file so that libxfs_init can check the size of all relevant on-disk structures. This seems like a better way to verify the struct size in userspace compared to the xfs/122 test in xfstests that needs constant updates. Signed-off-by: Christoph Hellwig <hch@lst.de> --- Note that xfs_ondisk.h should move to libxfs if this gets applied. include/xfs.h | 4 + libxfs/init.c | 7 ++ libxfs/xfs_ondisk.h | 195 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 206 insertions(+) create mode 100644 libxfs/xfs_ondisk.h