Introduce a core helper function for VFIO_MIG_GET_PRECOPY_INFO and adapt all drivers to use it. It centralizes the common code and ensures that output flags are cleared on entry, in case user opts in to VFIO_DEVICE_FEATURE_MIG_PRECOPY_INFOv2. This preventing any unintended echoing of userspace data back to userspace. Signed-off-by: Yishai Hadas --- .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 17 +++----- drivers/vfio/pci/mlx5/main.c | 18 +++------ drivers/vfio/pci/qat/main.c | 17 +++----- drivers/vfio/pci/virtio/migrate.c | 17 +++----- include/linux/vfio.h | 39 +++++++++++++++++++ samples/vfio-mdev/mtty.c | 16 +++----- 6 files changed, 68 insertions(+), 56 deletions(-) diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c index 1d367cff7dcf..bb121f635b9f 100644 --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c @@ -857,18 +857,12 @@ static long hisi_acc_vf_precopy_ioctl(struct file *filp, struct hisi_acc_vf_core_device *hisi_acc_vdev = migf->hisi_acc_vdev; loff_t *pos = &filp->f_pos; struct vfio_precopy_info info; - unsigned long minsz; int ret; - if (cmd != VFIO_MIG_GET_PRECOPY_INFO) - return -ENOTTY; - - minsz = offsetofend(struct vfio_precopy_info, dirty_bytes); - - if (copy_from_user(&info, (void __user *)arg, minsz)) - return -EFAULT; - if (info.argsz < minsz) - return -EINVAL; + ret = vfio_check_precopy_ioctl(&hisi_acc_vdev->core_device.vdev, cmd, + arg, &info); + if (ret) + return ret; mutex_lock(&hisi_acc_vdev->state_mutex); if (hisi_acc_vdev->mig_state != VFIO_DEVICE_STATE_PRE_COPY) { @@ -893,7 +887,8 @@ static long hisi_acc_vf_precopy_ioctl(struct file *filp, mutex_unlock(&migf->lock); mutex_unlock(&hisi_acc_vdev->state_mutex); - return copy_to_user((void __user *)arg, &info, minsz) ? -EFAULT : 0; + return copy_to_user((void __user *)arg, &info, + offsetofend(struct vfio_precopy_info, dirty_bytes)) ? -EFAULT : 0; out: mutex_unlock(&migf->lock); mutex_unlock(&hisi_acc_vdev->state_mutex); diff --git a/drivers/vfio/pci/mlx5/main.c b/drivers/vfio/pci/mlx5/main.c index dbba6173894b..fb541c17c712 100644 --- a/drivers/vfio/pci/mlx5/main.c +++ b/drivers/vfio/pci/mlx5/main.c @@ -463,21 +463,14 @@ static long mlx5vf_precopy_ioctl(struct file *filp, unsigned int cmd, struct mlx5_vhca_data_buffer *buf; struct vfio_precopy_info info = {}; loff_t *pos = &filp->f_pos; - unsigned long minsz; size_t inc_length = 0; bool end_of_data = false; int ret; - if (cmd != VFIO_MIG_GET_PRECOPY_INFO) - return -ENOTTY; - - minsz = offsetofend(struct vfio_precopy_info, dirty_bytes); - - if (copy_from_user(&info, (void __user *)arg, minsz)) - return -EFAULT; - - if (info.argsz < minsz) - return -EINVAL; + ret = vfio_check_precopy_ioctl(&mvdev->core_device.vdev, cmd, arg, + &info); + if (ret) + return ret; mutex_lock(&mvdev->state_mutex); if (mvdev->mig_state != VFIO_DEVICE_STATE_PRE_COPY && @@ -545,7 +538,8 @@ static long mlx5vf_precopy_ioctl(struct file *filp, unsigned int cmd, done: mlx5vf_state_mutex_unlock(mvdev); - if (copy_to_user((void __user *)arg, &info, minsz)) + if (copy_to_user((void __user *)arg, &info, + offsetofend(struct vfio_precopy_info, dirty_bytes))) return -EFAULT; return 0; diff --git a/drivers/vfio/pci/qat/main.c b/drivers/vfio/pci/qat/main.c index b982d4ae666c..b3a4b7a55696 100644 --- a/drivers/vfio/pci/qat/main.c +++ b/drivers/vfio/pci/qat/main.c @@ -121,18 +121,12 @@ static long qat_vf_precopy_ioctl(struct file *filp, unsigned int cmd, struct qat_mig_dev *mig_dev = qat_vdev->mdev; struct vfio_precopy_info info; loff_t *pos = &filp->f_pos; - unsigned long minsz; int ret = 0; - if (cmd != VFIO_MIG_GET_PRECOPY_INFO) - return -ENOTTY; - - minsz = offsetofend(struct vfio_precopy_info, dirty_bytes); - - if (copy_from_user(&info, (void __user *)arg, minsz)) - return -EFAULT; - if (info.argsz < minsz) - return -EINVAL; + ret = vfio_check_precopy_ioctl(&qat_vdev->core_device.vdev, cmd, arg, + &info); + if (ret) + return ret; mutex_lock(&qat_vdev->state_mutex); if (qat_vdev->mig_state != VFIO_DEVICE_STATE_PRE_COPY && @@ -160,7 +154,8 @@ static long qat_vf_precopy_ioctl(struct file *filp, unsigned int cmd, mutex_unlock(&qat_vdev->state_mutex); if (ret) return ret; - return copy_to_user((void __user *)arg, &info, minsz) ? -EFAULT : 0; + return copy_to_user((void __user *)arg, &info, + offsetofend(struct vfio_precopy_info, dirty_bytes)) ? -EFAULT : 0; } static ssize_t qat_vf_save_read(struct file *filp, char __user *buf, diff --git a/drivers/vfio/pci/virtio/migrate.c b/drivers/vfio/pci/virtio/migrate.c index 35fa2d6ed611..7e11834ad512 100644 --- a/drivers/vfio/pci/virtio/migrate.c +++ b/drivers/vfio/pci/virtio/migrate.c @@ -443,19 +443,13 @@ static long virtiovf_precopy_ioctl(struct file *filp, unsigned int cmd, struct vfio_precopy_info info = {}; loff_t *pos = &filp->f_pos; bool end_of_data = false; - unsigned long minsz; u32 ctx_size = 0; int ret; - if (cmd != VFIO_MIG_GET_PRECOPY_INFO) - return -ENOTTY; - - minsz = offsetofend(struct vfio_precopy_info, dirty_bytes); - if (copy_from_user(&info, (void __user *)arg, minsz)) - return -EFAULT; - - if (info.argsz < minsz) - return -EINVAL; + ret = vfio_check_precopy_ioctl(&virtvdev->core_device.vdev, cmd, arg, + &info); + if (ret) + return ret; mutex_lock(&virtvdev->state_mutex); if (virtvdev->mig_state != VFIO_DEVICE_STATE_PRE_COPY && @@ -514,7 +508,8 @@ static long virtiovf_precopy_ioctl(struct file *filp, unsigned int cmd, done: virtiovf_state_mutex_unlock(virtvdev); - if (copy_to_user((void __user *)arg, &info, minsz)) + if (copy_to_user((void __user *)arg, &info, + offsetofend(struct vfio_precopy_info, dirty_bytes))) return -EFAULT; return 0; diff --git a/include/linux/vfio.h b/include/linux/vfio.h index 7c1d33283e04..50b474334a19 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -16,6 +16,7 @@ #include #include #include +#include struct kvm; struct iommufd_ctx; @@ -285,6 +286,44 @@ static inline int vfio_check_feature(u32 flags, size_t argsz, u32 supported_ops, return 1; } +/** + * vfio_check_precopy_ioctl - Validate user input for the VFIO_MIG_GET_PRECOPY_INFO ioctl + * @vdev: The vfio device + * @cmd: Cmd from the ioctl + * @arg: Arg from the ioctl + * @info: Driver pointer to hold the userspace input to the ioctl + * + * For use in a driver's get_precopy_info. Checks that the inputs to the + * VFIO_MIG_GET_PRECOPY_INFO ioctl are correct. + + * Returns 0 on success, otherwise errno. + */ + +static inline int +vfio_check_precopy_ioctl(struct vfio_device *vdev, unsigned int cmd, + unsigned long arg, struct vfio_precopy_info *info) +{ + unsigned long minsz; + + if (cmd != VFIO_MIG_GET_PRECOPY_INFO) + return -ENOTTY; + + minsz = offsetofend(struct vfio_precopy_info, dirty_bytes); + + if (copy_from_user(info, (void __user *)arg, minsz)) + return -EFAULT; + + if (info->argsz < minsz) + return -EINVAL; + + /* keep v1 behaviour as is for compatibility reasons */ + if (vdev->precopy_info_v2) + /* flags are output, set its initial value to 0 */ + info->flags = 0; + + return 0; +} + struct vfio_device *_vfio_alloc_device(size_t size, struct device *dev, const struct vfio_device_ops *ops); #define vfio_alloc_device(dev_struct, member, dev, ops) \ diff --git a/samples/vfio-mdev/mtty.c b/samples/vfio-mdev/mtty.c index bd92c38379b8..c1070af69544 100644 --- a/samples/vfio-mdev/mtty.c +++ b/samples/vfio-mdev/mtty.c @@ -837,18 +837,11 @@ static long mtty_precopy_ioctl(struct file *filp, unsigned int cmd, struct mdev_state *mdev_state = migf->mdev_state; loff_t *pos = &filp->f_pos; struct vfio_precopy_info info = {}; - unsigned long minsz; int ret; - if (cmd != VFIO_MIG_GET_PRECOPY_INFO) - return -ENOTTY; - - minsz = offsetofend(struct vfio_precopy_info, dirty_bytes); - - if (copy_from_user(&info, (void __user *)arg, minsz)) - return -EFAULT; - if (info.argsz < minsz) - return -EINVAL; + ret = vfio_check_precopy_ioctl(&mdev_state->vdev, cmd, arg, &info); + if (ret) + return ret; mutex_lock(&mdev_state->state_mutex); if (mdev_state->state != VFIO_DEVICE_STATE_PRE_COPY && @@ -875,7 +868,8 @@ static long mtty_precopy_ioctl(struct file *filp, unsigned int cmd, info.initial_bytes = migf->filled_size - *pos; mutex_unlock(&migf->lock); - ret = copy_to_user((void __user *)arg, &info, minsz) ? -EFAULT : 0; + ret = copy_to_user((void __user *)arg, &info, + offsetofend(struct vfio_precopy_info, dirty_bytes)) ? -EFAULT : 0; unlock: mtty_state_mutex_unlock(mdev_state); return ret; -- 2.18.1