All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thierry Reding <treding@nvidia.com>
To: Tejun Heo <tj@kernel.org>
Cc: Paul Walmsley <paul@pwsan.com>,
	gregory.clement@free-electrons.com, hdegoede@redhat.com,
	pwalmsley@nvidia.com, linux-tegra@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-next@vger.kernel.org,
	linux-ide@vger.kernel.org
Subject: Re: next-20150120 broken on Tegra by "ata: libahci: Allow using multiple regulators"
Date: Wed, 21 Jan 2015 16:55:21 +0100	[thread overview]
Message-ID: <20150121155519.GA25166@ulmo.nvidia.com> (raw)
In-Reply-To: <20150121123134.GD8684@htj.dyndns.org>


[-- Attachment #1.1: Type: text/plain, Size: 499 bytes --]

On Wed, Jan 21, 2015 at 07:31:34AM -0500, Tejun Heo wrote:
> On Wed, Jan 21, 2015 at 11:50:03AM +0100, Thierry Reding wrote:
> > Tejun, preferably the attached patch should be squashed into commit
> > c7d7ddee7e24 ("ata: libahci: Allow using multiple regulators") to avoid
> > breaking bisectability. If you don't want to rewrite history, let me
> > know and I can turn it into a proper patch.
> 
> Yes, please make it a proper patch.
> 
> Thanks a lot!

Attaching the patch.

Thierry

[-- Attachment #1.2: 0001-ata-libahci-Fix-devres-cleanup-on-failure.patch --]
[-- Type: text/x-diff, Size: 2009 bytes --]

From a4f78e3ec05f1b2ad86aa44d6bd5394d75a23a06 Mon Sep 17 00:00:00 2001
From: Thierry Reding <treding@nvidia.com>
Date: Wed, 21 Jan 2015 11:50:52 +0100
Subject: [PATCH] ata: libahci: Fix devres cleanup on failure

Commit c7d7ddee7e24 ("ata: libahci: Allow using multiple regulators")
releases regulators during ahci_platform_put_resources(). That doesn't
work because the function is run as part of the devres machinery. Such
resources are torn down in reverse order. Since the array that holds
pointers to the regulators is allocated using devres after the device
context to which ahci_platform_put_resources() is attached, the memory
will be freed before calling ahci_platform_put_resources() and thereby
causing a use-after-free error.

This commit fixes this by using regular allocations for the array. The
memory can then be freed after the regulators have been released. This
conserves the advantages of using the managed API.

Reported-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/ata/libahci_platform.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 73a086664ee7..504d534ccbfe 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -276,6 +276,7 @@ static void ahci_platform_put_resources(struct device *dev, void *res)
 		if (hpriv->target_pwrs && hpriv->target_pwrs[c])
 			regulator_put(hpriv->target_pwrs[c]);
 
+	kfree(hpriv->target_pwrs);
 }
 
 static int ahci_platform_get_phy(struct ahci_host_priv *hpriv, u32 port,
@@ -412,7 +413,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev)
 		goto err_out;
 	}
 	sz = hpriv->nports * sizeof(*hpriv->target_pwrs);
-	hpriv->target_pwrs = devm_kzalloc(dev, sz, GFP_KERNEL);
+	hpriv->target_pwrs = kzalloc(sz, GFP_KERNEL);
 	if (!hpriv->target_pwrs) {
 		rc = -ENOMEM;
 		goto err_out;
-- 
2.1.3


[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Thierry Reding <treding@nvidia.com>
To: Tejun Heo <tj@kernel.org>
Cc: Paul Walmsley <paul@pwsan.com>,
	<gregory.clement@free-electrons.com>, <hdegoede@redhat.com>,
	<pwalmsley@nvidia.com>, <linux-tegra@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>, <linux-next@vger.kernel.org>,
	<linux-ide@vger.kernel.org>
Subject: Re: next-20150120 broken on Tegra by "ata: libahci: Allow using multiple regulators"
Date: Wed, 21 Jan 2015 16:55:21 +0100	[thread overview]
Message-ID: <20150121155519.GA25166@ulmo.nvidia.com> (raw)
In-Reply-To: <20150121123134.GD8684@htj.dyndns.org>


[-- Attachment #1.1: Type: text/plain, Size: 499 bytes --]

On Wed, Jan 21, 2015 at 07:31:34AM -0500, Tejun Heo wrote:
> On Wed, Jan 21, 2015 at 11:50:03AM +0100, Thierry Reding wrote:
> > Tejun, preferably the attached patch should be squashed into commit
> > c7d7ddee7e24 ("ata: libahci: Allow using multiple regulators") to avoid
> > breaking bisectability. If you don't want to rewrite history, let me
> > know and I can turn it into a proper patch.
> 
> Yes, please make it a proper patch.
> 
> Thanks a lot!

Attaching the patch.

Thierry

[-- Attachment #1.2: 0001-ata-libahci-Fix-devres-cleanup-on-failure.patch --]
[-- Type: text/x-diff, Size: 2009 bytes --]

From a4f78e3ec05f1b2ad86aa44d6bd5394d75a23a06 Mon Sep 17 00:00:00 2001
From: Thierry Reding <treding@nvidia.com>
Date: Wed, 21 Jan 2015 11:50:52 +0100
Subject: [PATCH] ata: libahci: Fix devres cleanup on failure

Commit c7d7ddee7e24 ("ata: libahci: Allow using multiple regulators")
releases regulators during ahci_platform_put_resources(). That doesn't
work because the function is run as part of the devres machinery. Such
resources are torn down in reverse order. Since the array that holds
pointers to the regulators is allocated using devres after the device
context to which ahci_platform_put_resources() is attached, the memory
will be freed before calling ahci_platform_put_resources() and thereby
causing a use-after-free error.

This commit fixes this by using regular allocations for the array. The
memory can then be freed after the regulators have been released. This
conserves the advantages of using the managed API.

Reported-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/ata/libahci_platform.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 73a086664ee7..504d534ccbfe 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -276,6 +276,7 @@ static void ahci_platform_put_resources(struct device *dev, void *res)
 		if (hpriv->target_pwrs && hpriv->target_pwrs[c])
 			regulator_put(hpriv->target_pwrs[c]);
 
+	kfree(hpriv->target_pwrs);
 }
 
 static int ahci_platform_get_phy(struct ahci_host_priv *hpriv, u32 port,
@@ -412,7 +413,7 @@ struct ahci_host_priv *ahci_platform_get_resources(struct platform_device *pdev)
 		goto err_out;
 	}
 	sz = hpriv->nports * sizeof(*hpriv->target_pwrs);
-	hpriv->target_pwrs = devm_kzalloc(dev, sz, GFP_KERNEL);
+	hpriv->target_pwrs = kzalloc(sz, GFP_KERNEL);
 	if (!hpriv->target_pwrs) {
 		rc = -ENOMEM;
 		goto err_out;
-- 
2.1.3


[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: treding@nvidia.com (Thierry Reding)
To: linux-arm-kernel@lists.infradead.org
Subject: next-20150120 broken on Tegra by "ata: libahci: Allow using multiple regulators"
Date: Wed, 21 Jan 2015 16:55:21 +0100	[thread overview]
Message-ID: <20150121155519.GA25166@ulmo.nvidia.com> (raw)
In-Reply-To: <20150121123134.GD8684@htj.dyndns.org>

On Wed, Jan 21, 2015 at 07:31:34AM -0500, Tejun Heo wrote:
> On Wed, Jan 21, 2015 at 11:50:03AM +0100, Thierry Reding wrote:
> > Tejun, preferably the attached patch should be squashed into commit
> > c7d7ddee7e24 ("ata: libahci: Allow using multiple regulators") to avoid
> > breaking bisectability. If you don't want to rewrite history, let me
> > know and I can turn it into a proper patch.
> 
> Yes, please make it a proper patch.
> 
> Thanks a lot!

Attaching the patch.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ata-libahci-Fix-devres-cleanup-on-failure.patch
Type: text/x-diff
Size: 1961 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150121/416f7897/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150121/416f7897/attachment.sig>

  reply	other threads:[~2015-01-21 15:55 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-21  0:03 next-20150120 broken on Tegra by "ata: libahci: Allow using multiple regulators" Paul Walmsley
2015-01-21  0:03 ` Paul Walmsley
2015-01-21  0:03 ` Paul Walmsley
2015-01-21 10:50 ` Thierry Reding
2015-01-21 10:50   ` Thierry Reding
2015-01-21 10:50   ` Thierry Reding
2015-01-21 11:23   ` Gregory CLEMENT
2015-01-21 11:23     ` Gregory CLEMENT
     [not found]   ` <20150121105001.GA9921-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>
2015-01-21 12:31     ` Tejun Heo
2015-01-21 12:31       ` Tejun Heo
2015-01-21 12:31       ` Tejun Heo
2015-01-21 15:55       ` Thierry Reding [this message]
2015-01-21 15:55         ` Thierry Reding
2015-01-21 15:55         ` Thierry Reding
     [not found]         ` <20150121155519.GA25166-AwZRO8vwLAwmlAP/+Wk3EA@public.gmane.org>
2015-01-21 16:25           ` Tejun Heo
2015-01-21 16:25             ` Tejun Heo
2015-01-21 16:25             ` Tejun Heo
  -- strict thread matches above, loose matches on Subject: below --
2015-01-21  0:00 Paul Walmsley
2015-01-21  0:00 ` Paul Walmsley

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=20150121155519.GA25166@ulmo.nvidia.com \
    --to=treding@nvidia.com \
    --cc=gregory.clement@free-electrons.com \
    --cc=hdegoede@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=paul@pwsan.com \
    --cc=pwalmsley@nvidia.com \
    --cc=tj@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.