From: Greg KH <gregkh@linuxfoundation.org>
To: Nikhil V <quic_nprakash@quicinc.com>
Cc: Charan Teja Kalla <quic_charante@quicinc.com>,
Joerg Roedel <joro@8bytes.org>, Will Deacon <will@kernel.org>,
Robin Murphy <robin.murphy@arm.com>,
linux-kernel@vger.kernel.org, iommu@lists.linux.dev,
stable@vger.kernel.org
Subject: Re: [PATCH RESEND STABLE v6.1] iommu: Avoid races around default domain allocations
Date: Mon, 4 Mar 2024 14:25:20 +0100 [thread overview]
Message-ID: <2024030403-self-morality-062e@gregkh> (raw)
In-Reply-To: <cbf1295589bd90083ad6f75a7fbced01f327c047.1708680521.git.quic_nprakash@quicinc.com>
On Mon, Mar 04, 2024 at 04:40:50PM +0530, Nikhil V wrote:
> From: Charan Teja Kalla <quic_charante@quicinc.com>
>
> This fix is applicable for LTS kernel, 6.1.y. In latest kernels, this race
> issue is fixed by the patch series [1] and [2]. The right thing to do here
> would have been propagating these changes from latest kernel to the stable
> branch, 6.1.y. However, these changes seems too intrusive to be picked for
> stable branches. Hence, the fix proposed can be taken as an alternative
> instead of backporting the patch series.
> [1] https://lore.kernel.org/all/0-v8-81230027b2fa+9d-iommu_all_defdom_jgg@nvidia.com/
> [2] https://lore.kernel.org/all/0-v5-1b99ae392328+44574-iommu_err_unwind_jgg@nvidia.com/
>
> Issue:
> A race condition is observed when arm_smmu_device_probe and
> modprobe of client devices happens in parallel. This results
> in the allocation of a new default domain for the iommu group
> even though it was previously allocated and the respective iova
> domain(iovad) was initialized. However, for this newly allocated
> default domain, iovad will not be initialized. As a result, for
> devices requesting dma allocations, this uninitialized iovad will
> be used, thereby causing NULL pointer dereference issue.
>
> Flow:
> - During arm_smmu_device_probe, bus_iommu_probe() will be called
> as part of iommu_device_register(). This results in the device probe,
> __iommu_probe_device().
>
> - When the modprobe of the client device happens in parallel, it
> sets up the DMA configuration for the device using of_dma_configure_id(),
> which inturn calls iommu_probe_device(). Later, default domain is
> allocated and attached using iommu_alloc_default_domain() and
> __iommu_attach_device() respectively. It then ends up initializing a
> mapping domain(IOVA domain) and rcaches for the device via
> arch_setup_dma_ops()->iommu_setup_dma_ops().
>
> - Now, in the bus_iommu_probe() path, it again tries to allocate
> a default domain via probe_alloc_default_domain(). This results in
> allocating a new default domain(along with IOVA domain) via
> __iommu_domain_alloc(). However, this newly allocated IOVA domain
> will not be initialized.
>
> - Now, when the same client device tries dma allocations via
> iommu_dma_alloc(), it ends up accessing the rcaches of the newly
> allocated IOVA domain, which is not initialized. This results
> into NULL pointer dereferencing.
>
> Fix this issue by adding a check in probe_alloc_default_domain()
> to see if the iommu_group already has a default domain allocated
> and initialized.
>
> Cc: <stable@vger.kernel.org> # see patch description, fix applicable only for 6.1.y
> Signed-off-by: Charan Teja Kalla <quic_charante@quicinc.com>
> Co-developed-by: Nikhil V <quic_nprakash@quicinc.com>
> Signed-off-by: Nikhil V <quic_nprakash@quicinc.com>
> ---
> drivers/iommu/iommu.c | 3 +++
> 1 file changed, 3 insertions(+)
Why RESEND? What happened to the first send?
thanks,
greg k-h
next prev parent reply other threads:[~2024-03-04 13:25 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-23 9:40 [PATCH] iommu: Avoid races around default domain allocations Nikhil V
2024-03-04 11:10 ` [PATCH RESEND STABLE v6.1] " Nikhil V
2024-02-23 9:44 ` [PATCH] " kernel test robot
2024-03-04 13:25 ` Greg KH [this message]
2024-03-05 7:31 ` [PATCH RESEND STABLE v6.1] " Nikhil V
2024-03-29 10:21 ` Greg KH
2024-03-29 10:26 ` Patch "iommu: Avoid races around default domain allocations" has been added to the 6.1-stable tree gregkh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2024030403-self-morality-062e@gregkh \
--to=gregkh@linuxfoundation.org \
--cc=iommu@lists.linux.dev \
--cc=joro@8bytes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=quic_charante@quicinc.com \
--cc=quic_nprakash@quicinc.com \
--cc=robin.murphy@arm.com \
--cc=stable@vger.kernel.org \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.