* [PATCH v2] btrfs: assert sizes of ioctl structures
@ 2020-07-14 9:32 Johannes Thumshirn
2020-07-14 12:32 ` David Sterba
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Thumshirn @ 2020-07-14 9:32 UTC (permalink / raw)
To: David Sterba; +Cc: linux-btrfs, Johannes Thumshirn
When expanding ioctl interfaces we want to make sure we're not changing
the size of the structures, otherwise it can lead to incorrect transfers
between kernel and user-space.
Build time assert the size of each structure so we're not running into any
incompatibilities.
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
Changes to v1:
- Stub out static_assert() for user-space (0day Bot)
- Sync asserts with the ones from btrfs-progs (David)
---
include/uapi/linux/btrfs.h | 36 ++++++++++++++++++++++++++++++++++--
1 file changed, 34 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
index 69b88f6cb57f..dc7205972f1c 100644
--- a/include/uapi/linux/btrfs.h
+++ b/include/uapi/linux/btrfs.h
@@ -22,6 +22,10 @@
#include <linux/types.h>
#include <linux/ioctl.h>
+#ifndef static_assert
+#define static_assert(x)
+#endif
+
#define BTRFS_IOCTL_MAGIC 0x94
#define BTRFS_VOL_NAME_MAX 255
#define BTRFS_LABEL_SIZE 256
@@ -32,6 +36,7 @@ struct btrfs_ioctl_vol_args {
__s64 fd;
char name[BTRFS_PATH_NAME_MAX + 1];
};
+static_assert(sizeof(struct btrfs_ioctl_vol_args) == 4096);
#define BTRFS_DEVICE_PATH_NAME_MAX 1024
#define BTRFS_SUBVOL_NAME_MAX 4039
@@ -78,6 +83,7 @@ struct btrfs_qgroup_limit {
__u64 rsv_rfer;
__u64 rsv_excl;
};
+static_assert(sizeof(struct btrfs_qgroup_limit) == 40);
/*
* flags definition for qgroup inheritance
@@ -95,11 +101,13 @@ struct btrfs_qgroup_inherit {
struct btrfs_qgroup_limit lim;
__u64 qgroups[0];
};
+static_assert(sizeof(struct btrfs_qgroup_inherit) == 72);
struct btrfs_ioctl_qgroup_limit_args {
__u64 qgroupid;
struct btrfs_qgroup_limit lim;
};
+static_assert(sizeof(struct btrfs_ioctl_qgroup_limit_args) == 48);
/*
* Arguments for specification of subvolumes or devices, supporting by-name or
@@ -142,6 +150,7 @@ struct btrfs_ioctl_vol_args_v2 {
__u64 subvolid;
};
};
+static_assert(sizeof(struct btrfs_ioctl_vol_args_v2) == 4096);
/*
* structure to report errors and progress to userspace, either as a
@@ -190,6 +199,7 @@ struct btrfs_ioctl_scrub_args {
/* pad to 1k */
__u64 unused[(1024-32-sizeof(struct btrfs_scrub_progress))/8];
};
+static_assert(sizeof(struct 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
@@ -200,6 +210,7 @@ struct btrfs_ioctl_dev_replace_start_params {
__u8 srcdev_name[BTRFS_DEVICE_PATH_NAME_MAX + 1]; /* in */
__u8 tgtdev_name[BTRFS_DEVICE_PATH_NAME_MAX + 1]; /* in */
};
+static_assert(sizeof(struct btrfs_ioctl_dev_replace_start_params) == 2072);
#define BTRFS_IOCTL_DEV_REPLACE_STATE_NEVER_STARTED 0
#define BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED 1
@@ -214,6 +225,7 @@ struct btrfs_ioctl_dev_replace_status_params {
__u64 num_write_errors; /* out */
__u64 num_uncorrectable_read_errors; /* out */
};
+static_assert(sizeof(struct btrfs_ioctl_dev_replace_status_params) == 48);
#define BTRFS_IOCTL_DEV_REPLACE_CMD_START 0
#define BTRFS_IOCTL_DEV_REPLACE_CMD_STATUS 1
@@ -233,6 +245,7 @@ struct btrfs_ioctl_dev_replace_args {
__u64 spare[64];
};
+static_assert(sizeof(struct btrfs_ioctl_dev_replace_args) == 2600);
struct btrfs_ioctl_dev_info_args {
__u64 devid; /* in/out */
@@ -242,6 +255,7 @@ struct btrfs_ioctl_dev_info_args {
__u64 unused[379]; /* pad to 4k */
__u8 path[BTRFS_DEVICE_PATH_NAME_MAX]; /* out */
};
+static_assert(sizeof(struct btrfs_ioctl_dev_info_args) == 4096);
/*
* Retrieve information about the filesystem
@@ -270,7 +284,7 @@ struct btrfs_ioctl_fs_info_args {
__u8 metadata_uuid[BTRFS_FSID_SIZE]; /* out */
__u8 reserved[944]; /* pad to 1k */
};
-
+static_assert(sizeof(struct btrfs_ioctl_fs_info_args) == 1024);
/*
* feature flags
@@ -314,6 +328,7 @@ struct btrfs_ioctl_feature_flags {
__u64 compat_ro_flags;
__u64 incompat_flags;
};
+static_assert(sizeof(struct btrfs_ioctl_feature_flags) == 24);
/* balance control ioctl modes */
#define BTRFS_BALANCE_CTL_PAUSE 1
@@ -453,6 +468,7 @@ struct btrfs_ioctl_balance_args {
__u64 unused[72]; /* pad to 1k */
};
+static_assert(sizeof(struct btrfs_ioctl_balance_args) == 1024);
#define BTRFS_INO_LOOKUP_PATH_MAX 4080
struct btrfs_ioctl_ino_lookup_args {
@@ -460,6 +476,7 @@ struct btrfs_ioctl_ino_lookup_args {
__u64 objectid;
char name[BTRFS_INO_LOOKUP_PATH_MAX];
};
+static_assert(sizeof(struct 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 {
@@ -475,6 +492,7 @@ struct btrfs_ioctl_ino_lookup_user_args {
*/
char path[BTRFS_INO_LOOKUP_USER_PATH_MAX];
};
+static_assert(sizeof(struct btrfs_ioctl_ino_lookup_user_args) == 4096);
/* Search criteria for the btrfs SEARCH ioctl family. */
struct btrfs_ioctl_search_key {
@@ -553,6 +571,7 @@ struct btrfs_ioctl_search_args {
struct btrfs_ioctl_search_key key;
char buf[BTRFS_SEARCH_ARGS_BUFSIZE];
};
+static_assert(sizeof(struct btrfs_ioctl_search_args) == 4096);
struct btrfs_ioctl_search_args_v2 {
struct btrfs_ioctl_search_key key; /* in/out - search parameters */
@@ -561,12 +580,14 @@ struct btrfs_ioctl_search_args_v2 {
* to store item */
__u64 buf[0]; /* out - found items */
};
+static_assert(sizeof(struct btrfs_ioctl_search_args_v2) == 112);
struct btrfs_ioctl_clone_range_args {
__s64 src_fd;
__u64 src_offset, src_length;
__u64 dest_offset;
};
+static_assert(sizeof(struct btrfs_ioctl_clone_range_args) == 32);
/*
* flags definition for the defrag range ioctl
@@ -606,7 +627,7 @@ struct btrfs_ioctl_defrag_range_args {
/* spare for later */
__u32 unused[4];
};
-
+static_assert(sizeof(struct btrfs_ioctl_defrag_range_args) == 48);
#define BTRFS_SAME_DATA_DIFFERS 1
/* For extent-same ioctl */
@@ -632,6 +653,7 @@ struct btrfs_ioctl_same_args {
__u32 reserved2;
struct btrfs_ioctl_same_extent_info info[0];
};
+static_assert(sizeof(struct btrfs_ioctl_same_args) == 24);
struct btrfs_ioctl_space_info {
__u64 flags;
@@ -644,6 +666,7 @@ struct btrfs_ioctl_space_args {
__u64 total_spaces;
struct btrfs_ioctl_space_info spaces[0];
};
+static_assert(sizeof(struct btrfs_ioctl_space_args) == 16);
struct btrfs_data_container {
__u32 bytes_left; /* out -- bytes not needed to deliver output */
@@ -660,6 +683,7 @@ struct btrfs_ioctl_ino_path_args {
/* struct btrfs_data_container *fspath; out */
__u64 fspath; /* out */
};
+static_assert(sizeof(struct btrfs_ioctl_ino_path_args) == 56);
struct btrfs_ioctl_logical_ino_args {
__u64 logical; /* in */
@@ -710,6 +734,7 @@ struct btrfs_ioctl_get_dev_stats {
*/
__u64 unused[128 - 2 - BTRFS_DEV_STAT_VALUES_MAX];
};
+static_assert(sizeof(struct btrfs_ioctl_get_dev_stats) == 1032);
#define BTRFS_QUOTA_CTL_ENABLE 1
#define BTRFS_QUOTA_CTL_DISABLE 2
@@ -718,12 +743,14 @@ struct btrfs_ioctl_quota_ctl_args {
__u64 cmd;
__u64 status;
};
+static_assert(sizeof(struct btrfs_ioctl_quota_ctl_args) == 16);
struct btrfs_ioctl_quota_rescan_args {
__u64 flags;
__u64 progress;
__u64 reserved[6];
};
+static_assert(sizeof(struct btrfs_ioctl_quota_rescan_args) == 64);
struct btrfs_ioctl_qgroup_assign_args {
__u64 assign;
@@ -735,6 +762,8 @@ struct btrfs_ioctl_qgroup_create_args {
__u64 create;
__u64 qgroupid;
};
+static_assert(sizeof(struct btrfs_ioctl_qgroup_create_args) == 16);
+
struct btrfs_ioctl_timespec {
__u64 sec;
__u32 nsec;
@@ -749,6 +778,7 @@ struct btrfs_ioctl_received_subvol_args {
__u64 flags; /* in */
__u64 reserved[16]; /* in */
};
+static_assert(sizeof(struct btrfs_ioctl_received_subvol_args) == 200);
/*
* Caller doesn't want file data in the send stream, even if the
@@ -783,6 +813,7 @@ struct btrfs_ioctl_send_args {
__u64 flags; /* in */
__u64 reserved[4]; /* in */
};
+static_assert(sizeof(struct btrfs_ioctl_send_args) == 72);
/*
* Information about a fs tree root.
@@ -859,6 +890,7 @@ struct btrfs_ioctl_get_subvol_rootref_args {
__u8 num_items;
__u8 align[7];
};
+static_assert(sizeof(struct btrfs_ioctl_get_subvol_rootref_args) == 4096);
/* Error codes as returned by the kernel */
enum btrfs_err_code {
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] btrfs: assert sizes of ioctl structures
2020-07-14 9:32 [PATCH v2] btrfs: assert sizes of ioctl structures Johannes Thumshirn
@ 2020-07-14 12:32 ` David Sterba
2020-07-14 14:49 ` Johannes Thumshirn
0 siblings, 1 reply; 6+ messages in thread
From: David Sterba @ 2020-07-14 12:32 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: David Sterba, linux-btrfs
On Tue, Jul 14, 2020 at 06:32:36PM +0900, Johannes Thumshirn wrote:
> When expanding ioctl interfaces we want to make sure we're not changing
> the size of the structures, otherwise it can lead to incorrect transfers
> between kernel and user-space.
>
> Build time assert the size of each structure so we're not running into any
> incompatibilities.
>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
I've tried 32bit build and the assertion fails for many structures, but
I was expecting only the send one because it contains the pointer.
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
CC [M] fs/btrfs/super.o
In file included from ./include/linux/bits.h:22,
from ./include/linux/bitops.h:5,
from ./include/linux/kernel.h:12,
from ./arch/x86/include/asm/percpu.h:45,
from ./arch/x86/include/asm/current.h:6,
from ./include/linux/sched.h:12,
from ./include/linux/blkdev.h:5,
from fs/btrfs/super.c:6:
./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct btrfs_ioctl_dev_replace_start_params) == 2072"
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~~~~~~~~~~~
./include/linux/build_bug.h:77:34: note: in expansion of macro ‘__static_assert’
77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
| ^~~~~~~~~~~~~~~
./include/uapi/linux/btrfs.h:213:1: note: in expansion of macro ‘static_assert’
213 | static_assert(sizeof(struct btrfs_ioctl_dev_replace_start_params) == 2072);
| ^~~~~~~~~~~~~
./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct btrfs_ioctl_dev_replace_args) == 2600"
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~~~~~~~~~~~
./include/linux/build_bug.h:77:34: note: in expansion of macro ‘__static_assert’
77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
| ^~~~~~~~~~~~~~~
./include/uapi/linux/btrfs.h:248:1: note: in expansion of macro ‘static_assert’
248 | static_assert(sizeof(struct btrfs_ioctl_dev_replace_args) == 2600);
| ^~~~~~~~~~~~~
./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct btrfs_ioctl_received_subvol_args) == 200"
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~~~~~~~~~~~
./include/linux/build_bug.h:77:34: note: in expansion of macro ‘__static_assert’
77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
| ^~~~~~~~~~~~~~~
./include/uapi/linux/btrfs.h:781:1: note: in expansion of macro ‘static_assert’
781 | static_assert(sizeof(struct btrfs_ioctl_received_subvol_args) == 200);
| ^~~~~~~~~~~~~
./include/linux/build_bug.h:78:41: error: static assertion failed: "sizeof(struct btrfs_ioctl_send_args) == 72"
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~~~~~~~~~~~
./include/linux/build_bug.h:77:34: note: in expansion of macro ‘__static_assert’
77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
| ^~~~~~~~~~~~~~~
./include/uapi/linux/btrfs.h:816:1: note: in expansion of macro ‘static_assert’
816 | static_assert(sizeof(struct btrfs_ioctl_send_args) == 72);
| ^~~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:281: fs/btrfs/super.o] Error 1
make[1]: *** [scripts/Makefile.build:497: fs/btrfs] Error 2
make: *** [Makefile:1756: fs] Error 2
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] btrfs: assert sizes of ioctl structures
2020-07-14 12:32 ` David Sterba
@ 2020-07-14 14:49 ` Johannes Thumshirn
2020-07-14 14:55 ` David Sterba
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Thumshirn @ 2020-07-14 14:49 UTC (permalink / raw)
To: dsterba; +Cc: linux-btrfs
On 14/07/2020 14:33, David Sterba wrote:
> On Tue, Jul 14, 2020 at 06:32:36PM +0900, Johannes Thumshirn wrote:
>> When expanding ioctl interfaces we want to make sure we're not changing
>> the size of the structures, otherwise it can lead to incorrect transfers
>> between kernel and user-space.
>>
>> Build time assert the size of each structure so we're not running into any
>> incompatibilities.
>>
>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>
> I've tried 32bit build and the assertion fails for many structures, but
> I was expecting only the send one because it contains the pointer.
I wonder if we should have two different asserts for 32 and 64bit for
these structures or remove the asserts from them.
Having a 32 and 64bit assert will add some ifdeffery, let me see how
ugly this will get.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] btrfs: assert sizes of ioctl structures
2020-07-14 14:49 ` Johannes Thumshirn
@ 2020-07-14 14:55 ` David Sterba
2020-07-15 7:12 ` Johannes Thumshirn
0 siblings, 1 reply; 6+ messages in thread
From: David Sterba @ 2020-07-14 14:55 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: linux-btrfs
On Tue, Jul 14, 2020 at 02:49:31PM +0000, Johannes Thumshirn wrote:
> On 14/07/2020 14:33, David Sterba wrote:
> > On Tue, Jul 14, 2020 at 06:32:36PM +0900, Johannes Thumshirn wrote:
> >> When expanding ioctl interfaces we want to make sure we're not changing
> >> the size of the structures, otherwise it can lead to incorrect transfers
> >> between kernel and user-space.
> >>
> >> Build time assert the size of each structure so we're not running into any
> >> incompatibilities.
> >>
> >> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> >
> > I've tried 32bit build and the assertion fails for many structures, but
> > I was expecting only the send one because it contains the pointer.
>
> I wonder if we should have two different asserts for 32 and 64bit for
> these structures or remove the asserts from them.
>
> Having a 32 and 64bit assert will add some ifdeffery, let me see how
> ugly this will get.
Progs do the switch using sizeof(long) and ?: operator but I don't know
if this works with _Static_assert as progs use the struct + bitfield
way.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] btrfs: assert sizes of ioctl structures
2020-07-14 14:55 ` David Sterba
@ 2020-07-15 7:12 ` Johannes Thumshirn
2020-07-15 7:56 ` David Sterba
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Thumshirn @ 2020-07-15 7:12 UTC (permalink / raw)
To: dsterba; +Cc: linux-btrfs
On 14/07/2020 16:56, David Sterba wrote:
> On Tue, Jul 14, 2020 at 02:49:31PM +0000, Johannes Thumshirn wrote:
>> On 14/07/2020 14:33, David Sterba wrote:
>>> On Tue, Jul 14, 2020 at 06:32:36PM +0900, Johannes Thumshirn wrote:
>>>> When expanding ioctl interfaces we want to make sure we're not changing
>>>> the size of the structures, otherwise it can lead to incorrect transfers
>>>> between kernel and user-space.
>>>>
>>>> Build time assert the size of each structure so we're not running into any
>>>> incompatibilities.
>>>>
>>>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>>>
>>> I've tried 32bit build and the assertion fails for many structures, but
>>> I was expecting only the send one because it contains the pointer.
>>
>> I wonder if we should have two different asserts for 32 and 64bit for
>> these structures or remove the asserts from them.
>>
>> Having a 32 and 64bit assert will add some ifdeffery, let me see how
>> ugly this will get.
>
> Progs do the switch using sizeof(long) and ?: operator but I don't know
> if this works with _Static_assert as progs use the struct + bitfield
> way.
>
I can try but it's ugly as hell IMHO
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] btrfs: assert sizes of ioctl structures
2020-07-15 7:12 ` Johannes Thumshirn
@ 2020-07-15 7:56 ` David Sterba
0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2020-07-15 7:56 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: dsterba, linux-btrfs
On Wed, Jul 15, 2020 at 07:12:09AM +0000, Johannes Thumshirn wrote:
> On 14/07/2020 16:56, David Sterba wrote:
> > On Tue, Jul 14, 2020 at 02:49:31PM +0000, Johannes Thumshirn wrote:
> >> On 14/07/2020 14:33, David Sterba wrote:
> >>> On Tue, Jul 14, 2020 at 06:32:36PM +0900, Johannes Thumshirn wrote:
> >>>> When expanding ioctl interfaces we want to make sure we're not changing
> >>>> the size of the structures, otherwise it can lead to incorrect transfers
> >>>> between kernel and user-space.
> >>>>
> >>>> Build time assert the size of each structure so we're not running into any
> >>>> incompatibilities.
> >>>>
> >>>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> >>>
> >>> I've tried 32bit build and the assertion fails for many structures, but
> >>> I was expecting only the send one because it contains the pointer.
> >>
> >> I wonder if we should have two different asserts for 32 and 64bit for
> >> these structures or remove the asserts from them.
> >>
> >> Having a 32 and 64bit assert will add some ifdeffery, let me see how
> >> ugly this will get.
> >
> > Progs do the switch using sizeof(long) and ?: operator but I don't know
> > if this works with _Static_assert as progs use the struct + bitfield
> > way.
> >
>
> I can try but it's ugly as hell IMHO
Or we can add macros like
ASSERT_STRUCT_SIZE(struct name, 64);
ASSERT_STRUCT_SIZE_32_64(struct name, 32, 64);
and then do the ifdefs at the definition time.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-15 7:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-14 9:32 [PATCH v2] btrfs: assert sizes of ioctl structures Johannes Thumshirn
2020-07-14 12:32 ` David Sterba
2020-07-14 14:49 ` Johannes Thumshirn
2020-07-14 14:55 ` David Sterba
2020-07-15 7:12 ` Johannes Thumshirn
2020-07-15 7:56 ` David Sterba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.