Re: [PATCH] xfs: port xfs/122 to the kernel

Dave Chinner
2024-10-15T09:09:57Z

On Fri, Oct 11, 2024 at 11:24:07AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
>
> Check this with every kernel and userspace build, so we can drop the
> nonsense in xfs/122. Roughly drafted with:
>
> sed -e 's/^offsetof/\tXFS_CHECK_OFFSET/g' \
> -e 's/^sizeof/\tXFS_CHECK_STRUCT_SIZE/g' \
> -e 's/ = \([0-9]*\)/,\t\t\t\1);/g' \
> -e 's/xfs_sb_t/struct xfs_dsb/g' \
> -e 's/),/,/g' \
> -e 's/xfs_\([a-z0-9_]*\)_t,/struct xfs_\1,/g' \
> < tests/xfs/122.out | sort
>
> and then manual fixups.

[snip on disk structures]

I don't think we can type check all these ioctl structures,
especially the old ones.

i.e. The old ioctl structures are not padded to 64 bit boundaries,
nor are they constructed without internal padding holes, and this is
why compat ioctls exist. Hence any ioctl structure that has a compat
definition in xfs_ioctl32.h can't be size checked like this....

> + XFS_CHECK_STRUCT_SIZE(struct xfs_error_injection, 8);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_exchange_range, 40);
> + XFS_CHECK_STRUCT_SIZE(xfs_exntst_t, 4);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_fid, 16);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_fs_eofblocks, 128);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_fsid, 8);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_fsop_counts, 32);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_fsop_geom, 256);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_fsop_geom_v1, 112);

e.g. xfs_fsop_geom_v1 is 108 bytes on 32 bit systems, not 112:

struct compat_xfs_fsop_geom_v1 {
__u32 blocksize; /* 0 4 */
__u32 rtextsize; /* 4 4 */
__u32 agblocks; /* 8 4 */
__u32 agcount; /* 12 4 */
__u32 logblocks; /* 16 4 */
__u32 sectsize; /* 20 4 */
__u32 inodesize; /* 24 4 */
__u32 imaxpct; /* 28 4 */
__u64 datablocks; /* 32 8 */
__u64 rtblocks; /* 40 8 */
__u64 rtextents; /* 48 8 */
__u64 logstart; /* 56 8 */
/* --- cacheline 1 boundary (64 bytes) --- */
unsigned char uuid[16]; /* 64 16 */
__u32 sunit; /* 80 4 */
__u32 swidth; /* 84 4 */
__s32 version; /* 88 4 */
__u32 flags; /* 92 4 */
__u32 logsectsize; /* 96 4 */
__u32 rtsectsize; /* 100 4 */
__u32 dirblocksize; /* 104 4 */

/* size: 108, cachelines: 2, members: 20 */
/* last cacheline: 44 bytes */
} __attribute__((__packed__));

I'm not sure we need to size check these structures - if they change
size, the ioctl number will change and that means all the userspace
test code built against the system /usr/include/xfs/xfs_fs.h file
that exercises the ioctls will stop working, right? i.e. breakage
should be pretty obvious...

-Dave.
--
Dave Chinner
david@fromorbit.com