[PATCH OLK-6.6 0/7] uacce: fix some problems
From: JiangShui Yang <yangjiangshui@h-partners.com> Chenghai Huang (1): crypto: hisilicon/zip - correct the wrong word in the printed information Longfang Liu (1): migration: modify dfx error type without VM driver Weili Qian (1): uacce: check the qfr address before releasing the qfr Yang Shen (4): uacce: add error number for the unsupported mremap uacce: add stop_queue to replace put_queue in ioctl uacce: add mutex for check qfr crypto: hisilicon/qm - fix the uacce_alloc return value check drivers/crypto/hisilicon/qm.c | 8 ++ drivers/crypto/hisilicon/zip/zip_crypto.c | 2 +- drivers/misc/uacce/uacce.c | 76 ++++++++++++------- .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 2 +- 4 files changed, 60 insertions(+), 28 deletions(-) -- 2.43.0
driver inclusion category: feature bugzilla: https://gitee.com/openeuler/kernel/issues/IB9H0P CVE: NA ---------------------------------------------------------------------- Since the driver doesn't support the mremap, add an error number to show users the fail. Signed-off-by: Yang Shen <shenyang39@huawei.com> Signed-off-by: JiangShui Yang <yangjiangshui@h-partners.com> --- drivers/misc/uacce/uacce.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c index 6e142a04b080..6202a4e2c60f 100644 --- a/drivers/misc/uacce/uacce.c +++ b/drivers/misc/uacce/uacce.c @@ -392,8 +392,14 @@ static void uacce_vma_close(struct vm_area_struct *vma) } } +static int uacce_vma_mremap(struct vm_area_struct *area) +{ + return -EPERM; +} + static const struct vm_operations_struct uacce_vm_ops = { .close = uacce_vma_close, + .mremap = uacce_vma_mremap, }; static int get_sort_base(struct uacce_dma_slice *list, int low, int high, -- 2.43.0
From: Weili Qian <qianweili@huawei.com> driver inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IB9H0P CVE: NA ---------------------------------------------------------------------- When qfr type is UACCE_QFRT_SS, qfr may be noiommu_ss_default_qfr. The memory is global static memory and cannot be freed. Therefore, the qfr address needs to be checked before the qfr is released. Fixes: c0b0e89513ec ("uacce: support UACCE_MODE_NOIOMMU mode") Signed-off-by: Weili Qian <qianweili@huawei.com> Signed-off-by: JiangShui Yang <yangjiangshui@h-partners.com> --- drivers/misc/uacce/uacce.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c index 6202a4e2c60f..34ba926c1d56 100644 --- a/drivers/misc/uacce/uacce.c +++ b/drivers/misc/uacce/uacce.c @@ -360,16 +360,13 @@ static void uacce_vma_close(struct vm_area_struct *vma) struct uacce_queue *q = vma->vm_private_data; struct uacce_qfile_region *qfr = NULL; struct uacce_device *uacce = q->uacce; - struct device *dev = &q->uacce->dev; if (vma->vm_pgoff >= UACCE_MAX_REGION) return; qfr = q->qfrs[vma->vm_pgoff]; - if (!qfr) { - dev_err(dev, "qfr NULL, type %lu!\n", vma->vm_pgoff); + if (!qfr) return; - } if (qfr->type == UACCE_QFRT_SS && atomic_read(¤t->active_mm->mm_users) > 0) { @@ -383,7 +380,8 @@ static void uacce_vma_close(struct vm_area_struct *vma) uacce_free_dma_buffers(q); q->qfrs[vma->vm_pgoff] = NULL; mutex_unlock(&uacce->mutex); - kfree(qfr); + if (qfr != &noiommu_ss_default_qfr) + kfree(qfr); } else if (qfr->type != UACCE_QFRT_SS) { mutex_lock(&q->mutex); q->qfrs[vma->vm_pgoff] = NULL; -- 2.43.0
driver inclusion category: feature bugzilla: https://gitee.com/openeuler/kernel/issues/IB9H0P CVE: NA ---------------------------------------------------------------------- Since the resources of uacce_queue cannot be ensured to be fully released before calling put_queue, directly putting the queue might pose a slight risk. However, the release interface can guarantee that the resources have already been released. Therefore, putting the queue through the release interface is a reliable action. Signed-off-by: Yang Shen <shenyang39@huawei.com> Signed-off-by: JiangShui Yang <yangjiangshui@h-partners.com> --- drivers/misc/uacce/uacce.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c index 34ba926c1d56..26df666a6529 100644 --- a/drivers/misc/uacce/uacce.c +++ b/drivers/misc/uacce/uacce.c @@ -47,22 +47,28 @@ static int uacce_start_queue(struct uacce_queue *q) return 0; } -static int uacce_put_queue(struct uacce_queue *q) +static int uacce_stop_queue(struct uacce_queue *q) { struct uacce_device *uacce = q->uacce; if ((q->state == UACCE_Q_STARTED) && uacce->ops->stop_queue) uacce->ops->stop_queue(q); - if ((q->state == UACCE_Q_INIT || q->state == UACCE_Q_STARTED) && - uacce->ops->put_queue) - uacce->ops->put_queue(q); - q->state = UACCE_Q_ZOMBIE; return 0; } +static void uacce_put_queue(struct uacce_queue *q) +{ + struct uacce_device *uacce = q->uacce; + + uacce_stop_queue(q); + + if (uacce->ops->put_queue) + uacce->ops->put_queue(q); +} + static long uacce_cmd_share_qfr(struct uacce_queue *src, int fd) { struct device *dev = &src->uacce->dev; @@ -222,7 +228,7 @@ static long uacce_fops_unl_ioctl(struct file *filep, ret = uacce_start_queue(q); break; case UACCE_CMD_PUT_Q: - ret = uacce_put_queue(q); + ret = uacce_stop_queue(q); break; case UACCE_CMD_SHARE_SVAS: ret = uacce_cmd_share_qfr(q, (int)arg); -- 2.43.0
driver inclusion category: feature bugzilla: https://gitee.com/openeuler/kernel/issues/IB9H0P CVE: NA ---------------------------------------------------------------------- Since the qfr operations can race in some cases, the validity checks require locking protection. Signed-off-by: Yang Shen <shenyang39@huawei.com> Signed-off-by: JiangShui Yang <yangjiangshui@h-partners.com> --- drivers/misc/uacce/uacce.c | 46 +++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c index 26df666a6529..1d50c2e07910 100644 --- a/drivers/misc/uacce/uacce.c +++ b/drivers/misc/uacce/uacce.c @@ -370,26 +370,35 @@ static void uacce_vma_close(struct vm_area_struct *vma) if (vma->vm_pgoff >= UACCE_MAX_REGION) return; - qfr = q->qfrs[vma->vm_pgoff]; - if (!qfr) - return; - - if (qfr->type == UACCE_QFRT_SS && + if (vma->vm_pgoff == UACCE_QFRT_SS && atomic_read(¤t->active_mm->mm_users) > 0) { /* * uacce_vma_close() and uacce_remove() may be executed concurrently. * To avoid accessing the same address at the same time, takes the uacce->mutex. */ mutex_lock(&uacce->mutex); + mutex_lock(&q->mutex); + qfr = q->qfrs[vma->vm_pgoff]; + if (!qfr) { + mutex_lock(&q->mutex); + mutex_unlock(&uacce->mutex); + return; + } if ((q->state == UACCE_Q_STARTED) && uacce->ops->stop_queue) uacce->ops->stop_queue(q); uacce_free_dma_buffers(q); q->qfrs[vma->vm_pgoff] = NULL; + mutex_unlock(&q->mutex); mutex_unlock(&uacce->mutex); if (qfr != &noiommu_ss_default_qfr) kfree(qfr); - } else if (qfr->type != UACCE_QFRT_SS) { + } else if (vma->vm_pgoff != UACCE_QFRT_SS) { mutex_lock(&q->mutex); + qfr = q->qfrs[vma->vm_pgoff]; + if (!qfr) { + mutex_unlock(&q->mutex); + return; + } q->qfrs[vma->vm_pgoff] = NULL; mutex_unlock(&q->mutex); kfree(qfr); @@ -625,24 +634,28 @@ static int uacce_fops_mmap(struct file *filep, struct vm_area_struct *vma) else return -EINVAL; - if (q->qfrs[type]) - return -EEXIST; - - qfr = kzalloc(sizeof(*qfr), GFP_KERNEL); - if (!qfr) - return -ENOMEM; - vm_flags_set(vma, VM_DONTCOPY | VM_DONTEXPAND | VM_WIPEONFORK); vma->vm_ops = &uacce_vm_ops; vma->vm_private_data = q; - qfr->type = type; mutex_lock(&q->mutex); + if (q->qfrs[type]) { + mutex_unlock(&q->mutex); + return -EEXIST; + } + + qfr = kzalloc(sizeof(*qfr), GFP_KERNEL); + if (!qfr) { + ret = -ENOMEM; + goto out_with_lock; + } + if (!uacce_queue_is_valid(q)) { ret = -ENXIO; goto out_with_lock; } + qfr->type = type; q->qfrs[type] = qfr; switch (type) { @@ -674,9 +687,10 @@ static int uacce_fops_mmap(struct file *filep, struct vm_area_struct *vma) return ret; out_with_lock: - mutex_unlock(&q->mutex); - kfree(qfr); q->qfrs[type] = NULL; + mutex_unlock(&q->mutex); + if (qfr) + kfree(qfr); return ret; } -- 2.43.0
From: Chenghai Huang <huangchenghai2@huawei.com> driver inclusion category: cleanup bugzilla: https://gitee.com/openeuler/kernel/issues/IB9H0P CVE: NA ---------------------------------------------------------------------- Correct the wrong word in the printed information. Signed-off-by: Chenghai Huang <huangchenghai2@huawei.com> Signed-off-by: JiangShui Yang <yangjiangshui@h-partners.com> --- drivers/crypto/hisilicon/zip/zip_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/crypto/hisilicon/zip/zip_crypto.c b/drivers/crypto/hisilicon/zip/zip_crypto.c index 7327f8f29b01..5cf72bbc311f 100644 --- a/drivers/crypto/hisilicon/zip/zip_crypto.c +++ b/drivers/crypto/hisilicon/zip/zip_crypto.c @@ -236,7 +236,7 @@ static int hisi_zip_do_work(struct hisi_zip_qp_ctx *qp_ctx, &req->dma_dst); if (IS_ERR(req->hw_dst)) { ret = PTR_ERR(req->hw_dst); - dev_err(dev, "failed to map the dst buffer to hw slg (%d)!\n", + dev_err(dev, "failed to map the dst buffer to hw sgl (%d)!\n", ret); goto err_unmap_input; } -- 2.43.0
driver inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IB9H0P CVE: NA ---------------------------------------------------------------------- The uacce_alloc mentions that caller needs check returned negotiated uacce->flags. And now it will clear the bit UACCE_DEV_SVA if failed to enable sva. So the qm need to remove uacce if 'flags' changes. Fixes: 9e00df7156e4 ("crypto: hisilicon - register zip engine to uacce") Signed-off-by: Yang Shen <shenyang39@huawei.com> Signed-off-by: JiangShui Yang <yangjiangshui@h-partners.com> --- drivers/crypto/hisilicon/qm.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/crypto/hisilicon/qm.c b/drivers/crypto/hisilicon/qm.c index f5d9ff496005..b3bd4aece611 100644 --- a/drivers/crypto/hisilicon/qm.c +++ b/drivers/crypto/hisilicon/qm.c @@ -2786,6 +2786,7 @@ static int qm_alloc_uacce(struct hisi_qm *qm) struct uacce_interface interface = {0}; struct pci_dev *pdev = qm->pdev; struct uacce_device *uacce; + u32 flags; int ret; ret = strscpy(interface.name, dev_driver_string(&pdev->dev), @@ -2804,11 +2805,18 @@ static int qm_alloc_uacce(struct hisi_qm *qm) } interface.ops = &uacce_qm_ops; + flags = interface.flags; uacce = uacce_alloc(&pdev->dev, &interface); if (IS_ERR(uacce)) { pci_err(pdev, "fail to alloc uacce device\n!"); return PTR_ERR(uacce); } + if (flags != interface.flags) { + pci_err(pdev, "fail to enable sva\n!"); + uacce_remove(uacce); + return -EINVAL; + } + qm->uacce = uacce; qm_uacce_base_init(qm); -- 2.43.0
From: Longfang Liu <liulongfang@huawei.com> driver inclusion category: cleanup bugzilla: https://gitee.com/openeuler/kernel/issues/IB9H0P CVE: NA ---------------------------------------------------------------------- In the live migration scenario, when the ACC driver is not insmod to the VM, the command to test the mailbox operation will detect that the device has no driver loaded. However, the error reported is the same as the abnormal parameter when testing dfx, and the error scenario cannot be distinguished. Therefore, this error type needs to be modified. Signed-off-by: Longfang Liu <liulongfang@huawei.com> Signed-off-by: JiangShui Yang <yangjiangshui@h-partners.com> --- drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c index 0a3589430105..c8b60d1384e4 100644 --- a/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c +++ b/drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c @@ -1523,7 +1523,7 @@ static ssize_t acc_vf_debug_write(struct file *filp, const char __user *buffer, case MB_TEST: ret = acc_vf_debug_test(hisi_acc_vdev); if (ret) - return -EINVAL; + return -EAGAIN; break; case MIG_DATA_DUMP: acc_vf_dev_data_dump(hisi_acc_vdev); -- 2.43.0
participants (1)
-
Yang Shen