lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c69c1163-a75b-6667-1979-f3aa356dc0b4@samsung.com>
Date:   Mon, 9 Mar 2020 09:45:24 +0900
From:   Inki Dae <inki.dae@...sung.com>
To:     Lukasz Luba <lukasz.luba@....com>, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-samsung-soc@...r.kernel.org
Cc:     jy0922.shim@...sung.com, sw0312.kim@...sung.com,
        kyungmin.park@...sung.com, airlied@...ux.ie, daniel@...ll.ch,
        kgene@...nel.org, krzk@...nel.org, b.zolnierkie@...sung.com,
        a.hajda@...sung.com, Dietmar.Eggemann@....com
Subject: Re: [PATCH] drm/exynos: Fix memory leak and release IOMMU mapping
 structures

Hi Lukasz,

20. 3. 5. 오전 7:00에 Lukasz Luba 이(가) 쓴 글:
> There is a memory leak which left some objects not freed. The reference
> counter of mapping: 'mapping->kref' was 2 when calling
> arm_iommu_detach_device(), so the release_iommu_mapping() won't be called.
> Since the old mapping structure is not going to be used any more (because
> it is detached and new one attached), call arm_iommu_release_mapping()
> to trigger cleanup.
> 
> Found using kmemleak detector, the output:
> 
> unreferenced object 0xc2137640 (size 64):
>   comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>   hex dump (first 32 bytes):
>     50 a3 14 c2 80 a2 14 c2 01 00 00 00 20 00 00 00  P........... ...
>     00 10 00 00 00 80 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>     [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>     [<ba07704b>] really_probe+0xb0/0x47c
>     [<4f510e4f>] driver_probe_device+0x78/0x1c4
>     [<7481a0cf>] device_driver_attach+0x58/0x60
>     [<0ff8f5c1>] __driver_attach+0xb8/0x158
>     [<86006144>] bus_for_each_dev+0x74/0xb4
>     [<10159dca>] bus_add_driver+0x1c0/0x200
>     [<8a265265>] driver_register+0x74/0x108
>     [<e0f3451a>] exynos_drm_init+0xb0/0x134
>     [<db3fc7ba>] do_one_initcall+0x90/0x458
>     [<6da35917>] kernel_init_freeable+0x188/0x200
>     [<db3f74d4>] kernel_init+0x8/0x110
>     [<1f3cddf9>] ret_from_fork+0x14/0x20
>     [<8cd12507>] 0x0
> unreferenced object 0xc214a280 (size 128):
>   comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>   hex dump (first 32 bytes):
>     00 a0 ec ed 00 00 00 00 00 00 00 00 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>     [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>     [<ba07704b>] really_probe+0xb0/0x47c
>     [<4f510e4f>] driver_probe_device+0x78/0x1c4
>     [<7481a0cf>] device_driver_attach+0x58/0x60
>     [<0ff8f5c1>] __driver_attach+0xb8/0x158
>     [<86006144>] bus_for_each_dev+0x74/0xb4
>     [<10159dca>] bus_add_driver+0x1c0/0x200
>     [<8a265265>] driver_register+0x74/0x108
>     [<e0f3451a>] exynos_drm_init+0xb0/0x134
>     [<db3fc7ba>] do_one_initcall+0x90/0x458
>     [<6da35917>] kernel_init_freeable+0x188/0x200
>     [<db3f74d4>] kernel_init+0x8/0x110
>     [<1f3cddf9>] ret_from_fork+0x14/0x20
>     [<8cd12507>] 0x0
> unreferenced object 0xedeca000 (size 4096):
>   comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>   hex dump (first 32 bytes):
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>   backtrace:
>     [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>     [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>     [<ba07704b>] really_probe+0xb0/0x47c
>     [<4f510e4f>] driver_probe_device+0x78/0x1c4
>     [<7481a0cf>] device_driver_attach+0x58/0x60
>     [<0ff8f5c1>] __driver_attach+0xb8/0x158
>     [<86006144>] bus_for_each_dev+0x74/0xb4
>     [<10159dca>] bus_add_driver+0x1c0/0x200
>     [<8a265265>] driver_register+0x74/0x108
>     [<e0f3451a>] exynos_drm_init+0xb0/0x134
>     [<db3fc7ba>] do_one_initcall+0x90/0x458
>     [<6da35917>] kernel_init_freeable+0x188/0x200
>     [<db3f74d4>] kernel_init+0x8/0x110
>     [<1f3cddf9>] ret_from_fork+0x14/0x20
>     [<8cd12507>] 0x0
> unreferenced object 0xc214a300 (size 128):
>   comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s)
>   hex dump (first 32 bytes):
>     00 a3 14 c2 00 a3 14 c2 00 40 18 c2 00 80 18 c2  .........@......
>     02 00 02 00 ad 4e ad de ff ff ff ff ff ff ff ff  .....N..........
>   backtrace:
>     [<08cbd8bc>] iommu_domain_alloc+0x24/0x50
>     [<b835abee>] arm_iommu_create_mapping+0xe4/0x134
>     [<3acd268d>] arch_setup_dma_ops+0x4c/0x104
>     [<9f7d2cce>] of_dma_configure+0x19c/0x3a4
>     [<ba07704b>] really_probe+0xb0/0x47c
>     [<4f510e4f>] driver_probe_device+0x78/0x1c4
>     [<7481a0cf>] device_driver_attach+0x58/0x60
>     [<0ff8f5c1>] __driver_attach+0xb8/0x158
>     [<86006144>] bus_for_each_dev+0x74/0xb4
>     [<10159dca>] bus_add_driver+0x1c0/0x200
>     [<8a265265>] driver_register+0x74/0x108
>     [<e0f3451a>] exynos_drm_init+0xb0/0x134
>     [<db3fc7ba>] do_one_initcall+0x90/0x458
>     [<6da35917>] kernel_init_freeable+0x188/0x200
>     [<db3f74d4>] kernel_init+0x8/0x110
>     [<1f3cddf9>] ret_from_fork+0x14/0x20
> 
> Signed-off-by: Lukasz Luba <lukasz.luba@....com>
> ---
> 
> Hi all,
> 
> I have discovered this issue on OdroidXU4 while running some stress tests
> for upcoming Energy Model. To reproduce it, kernel must be compiled with
> DEBUG_KMEMLEAK. When the boot has finished, type:
> # echo scan > /sys/kernel/debug/kmemleak
> # cat /sys/kernel/debug/kmemleak
> You should expect similar output to the one from the commit message.
> 
> I don't know if it should go via stable tree as well. I can resend with CC
> stable, if there is a need.

Thanks for fixup. BTW, as you commented on Marek's patch thread, with Marek's patch the memory leak will be solved.
Do you want Marek to rework his patch on top of your patch or are you ok me to pick up only Marek's one?

Marek's patch is conflicted with your one.

Thanks,
Inki Dae

> 
> Regards,
> Lukasz Luba
> 
>  drivers/gpu/drm/exynos/exynos_drm_dma.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dma.c b/drivers/gpu/drm/exynos/exynos_drm_dma.c
> index 9ebc02768847..45f209ec107f 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dma.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dma.c
> @@ -74,8 +74,13 @@ static int drm_iommu_attach_device(struct drm_device *drm_dev,
>  		return ret;
>  
>  	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
> -		if (to_dma_iommu_mapping(subdrv_dev))
> +		struct dma_iommu_mapping *mapping =
> +					to_dma_iommu_mapping(subdrv_dev);
> +
> +		if (mapping) {
>  			arm_iommu_detach_device(subdrv_dev);
> +			arm_iommu_release_mapping(mapping);
> +		}
>  
>  		ret = arm_iommu_attach_device(subdrv_dev, priv->mapping);
>  	} else if (IS_ENABLED(CONFIG_IOMMU_DMA)) {
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ