From: Lee Jones <lee.jones@linaro.org>
To: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>,
Krzysztof Kozlowski <krzk@kernel.org>,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Subject: Re: [PATCH 1/1] mfd: core: Fix memory leak of 'cell'
Date: Thu, 13 Aug 2020 09:01:00 +0100 [thread overview]
Message-ID: <20200813080100.GI4354@dell> (raw)
In-Reply-To: <79bcd1b5-2247-5e00-7d49-2f94f9b40744@samsung.com>
On Thu, 13 Aug 2020, Marek Szyprowski wrote:
> Hi All,
>
> On 05.08.2020 16:26, Marek Szyprowski wrote:
> > On 16.07.2020 16:28, Lee Jones wrote:
> >> When creating a platform device from an MFD cell description, we
> >> allocate some memory and make a copy which is then stored inside the
> >> platform_device's structure. However, care is not currently taken to
> >> free the allocated memory when the platform device is torn down.
> >>
> >> This patch takes care of the leak.
> >>
> >> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >
> > This patch landed recently in linux-next as commit 126f33704d9d ("mfd:
> > core: Fix memory leak of 'cell'"). Sadly it causes a regression on
> > Samsung Exynos4412-based Trats2 board:
> > ...
> > Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> >
> > I suspect that this is somehow related to the deferred probe and/or
> > devm_ helpers, but I didn't analyze it further yet. Reverting it on
> > top of current linux-next (and resolving conflict) fixes the boot.
> > Bisecting it was really hard because the issue is not fully
> > reproducible, what suggests memory trashing. Various tests of
> > linux-next with the reverted patch have shown at least that the issue
> > is gone.
> >
> > I've compiled the kernel from exynos_defconfig, the dts used for the
> > test is arch/arm/boot/dts/exynos4412-trats2.dts
>
> Finally I've found some time to analyze this issue.
>
> Indeed this patch is wrong it causes double free on the mfd_cell.
> mfd_cell is already properly freed by the platform_device_release()
> function when kref of the pdev goes down to zero:
> https://elixir.bootlin.com/linux/latest/source/drivers/base/platform.c#L426
Thank you for the explanation.
> I will send a revert with the explaination.
No need. I remove it from my tree. Thanks.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: Lee Jones <lee.jones@linaro.org>
To: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org,
Sylwester Nawrocki <s.nawrocki@samsung.com>,
Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
Krzysztof Kozlowski <krzk@kernel.org>
Subject: Re: [PATCH 1/1] mfd: core: Fix memory leak of 'cell'
Date: Thu, 13 Aug 2020 09:01:00 +0100 [thread overview]
Message-ID: <20200813080100.GI4354@dell> (raw)
In-Reply-To: <79bcd1b5-2247-5e00-7d49-2f94f9b40744@samsung.com>
On Thu, 13 Aug 2020, Marek Szyprowski wrote:
> Hi All,
>
> On 05.08.2020 16:26, Marek Szyprowski wrote:
> > On 16.07.2020 16:28, Lee Jones wrote:
> >> When creating a platform device from an MFD cell description, we
> >> allocate some memory and make a copy which is then stored inside the
> >> platform_device's structure. However, care is not currently taken to
> >> free the allocated memory when the platform device is torn down.
> >>
> >> This patch takes care of the leak.
> >>
> >> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >
> > This patch landed recently in linux-next as commit 126f33704d9d ("mfd:
> > core: Fix memory leak of 'cell'"). Sadly it causes a regression on
> > Samsung Exynos4412-based Trats2 board:
> > ...
> > Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> >
> > I suspect that this is somehow related to the deferred probe and/or
> > devm_ helpers, but I didn't analyze it further yet. Reverting it on
> > top of current linux-next (and resolving conflict) fixes the boot.
> > Bisecting it was really hard because the issue is not fully
> > reproducible, what suggests memory trashing. Various tests of
> > linux-next with the reverted patch have shown at least that the issue
> > is gone.
> >
> > I've compiled the kernel from exynos_defconfig, the dts used for the
> > test is arch/arm/boot/dts/exynos4412-trats2.dts
>
> Finally I've found some time to analyze this issue.
>
> Indeed this patch is wrong it causes double free on the mfd_cell.
> mfd_cell is already properly freed by the platform_device_release()
> function when kref of the pdev goes down to zero:
> https://elixir.bootlin.com/linux/latest/source/drivers/base/platform.c#L426
Thank you for the explanation.
> I will send a revert with the explaination.
No need. I remove it from my tree. Thanks.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
next prev parent reply other threads:[~2020-08-13 8:02 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20200805142610eucas1p1dd85e87d57dab1433d3d31a9c0ec49d6@eucas1p1.samsung.com>
2020-07-16 14:28 ` [PATCH 1/1] mfd: core: Fix memory leak of 'cell' Lee Jones
2020-07-16 14:28 ` Lee Jones
2020-08-05 14:26 ` Marek Szyprowski
2020-08-05 14:26 ` Marek Szyprowski
2020-08-13 7:24 ` Marek Szyprowski
2020-08-13 7:24 ` Marek Szyprowski
2020-08-13 8:01 ` Lee Jones [this message]
2020-08-13 8:01 ` Lee Jones
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=20200813080100.GI4354@dell \
--to=lee.jones@linaro.org \
--cc=b.zolnierkie@samsung.com \
--cc=krzk@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=s.nawrocki@samsung.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.