[REGRESSION] component: add support for releasing match data

Commit Message

Russell King - ARM Linux Jan. 26, 2016, 2:48 p.m. UTC

On Tue, Jan 26, 2016 at 03:28:34PM +0100, Maarten Lankhorst wrote:
> Something similar to a segfault. It's trying to call 0x6b6b6b6b6b which
> is POISON_FREE.
> 
> mc appears to be freed already, so calling mc->release would jump to
> invalid data.

It seems that my devm foo wasn't quite up to scratch.  Quite why it
doesn't show here while testing it (I have patches for the etnaviv
GPU driver, which I'm regularly inserting/removing) I'm not sure.

Please test this patch - it seems "no worse" for me (in that it didn't
crash before, and it still doesn't crash.)

Thanks.

 drivers/base/component.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Maarten Lankhorst Jan. 26, 2016, 4:21 p.m. UTC | #1

Op 26-01-16 om 15:48 schreef Russell King - ARM Linux:
> On Tue, Jan 26, 2016 at 03:28:34PM +0100, Maarten Lankhorst wrote:
>> Something similar to a segfault. It's trying to call 0x6b6b6b6b6b which
>> is POISON_FREE.
>>
>> mc appears to be freed already, so calling mc->release would jump to
>> invalid data.
> It seems that my devm foo wasn't quite up to scratch.  Quite why it
> doesn't show here while testing it (I have patches for the etnaviv
> GPU driver, which I'm regularly inserting/removing) I'm not sure.
>
> Please test this patch - it seems "no worse" for me (in that it didn't
> crash before, and it still doesn't crash.)
That one works, thanks. :)

~Maarten

Daniel Vetter Jan. 26, 2016, 4:26 p.m. UTC | #2

On Tue, Jan 26, 2016 at 05:21:06PM +0100, Maarten Lankhorst wrote:
> Op 26-01-16 om 15:48 schreef Russell King - ARM Linux:
> > On Tue, Jan 26, 2016 at 03:28:34PM +0100, Maarten Lankhorst wrote:
> >> Something similar to a segfault. It's trying to call 0x6b6b6b6b6b which
> >> is POISON_FREE.
> >>
> >> mc appears to be freed already, so calling mc->release would jump to
> >> invalid data.
> > It seems that my devm foo wasn't quite up to scratch.  Quite why it
> > doesn't show here while testing it (I have patches for the etnaviv
> > GPU driver, which I'm regularly inserting/removing) I'm not sure.
> >
> > Please test this patch - it seems "no worse" for me (in that it didn't
> > crash before, and it still doesn't crash.)
> That one works, thanks. :)

Awesome, applied to the CI-only topic branch to unblock the robots, will
fall out automatically once the proper patch lands upstream.
-Daniel

Patch

diff --git a/drivers/base/component.c b/drivers/base/component.c
index 89f5cf68d80a..05cd26c02449 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -206,6 +206,8 @@  static void component_match_release(struct device *master,
 		if (mc->release)
 			mc->release(master, mc->data);
 	}
+
+	kfree(match->compare);
 }
 
 static void devm_component_match_release(struct device *dev, void *res)
@@ -221,14 +223,14 @@  static int component_match_realloc(struct device *dev,
 	if (match->alloc == num)
 		return 0;
 
-	new = devm_kmalloc_array(dev, num, sizeof(*new), GFP_KERNEL);
+	new = kmalloc_array(num, sizeof(*new), GFP_KERNEL);
 	if (!new)
 		return -ENOMEM;
 
 	if (match->compare) {
 		memcpy(new, match->compare, sizeof(*new) *
 					    min(match->num, num));
-		devm_kfree(dev, match->compare);
+		kfree(match->compare);
 	}
 	match->compare = new;
 	match->alloc = num;