Le 23/08/2020 à 21:59, Filip Bozuta a écrit :
> This patch introduces functionality for following btrfs ioctls:
>
> BTRFS_IOC_SUBVOL_CREATE_V2 - Adding a new btrfs subvolume
>
> Create a new btrfs subvolume (same as with BTRFS_IOC_SUBVOL_CREATE).
> The third ioctl's argument is a pointer to a following type:
>
> struct btrfs_ioctl_vol_args_v2 {
> __s64 fd;
> __u64 transid;
> __u64 flags;
> union {
> struct {
> __u64 size;
> struct btrfs_qgroup_inherit __user *qgroup_inherit;
> };
> __u64 unused[4];
> };
> union {
> char name[BTRFS_SUBVOL_NAME_MAX + 1];
> __u64 devid;
> __u64 subvolid; /* added in kernel version 5.8 */
> };
> };
>
> When calling this ioctl, the 'name' field should be filled with
> the aproppriate value that contains the name of the subvolume that
> is to be created. The flags field can take values that are
> 'BTRFS_SUBVOL_RDONLY' or 'BTRFS_SUBVOL_QGROUP_INHERIT'. If the
> latter is specified, the field 'qgroup_inherit' should be filled
> with aproppriate values of the quota group in which the newly
> created subvolume is to be added. The definition of
> 'struct btrfs_qgroup_inherit' can be found at:
>
> https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/btrfs.h#L90
>
> BTRFS_IOC_SNAP_CREATE_V2 - Adding a btrfs snapshot for a subvolume
>
> Create a new btrfs snapshot for a specified subvolume (same as with
> BTRFS_IOC_SNAP_CREATE). The ioctl's third argument is a pointer to
> the above mentioned 'struct btrfs_ioctl_vol_args_v2'. Before calling
> this ioctl, field 'fd' should be filled with the aproppriate file
> descriptor value for the btrfs subvolume for which the snapshot is
> to be created. Also, the 'name' field should be filled with the
> aproppriate value that represents the name of the snapshot that is
> to be created. The 'flags' field takes the same values as in case
> of 'BTRFS_IOC_SUBVOL_CREATE_V2' and represents the same functionality.
>
> BTRFS_IOC_RM_DEV_V2 - Removing a btrfs device
>
> Delete a btrfs device (same as with BTRFS_IOC_RM_DEV). The ioctl's third
> argument is a pointer to the above mentioned 'struct
> btrfs_ioctl_vol_args_v2'.
> Before calling this ioctl, either the 'name' or the 'devid' field should
> be filled with the name or id of the device that is to be removed. Also,
> the
> flags field should be filled either with 0 or 'BTRFS_DEVICE_SPEC_BY_ID'
> depending on if the device is gonna be specified via name or id.
>
> BTRFS_IOC_SNAP_DESTROY_V2 - Removing a btrfs snapshot
>
> Remove a btrfs snapshot (same as with BTRFS_IOC_SNAP_DESTROY). The ioctl's
> third argument is a pointer to the above mentioned 'struct
> btrfs_ioctl_vol_args_v2'.
> Before calling this ioctl, either the 'name' or the 'subvolid' field
> should
> be filled with the name or id of the snapshot that is to be removed.
> Also, the
> flags field should be filled either with 0 or 'BTRFS_SUBVOL_SPEC_BY_ID'
> depending on if the snapshot is gonna be specified via name or id.
>
> Implementation notes:
>
> Since the third argument of the implemented ioctl's is a
> structure that contains unions, a special converting function
> 'target_to_host_btrfs_ioctl_vol_args_v2' was defined in 'syscall.c'.
> This function is called instead of 'thunk_convert()' to convert
> the values of the third argument from target to host. All of
> the ioctls in this patch are of type 'IOW' which is why a converting
> function from host to target is not required. Also, a separate printing
> function was defined in file 'strace.c' that is called instead of
> 'thunk_print()' to print the contents of the third argument.
>
> Signed-off-by: Filip Bozuta <filip.boz...@syrmia.com>
> Based-on: <20200811164553.27713-2-filip.boz...@syrmia.com>
> Based-on: <20200723210233.349690-4-filip.boz...@syrmia.com>
> ---
> linux-user/ioctls.h | 16 +++++++
> linux-user/qemu.h | 5 ++
> linux-user/strace.c | 95 ++++++++++++++++++++++++++++++++++++++
> linux-user/syscall.c | 77 ++++++++++++++++++++++++++++++
> linux-user/syscall_defs.h | 28 +++++++++++
> linux-user/syscall_types.h | 5 ++
> thunk.c | 2 +-
> 7 files changed, 227 insertions(+), 1 deletion(-)
>
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index a69a0bd347..4add164673 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -708,6 +708,11 @@ static inline uint64_t target_offset64(uint64_t word0,
> uint64_t word1)
>
> void print_termios(void *arg);
>
> +#if defined(BTRFS_IOC_SUBVOL_CREATE_V2) || defined(BTRFS_IOC_SNAP_CREATE_V2)
> \
> + || defined(BTRFS_IOC_SNAP_DESTROY_V2) || defined(BTRFS_IOC_RM_DEV_V2)
The #if is not needed on the declaration of the symbol. It can be unused
or not defined, there will be no error.
> +void print_btrfs_ioctl_vol_args_v2(void *arg);
> +#endif
> +
> /* ARM EABI and MIPS expect 64bit types aligned even on pairs or registers */
> #ifdef TARGET_ARM
> static inline int regpairs_aligned(void *cpu_env, int num)
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index b9ba39ce6e..04fe38a846 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -9,6 +9,9 @@
> #include <netinet/tcp.h>
> #include <linux/if_packet.h>
> #include <linux/netlink.h>
> +#ifdef CONFIG_BTRFS
> +#include <linux/btrfs.h>
> +#endif
> #include <sched.h>
> #include "qemu.h"
>
...
> @@ -1774,6 +1793,82 @@ print_termios(void *arg)
> qemu_log("}");
> }
>
> +#if defined(BTRFS_IOC_SUBVOL_CREATE_V2) || defined(BTRFS_IOC_SNAP_CREATE_V2)
> \
> + || defined(BTRFS_IOC_SNAP_DESTROY_V2) || defined(BTRFS_IOC_RM_DEV_V2)
> +void
> +print_btrfs_ioctl_vol_args_v2(void *arg)
> +{
> + struct target_btrfs_ioctl_vol_args_v2 *target_args_v2 = arg;
> + uint64_t flags = tswap64(target_args_v2->flags);
> + int device_spec_by_id = 0;
> + int subvol_spec_by_id = 0;
> +
> + qemu_log("{fd = %" PRId64 ", flags = ", tswap64(target_args_v2->fd));
> + print_flags(btrfs_args_vol_v2_flags, flags, 0);
> + qemu_log(" transid = %" PRIu64 ",", tswap64(target_args_v2->transid));
> +
> +#ifdef BTRFS_SUBVOL_QGROUP_INHERIT
> + if (flags & BTRFS_SUBVOL_QGROUP_INHERIT) {
> +
> + struct btrfs_qgroup_inherit *target_inherit;
> + abi_long inherit_addr = tswapal(target_args_v2->qgroup_inherit);
> +
> + target_inherit = lock_user(VERIFY_READ, inherit_addr,
> + sizeof(*target_inherit), 1);
> +
> + if (target_inherit) {
> + qemu_log(" size = %" PRIu64,
> + tswap64(target_args_v2->size));
> + qemu_log(", qgroup_inherit = {");
> +
> + qemu_log("flags = %" PRIu64,
> + tswap64(target_inherit->flags));
> + qemu_log(", num_qgroups = %" PRIu64,
> + tswap64(target_inherit->num_qgroups));
> + qemu_log(", num_ref_copies = %" PRIu64,
> + tswap64(target_inherit->num_ref_copies));
> + qemu_log(", num_excl_copies = %" PRIu64,
> + tswap64(target_inherit->num_excl_copies));
> +
> + qemu_log(", lim = {");
> + qemu_log("lim.flags = %" PRIu64,
> + tswap64(target_inherit->lim.flags));
> + qemu_log(", lim.max_rfer = %" PRIu64,
> + tswap64(target_inherit->lim.max_rfer));
> + qemu_log(", lim.max_excl = %" PRIu64,
> + tswap64(target_inherit->lim.max_excl));
> + qemu_log(", lim.rsv_rfer = %" PRIu64,
> + tswap64(target_inherit->lim.rsv_rfer));
> + qemu_log(", lim.rsv_excl = %" PRIu64 "}",
> + tswap64(target_inherit->lim.rsv_excl));
> +
> + qemu_log(", ...}, ");
> + }
> +
> + unlock_user(target_inherit, inherit_addr, 0);
> + }
> +#endif
> +
> +#ifdef BTRFS_DEVICE_SPEC_BY_ID
> + if (flags & BTRFS_DEVICE_SPEC_BY_ID) {
> + qemu_log(" devid = %" PRIu64, tswap64(target_args_v2->devid));
> + device_spec_by_id = 1;
> + }
> +#endif
> +#ifdef BTRFS_SUBVOL_SPEC_BY_ID
> + if (flags & BTRFS_SUBVOL_SPEC_BY_ID) {
> + qemu_log(" subvolid = %" PRIu64, tswap64(target_args_v2->subvolid));
> + subvol_spec_by_id = 1;
> + }
> +#endif
> + if (!device_spec_by_id && !subvol_spec_by_id) {
> + qemu_log(" name = \"%s\"", target_args_v2->name);
If the name is not NUL terminated we can have an overflow here.
> + }
> +
> + qemu_log("}");
> +}
> +#endif
> +
> #undef UNUSED
>
> #ifdef TARGET_NR_accept
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index b58098b5bb..78d7106b80 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -5668,6 +5668,83 @@ static const StructEntry struct_termios_def = {
> .print = print_termios,
> };
>
> +#if defined(BTRFS_IOC_SUBVOL_CREATE_V2) || defined(BTRFS_IOC_SNAP_CREATE_V2)
> \
> + || defined(BTRFS_SNAP_DESTROY_V2) || defined(BTRFS_RM_DEV_V2)
> +static void target_to_host_btrfs_ioctl_vol_args_v2(void *dst, const void
> *src)
> +{
> + struct btrfs_ioctl_vol_args_v2 *host_args_v2 = dst;
> + const struct target_btrfs_ioctl_vol_args_v2 *target_args_v2 = src;
> + int device_spec_by_id = 0;
> + int subvol_spec_by_id = 0;
> +
> + __get_user(host_args_v2->fd, &target_args_v2->fd);
> + __get_user(host_args_v2->flags, &target_args_v2->flags);
> + __get_user(host_args_v2->transid, &target_args_v2->transid);
> +
> + if (host_args_v2->flags & BTRFS_SUBVOL_QGROUP_INHERIT) {
> + struct btrfs_qgroup_inherit *target_inherit;
> + abi_long inherit_addr = tswapal(target_args_v2->qgroup_inherit);
> +
> + target_inherit = lock_user(VERIFY_READ, inherit_addr,
> + sizeof(*target_inherit), 1);
> +
> + host_args_v2->qgroup_inherit = g_new(struct btrfs_qgroup_inherit, 1);
Where do you free this memory?
And the size is in fact variable and given by size in
btrfs_ioctl_vol_args_v2.
> +
> + if (!target_inherit) {
> + host_args_v2->qgroup_inherit = NULL;
And you overwrite the pointer here
> + } else {
> + __get_user(host_args_v2->qgroup_inherit->flags,
> + &target_inherit->flags);
> + __get_user(host_args_v2->qgroup_inherit->num_qgroups,
> + &target_inherit->num_qgroups);
> + __get_user(host_args_v2->qgroup_inherit->num_ref_copies,
> + &target_inherit->num_ref_copies);
> + __get_user(host_args_v2->qgroup_inherit->num_excl_copies,
> + &target_inherit->num_excl_copies);
> + __get_user(host_args_v2->qgroup_inherit->lim.flags,
> + &target_inherit->lim.flags);
> + __get_user(host_args_v2->qgroup_inherit->lim.max_rfer,
> + &target_inherit->lim.max_rfer);
> + __get_user(host_args_v2->qgroup_inherit->lim.max_excl,
> + &target_inherit->lim.max_excl);
> + __get_user(host_args_v2->qgroup_inherit->lim.rsv_rfer,
> + &target_inherit->lim.rsv_rfer);
> + __get_user(host_args_v2->qgroup_inherit->lim.rsv_excl,
> + &target_inherit->lim.rsv_excl);
And what about qgroups?
> + }
> +
> + unlock_user(target_inherit, inherit_addr, 0);
> + }
> +
> +#ifdef BTRFS_DEVICE_SPEC_BY_ID
> + if (host_args_v2->flags & BTRFS_DEVICE_SPEC_BY_ID) {
> + __get_user(host_args_v2->devid, &target_args_v2->devid);
> + device_spec_by_id = 1;
> + }
> +#endif
> +#ifdef BTRFS_SUBVOL_SPEC_BY_ID
> + if (host_args_v2->flags & BTRFS_SUBVOL_SPEC_BY_ID) {
> + __get_user(host_args_v2->subvolid, &target_args_v2->subvolid);
> + subvol_spec_by_id = 1;
> + }
> +#endif
> +
> + if (!device_spec_by_id & !subvol_spec_by_id) {
> + memcpy(host_args_v2->name, target_args_v2->name,
> + BTRFS_SUBVOL_NAME_MAX + 1);
> + }
> +}
> +
> +static const StructEntry struct_btrfs_ioctl_vol_args_v2_def = {
> + .convert = { NULL, target_to_host_btrfs_ioctl_vol_args_v2},
> + .print = print_btrfs_ioctl_vol_args_v2,
> + .align = { __alignof__(struct target_btrfs_ioctl_vol_args_v2),
> + __alignof__(struct btrfs_ioctl_vol_args_v2) },
> + .size = { sizeof(struct target_btrfs_ioctl_vol_args_v2),
> + sizeof(struct btrfs_ioctl_vol_args_v2) },
> +};
> +#endif
> +
> static bitmask_transtbl mmap_flags_tbl[] = {
> { TARGET_MAP_SHARED, TARGET_MAP_SHARED, MAP_SHARED, MAP_SHARED },
> { TARGET_MAP_PRIVATE, TARGET_MAP_PRIVATE, MAP_PRIVATE, MAP_PRIVATE },
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 969377d622..d2d4d1e1cb 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -967,6 +967,26 @@ struct target_rtc_pll_info {
> #define TARGET_FS_IOC32_GETVERSION TARGET_IOR('v', 1, int)
> #define TARGET_FS_IOC32_SETVERSION TARGET_IOW('v', 2, int)
>
> +#define TARGET_BTRFS_SUBVOL_NAME_MAX 4039
As you use BTRFS_SUBVOL_NAME_MAX in memcpy() of
target_to_host_btrfs_ioctl_vol_args_v2, I think you don't need to define
the TARGET_ variant (the value is the same for all the targets).
> +
> +struct target_btrfs_ioctl_vol_args_v2 {
> + uint64_t fd;
fd is signed in btrfs_ioctl_vol_args_v2
> + uint64_t transid;
> + uint64_t flags;
> + union {
> + struct {
> + uint64_t size;
> + abi_long qgroup_inherit;
> + };
> + uint64_t unused[4];
> + };
> + union {
> + char name[TARGET_BTRFS_SUBVOL_NAME_MAX + 1];
> + uint64_t devid;
> + uint64_t subvolid;
> + };
> +};
> +
> /* btrfs ioctls */
> #define TARGET_BTRFS_IOC_SNAP_CREATE
> TARGET_IOWU(BTRFS_IOCTL_MAGIC, 1)
> #define TARGET_BTRFS_IOC_SCAN_DEV
> TARGET_IOWU(BTRFS_IOCTL_MAGIC, 4)
Thanks,
Laurent
