On Tue, Aug 29, 2023, at 14:29, Viresh Kumar via Stratos-dev wrote:
+/* For privcmd_ioeventfd::flags */ +#define PRIVCMD_IOEVENTFD_FLAG_DEASSIGN (1 << 0)
+struct privcmd_ioeventfd {
- void __user *ioreq;
- unsigned int __user *ports;
- __u64 addr;
- __u32 addr_len;
- __u32 event_fd;
- __u32 vcpus;
- __u32 vq;
- __u32 flags;
- domid_t dom;
- __u8 pad[2];
+};
Using indirect pointers in an ioctl command argument means that the layout is architecture specific, in particular you can't use the same one from 32-bit compat tasks. The general recommendation is to have __u64 members and use u64_to_user_ptr() to access it from the kernel if you are unable to avoid the pointers altogether.
/*
- @cmd: IOCTL_PRIVCMD_HYPERCALL
- @arg: &privcmd_hypercall_t
@@ -139,5 +155,7 @@ struct privcmd_irqfd { _IOC(_IOC_NONE, 'P', 7, sizeof(struct privcmd_mmap_resource)) #define IOCTL_PRIVCMD_IRQFD \ _IOC(_IOC_NONE, 'P', 8, sizeof(struct privcmd_irqfd)) +#define IOCTL_PRIVCMD_IOEVENTFD \
- _IOC(_IOC_NONE, 'P', 9, sizeof(struct privcmd_ioeventfd))
_IOC() an internal helper that you should not use in driver code. In particular, you go the data direction wrong here, which breaks a number of tools, as having "_IOC_NONE" should never be paired with a nonzero size.
Instead, you should use the existing _IOWR() or _IOR() macros without the 'sizeof()' like
#define IOCTL_PRIVCMD_IOEVENTFD _IOWR('P', 9, struct privcmd_ioeventfd)
You clearly copied this from the other ioctl command definitions in the same file that we can't easily fix without breaking the user ABI, but I still think you should try to do it correctly for new commands.
Arnd