[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.22.394.2210051719000.3690179@ubuntu-linux-20-04-desktop>
Date: Wed, 5 Oct 2022 17:19:07 -0700 (PDT)
From: Stefano Stabellini <sstabellini@...nel.org>
To: Oleksandr Tyshchenko <olekstysh@...il.com>
cc: xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org,
Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
Stefano Stabellini <sstabellini@...nel.org>,
Juergen Gross <jgross@...e.com>
Subject: Re: [PATCH 2/2] xen/virtio: Fix potential deadlock when accessing
xen_grant_dma_devices
On Wed, 5 Oct 2022, Oleksandr Tyshchenko wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>
>
> As find_xen_grant_dma_data() is called from both interrupt and process
> contexts, the access to xen_grant_dma_devices XArray must be protected
> by xa_lock_irqsave to avoid deadlock scenario.
> As XArray API doesn't provide xa_store_irqsave helper, call lockless
> __xa_store directly and guard it externally.
>
> Also move the storage of the XArray's entry to a separate helper.
>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>
> Fixes: d6aca3504c7d ("xen/grant-dma-ops: Add option to restrict memory access under Xen")
Reviewed-by: Stefano Stabellini <sstabellini@...nel.org>
> ---
> drivers/xen/grant-dma-ops.c | 24 +++++++++++++++++++-----
> 1 file changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/xen/grant-dma-ops.c b/drivers/xen/grant-dma-ops.c
> index 1998d0e8ce82..c66f56d24013 100644
> --- a/drivers/xen/grant-dma-ops.c
> +++ b/drivers/xen/grant-dma-ops.c
> @@ -25,7 +25,7 @@ struct xen_grant_dma_data {
> bool broken;
> };
>
> -static DEFINE_XARRAY(xen_grant_dma_devices);
> +static DEFINE_XARRAY_FLAGS(xen_grant_dma_devices, XA_FLAGS_LOCK_IRQ);
>
> #define XEN_GRANT_DMA_ADDR_OFF (1ULL << 63)
>
> @@ -42,14 +42,29 @@ static inline grant_ref_t dma_to_grant(dma_addr_t dma)
> static struct xen_grant_dma_data *find_xen_grant_dma_data(struct device *dev)
> {
> struct xen_grant_dma_data *data;
> + unsigned long flags;
>
> - xa_lock(&xen_grant_dma_devices);
> + xa_lock_irqsave(&xen_grant_dma_devices, flags);
> data = xa_load(&xen_grant_dma_devices, (unsigned long)dev);
> - xa_unlock(&xen_grant_dma_devices);
> + xa_unlock_irqrestore(&xen_grant_dma_devices, flags);
>
> return data;
> }
>
> +static int store_xen_grant_dma_data(struct device *dev,
> + struct xen_grant_dma_data *data)
> +{
> + unsigned long flags;
> + int ret;
> +
> + xa_lock_irqsave(&xen_grant_dma_devices, flags);
> + ret = xa_err(__xa_store(&xen_grant_dma_devices, (unsigned long)dev, data,
> + GFP_ATOMIC));
> + xa_unlock_irqrestore(&xen_grant_dma_devices, flags);
> +
> + return ret;
> +}
> +
> /*
> * DMA ops for Xen frontends (e.g. virtio).
> *
> @@ -338,8 +353,7 @@ void xen_grant_setup_dma_ops(struct device *dev)
> */
> data->backend_domid = iommu_spec.args[0];
>
> - if (xa_err(xa_store(&xen_grant_dma_devices, (unsigned long)dev, data,
> - GFP_KERNEL))) {
> + if (store_xen_grant_dma_data(dev, data)) {
> dev_err(dev, "Cannot store Xen grant DMA data\n");
> goto err;
> }
> --
> 2.25.1
>
Powered by blists - more mailing lists