diff --git a/kmod/src/ioctl.c b/kmod/src/ioctl.c index 4818ba98..e5e7f5a7 100644 --- a/kmod/src/ioctl.c +++ b/kmod/src/ioctl.c @@ -500,11 +500,14 @@ static long scoutfs_ioc_item_cache_keys(struct file *file, unsigned long arg) { struct super_block *sb = file_inode(file)->i_sb; struct scoutfs_ioctl_item_cache_keys ick; - struct scoutfs_key __user *ukeys; + struct scoutfs_ioctl_key __user *ukeys; + struct scoutfs_ioctl_key ikeys[16]; struct scoutfs_key keys[16]; + struct scoutfs_key key; unsigned int nr; int total; int ret; + int i; if (copy_from_user(&ick, (void __user *)arg, sizeof(ick))) return -EFAULT; @@ -512,6 +515,8 @@ static long scoutfs_ioc_item_cache_keys(struct file *file, unsigned long arg) if (ick.which > SCOUTFS_IOC_ITEM_CACHE_KEYS_RANGES) return -EINVAL; + scoutfs_key_copy_types(&key, &ick.ikey); + ukeys = (void __user *)(long)ick.buf_ptr; total = 0; ret = 0; @@ -519,21 +524,23 @@ static long scoutfs_ioc_item_cache_keys(struct file *file, unsigned long arg) nr = min_t(size_t, ick.buf_nr, ARRAY_SIZE(keys)); if (ick.which == SCOUTFS_IOC_ITEM_CACHE_KEYS_ITEMS) - ret = scoutfs_item_copy_keys(sb, &ick.key, keys, nr); + ret = scoutfs_item_copy_keys(sb, &key, keys, nr); else - ret = scoutfs_item_copy_range_keys(sb, &ick.key, keys, - nr); + ret = scoutfs_item_copy_range_keys(sb, &key, keys, nr); BUG_ON(ret > nr); /* stack overflow \o/ */ if (ret <= 0) break; - if (copy_to_user(ukeys, keys, ret * sizeof(keys[0]))) { + for (i = 0; i < ret; i++) + scoutfs_key_copy_types(&ikeys[i], &keys[i]); + + if (copy_to_user(ukeys, ikeys, ret * sizeof(ikeys[0]))) { ret = -EFAULT; break; } - ick.key = keys[ret - 1]; - scoutfs_key_inc(&ick.key); + key = keys[ret - 1]; + scoutfs_key_inc(&key); ukeys += ret; ick.buf_nr -= ret; @@ -668,8 +675,8 @@ static long scoutfs_ioc_setattr_more(struct file *file, unsigned long arg) scoutfs_inode_set_data_version(inode, sm.data_version); if (sm.i_size) i_size_write(inode, sm.i_size); - inode->i_ctime.tv_sec = le64_to_cpu(sm.ctime.sec); - inode->i_ctime.tv_nsec = le32_to_cpu(sm.ctime.nsec); + inode->i_ctime.tv_sec = sm.ctime_sec; + inode->i_ctime.tv_nsec = sm.ctime_nsec; scoutfs_update_inode_item(inode, lock, &ind_locks); ret = 0; diff --git a/kmod/src/ioctl.h b/kmod/src/ioctl.h index b9966c68..0397b634 100644 --- a/kmod/src/ioctl.h +++ b/kmod/src/ioctl.h @@ -1,14 +1,41 @@ #ifndef _SCOUTFS_IOCTL_H_ #define _SCOUTFS_IOCTL_H_ +/* + * We naturally align explicit width fields in the ioctl structs so that + * userspace doesn't need to deal with padding or unaligned packing and + * we don't have to deal with 32/64 compat. It makes it a little + * awkward to communicate persistent packed structs through the ioctls + * but that happens very rarely. An interesting special case are + * 0length arrays that follow the structs. We make those start at the + * next aligned offset of the struct to be safe. + * + * This is enforced by pahole scripting in external build environments. + */ + /* XXX I have no idea how these are chosen. */ #define SCOUTFS_IOCTL_MAGIC 's' +/* + * Packed scoutfs keys rarely cross the ioctl boundary so we have a + * translation struct. + */ +struct scoutfs_ioctl_key { + __le64 _sk_first; + __le64 _sk_second; + __le64 _sk_third; + __u8 _sk_fourth; + __u8 sk_type; + __u8 sk_zone; + __u8 _pad[5]; +}; + struct scoutfs_ioctl_walk_inodes_entry { __u64 major; - __u32 minor; __u64 ino; -} __packed; + __u32 minor; + __u8 _pad[4]; +}; /* * Walk inodes in an index that is sorted by one of their fields. @@ -48,7 +75,8 @@ struct scoutfs_ioctl_walk_inodes { __u64 entries_ptr; __u32 nr_entries; __u8 index; -} __packed; + __u8 _pad[11]; /* padded to align walk_inodes_entry total size */ +}; enum { SCOUTFS_IOC_WALK_INODES_META_SEQ = 0, @@ -127,14 +155,16 @@ struct scoutfs_ioctl_ino_path { __u64 dir_pos; __u64 result_ptr; __u16 result_bytes; -} __packed; + __u8 _pad[6]; +}; struct scoutfs_ioctl_ino_path_result { __u64 dir_ino; __u64 dir_pos; __u16 path_bytes; + __u8 _pad[6]; __u8 path[0]; -} __packed; +}; /* Get a single path from the root to the given inode number */ #define SCOUTFS_IOC_INO_PATH _IOW(SCOUTFS_IOCTL_MAGIC, 2, \ @@ -168,7 +198,7 @@ struct scoutfs_ioctl_release { __u64 block; __u64 count; __u64 data_version; -} __packed; +}; #define SCOUTFS_IOC_RELEASE _IOW(SCOUTFS_IOCTL_MAGIC, 5, \ struct scoutfs_ioctl_release) @@ -178,7 +208,8 @@ struct scoutfs_ioctl_stage { __u64 buf_ptr; __u64 offset; __s32 count; -} __packed; + __u32 _pad; +}; #define SCOUTFS_IOC_STAGE _IOW(SCOUTFS_IOCTL_MAGIC, 6, \ struct scoutfs_ioctl_stage) @@ -203,11 +234,12 @@ struct scoutfs_ioctl_stat_more { __u64 data_version; __u64 online_blocks; __u64 offline_blocks; -} __packed; +}; #define SCOUTFS_IOC_STAT_MORE _IOW(SCOUTFS_IOCTL_MAGIC, 7, \ struct scoutfs_ioctl_stat_more) + /* * Fills the buffer with either the keys for the cached items or the * keys for the cached ranges found starting with the given key. The @@ -215,11 +247,12 @@ struct scoutfs_ioctl_stat_more { * keys the returned number will always be a multiple of two. */ struct scoutfs_ioctl_item_cache_keys { - struct scoutfs_key key; + struct scoutfs_ioctl_key ikey; __u64 buf_ptr; __u16 buf_nr; __u8 which; -} __packed; + __u8 _pad[21]; /* padded to align _ioctl_key total size */ +}; enum { SCOUTFS_IOC_ITEM_CACHE_KEYS_ITEMS = 0, @@ -233,7 +266,8 @@ struct scoutfs_ioctl_data_waiting_entry { __u64 ino; __u64 iblock; __u8 op; -} __packed; + __u8 _pad[7]; +}; #define SCOUTFS_IOC_DWO_READ (1 << 0) #define SCOUTFS_IOC_DWO_WRITE (1 << 1) @@ -246,7 +280,8 @@ struct scoutfs_ioctl_data_waiting { __u64 after_iblock; __u64 ents_ptr; __u16 ents_nr; -} __packed; + __u8 _pad[6]; +}; #define SCOUTFS_IOC_DATA_WAITING_FLAGS_UNKNOWN (U8_MAX << 0) @@ -262,8 +297,10 @@ struct scoutfs_ioctl_setattr_more { __u64 data_version; __u64 i_size; __u64 flags; - struct scoutfs_timespec ctime; -} __packed; + __u64 ctime_sec; + __u32 ctime_nsec; + __u8 _pad[4]; +}; #define SCOUTFS_IOC_SETATTR_MORE_OFFLINE (1 << 0) #define SCOUTFS_IOC_SETATTR_MORE_UNKNOWN (U8_MAX << 1) @@ -276,7 +313,7 @@ struct scoutfs_ioctl_listxattr_raw { __u64 buf_ptr; __u32 buf_bytes; __u32 hash_pos; -} __packed; +}; #define SCOUTFS_IOC_LISTXATTR_RAW _IOW(SCOUTFS_IOCTL_MAGIC, 11, \ struct scoutfs_ioctl_listxattr_raw) @@ -306,7 +343,8 @@ struct scoutfs_ioctl_find_xattrs { __u64 inodes_ptr; __u16 name_bytes; __u16 nr_inodes; -} __packed; + __u8 _pad[4]; +}; #define SCOUTFS_IOC_FIND_XATTRS _IOW(SCOUTFS_IOCTL_MAGIC, 12, \ struct scoutfs_ioctl_find_xattrs) diff --git a/kmod/src/key.h b/kmod/src/key.h index abe89ee0..7709438d 100644 --- a/kmod/src/key.h +++ b/kmod/src/key.h @@ -52,6 +52,24 @@ do { \ __entry->name##_zone, __entry->name##_first, __entry->name##_type, \ __entry->name##_second, __entry->name##_third, __entry->name##_fourth +/* + * copy fields between keys with the same fields but different types. + * The destination type might have internal padding so we zero it. + */ +#define scoutfs_key_copy_types(a, b) \ +do { \ + __typeof__(a) _to = (a); \ + __typeof__(b) _from = (b); \ + \ + memset(_to, 0, sizeof(*_to)); \ + _to->sk_zone = _from->sk_zone; \ + _to->_sk_first = _from->_sk_first; \ + _to->sk_type = _from->sk_type; \ + _to->_sk_second = _from->_sk_second; \ + _to->_sk_third = _from->_sk_third; \ + _to->_sk_fourth = _from->_sk_fourth; \ +} while (0) + static inline void scoutfs_key_set_zeros(struct scoutfs_key *key) { key->sk_zone = 0;