From: Joerg Roedel <joro@8bytes.org>
To: Xiaomeng Tong <xiam0nd.tong@gmail.com>
Cc: agross@kernel.org, bjorn.andersson@linaro.org, will@kernel.org,
sricharan@codeaurora.org, linux-arm-msm@vger.kernel.org,
iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
Subject: Re: [PATCH] iommu: fix an incorrect NULL check on list iterator
Date: Thu, 28 Apr 2022 10:11:03 +0200 [thread overview]
Message-ID: <YmpMF3ico9tregcJ@8bytes.org> (raw)
In-Reply-To: <20220327053558.2821-1-xiam0nd.tong@gmail.com>
On Sun, Mar 27, 2022 at 01:35:58PM +0800, Xiaomeng Tong wrote:
> @@ -617,23 +617,17 @@ static int qcom_iommu_of_xlate(struct device *dev,
> {
> struct msm_iommu_dev *iommu;
> unsigned long flags;
> - int ret = 0;
>
> spin_lock_irqsave(&msm_iommu_lock, flags);
> list_for_each_entry(iommu, &qcom_iommu_devices, dev_node)
> - if (iommu->dev->of_node == spec->np)
> - break;
> -
> - if (!iommu || iommu->dev->of_node != spec->np) {
> - ret = -ENODEV;
> - goto fail;
> - }
> -
> - insert_iommu_master(dev, &iommu, spec);
> -fail:
> + if (iommu->dev->of_node == spec->np) {
> + insert_iommu_master(dev, &iommu, spec);
> + spin_unlock_irqrestore(&msm_iommu_lock, flags);
> + return 0;
> + }
> spin_unlock_irqrestore(&msm_iommu_lock, flags);
>
> - return ret;
> + return -ENODEV;
This looks a bit clumsy, a better fix is below:
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 50f57624610f..98d23c52537b 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -610,14 +610,16 @@ static void insert_iommu_master(struct device *dev,
static int qcom_iommu_of_xlate(struct device *dev,
struct of_phandle_args *spec)
{
- struct msm_iommu_dev *iommu;
+ struct msm_iommu_dev *iommu = NULL, *it;
unsigned long flags;
int ret = 0;
spin_lock_irqsave(&msm_iommu_lock, flags);
- list_for_each_entry(iommu, &qcom_iommu_devices, dev_node)
- if (iommu->dev->of_node == spec->np)
+ list_for_each_entry(it, &qcom_iommu_devices, dev_node)
+ if (it->dev->of_node == spec->np) {
+ iommu = it;
break;
+ }
if (!iommu || iommu->dev->of_node != spec->np) {
ret = -ENODEV;
Can you please verify this and re-submit?
Thanks,
Joerg
WARNING: multiple messages have this Message-ID (diff)
From: Joerg Roedel <joro@8bytes.org>
To: Xiaomeng Tong <xiam0nd.tong@gmail.com>
Cc: will@kernel.org, linux-arm-msm@vger.kernel.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org,
iommu@lists.linux-foundation.org, agross@kernel.org,
sricharan@codeaurora.org
Subject: Re: [PATCH] iommu: fix an incorrect NULL check on list iterator
Date: Thu, 28 Apr 2022 10:11:03 +0200 [thread overview]
Message-ID: <YmpMF3ico9tregcJ@8bytes.org> (raw)
In-Reply-To: <20220327053558.2821-1-xiam0nd.tong@gmail.com>
On Sun, Mar 27, 2022 at 01:35:58PM +0800, Xiaomeng Tong wrote:
> @@ -617,23 +617,17 @@ static int qcom_iommu_of_xlate(struct device *dev,
> {
> struct msm_iommu_dev *iommu;
> unsigned long flags;
> - int ret = 0;
>
> spin_lock_irqsave(&msm_iommu_lock, flags);
> list_for_each_entry(iommu, &qcom_iommu_devices, dev_node)
> - if (iommu->dev->of_node == spec->np)
> - break;
> -
> - if (!iommu || iommu->dev->of_node != spec->np) {
> - ret = -ENODEV;
> - goto fail;
> - }
> -
> - insert_iommu_master(dev, &iommu, spec);
> -fail:
> + if (iommu->dev->of_node == spec->np) {
> + insert_iommu_master(dev, &iommu, spec);
> + spin_unlock_irqrestore(&msm_iommu_lock, flags);
> + return 0;
> + }
> spin_unlock_irqrestore(&msm_iommu_lock, flags);
>
> - return ret;
> + return -ENODEV;
This looks a bit clumsy, a better fix is below:
diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
index 50f57624610f..98d23c52537b 100644
--- a/drivers/iommu/msm_iommu.c
+++ b/drivers/iommu/msm_iommu.c
@@ -610,14 +610,16 @@ static void insert_iommu_master(struct device *dev,
static int qcom_iommu_of_xlate(struct device *dev,
struct of_phandle_args *spec)
{
- struct msm_iommu_dev *iommu;
+ struct msm_iommu_dev *iommu = NULL, *it;
unsigned long flags;
int ret = 0;
spin_lock_irqsave(&msm_iommu_lock, flags);
- list_for_each_entry(iommu, &qcom_iommu_devices, dev_node)
- if (iommu->dev->of_node == spec->np)
+ list_for_each_entry(it, &qcom_iommu_devices, dev_node)
+ if (it->dev->of_node == spec->np) {
+ iommu = it;
break;
+ }
if (!iommu || iommu->dev->of_node != spec->np) {
ret = -ENODEV;
Can you please verify this and re-submit?
Thanks,
Joerg
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2022-04-28 8:11 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-27 5:35 [PATCH] iommu: fix an incorrect NULL check on list iterator Xiaomeng Tong
2022-03-27 5:35 ` Xiaomeng Tong
2022-04-28 8:11 ` Joerg Roedel [this message]
2022-04-28 8:11 ` Joerg Roedel
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=YmpMF3ico9tregcJ@8bytes.org \
--to=joro@8bytes.org \
--cc=agross@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=iommu@lists.linux-foundation.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sricharan@codeaurora.org \
--cc=stable@vger.kernel.org \
--cc=will@kernel.org \
--cc=xiam0nd.tong@gmail.com \
/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.