From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.gmx.net ([212.227.17.21]:44151 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725789AbeGLHFT (ORCPT ); Thu, 12 Jul 2018 03:05:19 -0400 Subject: Re: [PATCH] btrfs: Introduce compile time structure size check To: Nikolay Borisov , Qu Wenruo , linux-btrfs@vger.kernel.org References: <20180712061907.24783-1-wqu@suse.com> From: Qu Wenruo Message-ID: <50163977-ed21-3f97-dced-8bf08dfcfa33@gmx.com> Date: Thu, 12 Jul 2018 14:56:55 +0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Sender: linux-btrfs-owner@vger.kernel.org List-ID: On 2018年07月12日 14:27, Nikolay Borisov wrote: > > > On 12.07.2018 09:19, Qu Wenruo wrote: >> Introduce a new macro based compile time check for ioctl structures. >> >> The new macro is BTRFS_ASSERT_SIZE(), which is mostly copied from >> VMMDEV_ASSERT_SIZE(). >> >> Such check is only added to structure pended to power of 2. >> And exposed one structure, btrfs_ioctl_get_dev_stats() is not aligned >> well. >> The misalign is introduced by commit b27f7c0c150f ("btrfs: join DEV_STATS >> ioctls to one"). > > So what's the negative effect if a structure is not aligned to a power of 2? No real negative effect, until one day one may find the comment is not correct. Thanks, Qu > >> >> Signed-off-by: Qu Wenruo >> --- >> include/uapi/linux/btrfs.h | 18 +++++++++++++++++- >> 1 file changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h >> index 5ca1d21fc4a7..be1213c10080 100644 >> --- a/include/uapi/linux/btrfs.h >> +++ b/include/uapi/linux/btrfs.h >> @@ -22,6 +22,14 @@ >> #include >> #include >> >> +/* >> + * Compile time check on structure size, same method as >> + * VMMDEV_ASSERT_SIZE() from linux/vbox_vmmdev_types.h, to trigger a negative >> + * array size at compile time if size doesn't match. >> + */ >> +#define BTRFS_ASSERT_SIZE(type, size) \ >> + typedef char type ## _asrt_size[1 - 2 * !!(sizeof(struct type) != (size))] >> + >> #define BTRFS_IOCTL_MAGIC 0x94 >> #define BTRFS_VOL_NAME_MAX 255 >> #define BTRFS_LABEL_SIZE 256 >> @@ -32,6 +40,7 @@ struct btrfs_ioctl_vol_args { >> __s64 fd; >> char name[BTRFS_PATH_NAME_MAX + 1]; >> }; >> +BTRFS_ASSERT_SIZE(btrfs_ioctl_vol_args, 4096); >> >> #define BTRFS_DEVICE_PATH_NAME_MAX 1024 >> #define BTRFS_SUBVOL_NAME_MAX 4039 >> @@ -171,6 +180,7 @@ struct btrfs_ioctl_scrub_args { >> /* pad to 1k */ >> __u64 unused[(1024-32-sizeof(struct btrfs_scrub_progress))/8]; >> }; >> +BTRFS_ASSERT_SIZE(btrfs_ioctl_scrub_args, 1024); >> >> #define BTRFS_IOCTL_DEV_REPLACE_CONT_READING_FROM_SRCDEV_MODE_ALWAYS 0 >> #define BTRFS_IOCTL_DEV_REPLACE_CONT_READING_FROM_SRCDEV_MODE_AVOID 1 >> @@ -223,6 +233,7 @@ struct btrfs_ioctl_dev_info_args { >> __u64 unused[379]; /* pad to 4k */ >> __u8 path[BTRFS_DEVICE_PATH_NAME_MAX]; /* out */ >> }; >> +BTRFS_ASSERT_SIZE(btrfs_ioctl_dev_info_args, 4096); >> >> struct btrfs_ioctl_fs_info_args { >> __u64 max_id; /* out */ >> @@ -234,6 +245,7 @@ struct btrfs_ioctl_fs_info_args { >> __u32 reserved32; >> __u64 reserved[122]; /* pad to 1k */ >> }; >> +BTRFS_ASSERT_SIZE(btrfs_ioctl_fs_info_args, 1024); >> >> /* >> * feature flags >> @@ -414,6 +426,7 @@ struct btrfs_ioctl_balance_args { >> >> __u64 unused[72]; /* pad to 1k */ >> }; >> +BTRFS_ASSERT_SIZE(btrfs_ioctl_balance_args, 1024); >> >> #define BTRFS_INO_LOOKUP_PATH_MAX 4080 >> struct btrfs_ioctl_ino_lookup_args { >> @@ -421,6 +434,7 @@ struct btrfs_ioctl_ino_lookup_args { >> __u64 objectid; >> char name[BTRFS_INO_LOOKUP_PATH_MAX]; >> }; >> +BTRFS_ASSERT_SIZE(btrfs_ioctl_ino_lookup_args, 4096); >> >> #define BTRFS_INO_LOOKUP_USER_PATH_MAX (4080 - BTRFS_VOL_NAME_MAX - 1) >> struct btrfs_ioctl_ino_lookup_user_args { >> @@ -436,6 +450,7 @@ struct btrfs_ioctl_ino_lookup_user_args { >> */ >> char path[BTRFS_INO_LOOKUP_USER_PATH_MAX]; >> }; >> +BTRFS_ASSERT_SIZE(btrfs_ioctl_ino_lookup_args, 4096); >> >> /* Search criteria for the btrfs SEARCH ioctl family. */ >> struct btrfs_ioctl_search_key { >> @@ -664,8 +679,9 @@ struct btrfs_ioctl_get_dev_stats { >> /* out values: */ >> __u64 values[BTRFS_DEV_STAT_VALUES_MAX]; >> >> - __u64 unused[128 - 2 - BTRFS_DEV_STAT_VALUES_MAX]; /* pad to 1k */ >> + __u64 unused[128 - 3 - BTRFS_DEV_STAT_VALUES_MAX]; /* pad to 1k */ >> }; >> +BTRFS_ASSERT_SIZE(btrfs_ioctl_get_dev_stats, 1024); >> >> #define BTRFS_QUOTA_CTL_ENABLE 1 >> #define BTRFS_QUOTA_CTL_DISABLE 2 >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >