2015-06-11 07:09:39

by Zhang, Dongxing

[permalink] [raw]
Subject: [PATCH] tty/vt: Fix the memory leak in visual_init

If vc->vc_uni_pagedir_loc is not NULL, its refcount needs to be
decreased before vc_uni_pagedir_loc is re-assigned.

unreferenced object 0xffff88002cdd13b0 (size 512):
comm "setfont", pid 503, jiffies 4294896503 (age 722.828s)
hex dump (first 32 bytes):
40 92 61 2b 00 88 ff ff 00 00 00 00 00 00 00 00 @.a+............
00 00 00 00 00 00 00 00 a0 ad 61 2b 00 88 ff ff ..........a+....
backtrace:
[<ffffffff817b755e>] kmemleak_alloc+0x4e/0xb0
[<ffffffff811d4898>] kmem_cache_alloc_trace+0x1c8/0x240
[<ffffffff814ae7d3>] con_do_clear_unimap.isra.2+0x83/0xe0
[<ffffffff814ae9b2>] con_clear_unimap+0x22/0x40
[<ffffffff814a8db8>] vt_ioctl+0xeb8/0x1170
[<ffffffff8149b458>] tty_ioctl+0x208/0xca0
[<ffffffff81207858>] do_vfs_ioctl+0x2f8/0x510
[<ffffffff81207af1>] SyS_ioctl+0x81/0xa0
[<ffffffff817ca2b2>] system_call_fastpath+0x16/0x75
[<ffffffffffffffff>] 0xffffffffffffffff
unreferenced object 0xffff88002b619240 (size 256):
comm "setfont", pid 503, jiffies 4294896503 (age 722.828s)
hex dump (first 32 bytes):
90 bc 84 d5 00 88 ff ff 58 85 84 d5 00 88 ff ff ........X.......
88 ac 84 d5 00 88 ff ff e0 b1 84 d5 00 88 ff ff ................
backtrace:
[<ffffffff817b755e>] kmemleak_alloc+0x4e/0xb0
[<ffffffff811d4898>] kmem_cache_alloc_trace+0x1c8/0x240
[<ffffffff814ae286>] con_insert_unipair+0x86/0x170
[<ffffffff814af107>] con_set_unimap+0x1b7/0x280
[<ffffffff814a8d65>] vt_ioctl+0xe65/0x1170
[<ffffffff8149b458>] tty_ioctl+0x208/0xca0
[<ffffffff81207858>] do_vfs_ioctl+0x2f8/0x510
[<ffffffff81207af1>] SyS_ioctl+0x81/0xa0
[<ffffffff817ca2b2>] system_call_fastpath+0x16/0x75
[<ffffffffffffffff>] 0xffffffffffffffff

Signed-off-by: Dongxing Zhang <[email protected]>
Signed-off-by: Xiaoming Wang <[email protected]>
---
drivers/tty/vt/vt.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
index 4a24eb2..334c502 100644
--- a/drivers/tty/vt/vt.c
+++ b/drivers/tty/vt/vt.c
@@ -738,6 +738,8 @@ static void visual_init(struct vc_data *vc, int num, int init)
__module_get(vc->vc_sw->owner);
vc->vc_num = num;
vc->vc_display_fg = &master_display_fg;
+ if (vc->vc_uni_pagedir_loc)
+ con_free_unimap(vc);
vc->vc_uni_pagedir_loc = &vc->vc_uni_pagedir;
vc->vc_uni_pagedir = NULL;
vc->vc_hi_font_mask = 0;
--
1.9.1


2015-06-12 07:48:00

by Zhang, Dongxing

[permalink] [raw]
Subject: RE: [PATCH] tty/vt: Fix the memory leak in visual_init

Add more reviewers

> -----Original Message-----
> From: Zhang, Dongxing
> Sent: Wednesday, June 10, 2015 3:21 PM
> To: [email protected]; [email protected];
> [email protected]
> Cc: [email protected]; Zhang, Dongxing; Wang, Xiaoming
> Subject: [PATCH] tty/vt: Fix the memory leak in visual_init
>
> If vc->vc_uni_pagedir_loc is not NULL, its refcount needs to be decreased before
> vc_uni_pagedir_loc is re-assigned.
>
> unreferenced object 0xffff88002cdd13b0 (size 512):
> comm "setfont", pid 503, jiffies 4294896503 (age 722.828s)
> hex dump (first 32 bytes):
> 40 92 61 2b 00 88 ff ff 00 00 00 00 00 00 00 00 @.a+............
> 00 00 00 00 00 00 00 00 a0 ad 61 2b 00 88 ff ff ..........a+....
> backtrace:
> [<ffffffff817b755e>] kmemleak_alloc+0x4e/0xb0
> [<ffffffff811d4898>] kmem_cache_alloc_trace+0x1c8/0x240
> [<ffffffff814ae7d3>] con_do_clear_unimap.isra.2+0x83/0xe0
> [<ffffffff814ae9b2>] con_clear_unimap+0x22/0x40
> [<ffffffff814a8db8>] vt_ioctl+0xeb8/0x1170
> [<ffffffff8149b458>] tty_ioctl+0x208/0xca0
> [<ffffffff81207858>] do_vfs_ioctl+0x2f8/0x510
> [<ffffffff81207af1>] SyS_ioctl+0x81/0xa0
> [<ffffffff817ca2b2>] system_call_fastpath+0x16/0x75
> [<ffffffffffffffff>] 0xffffffffffffffff unreferenced object 0xffff88002b619240
> (size 256):
> comm "setfont", pid 503, jiffies 4294896503 (age 722.828s)
> hex dump (first 32 bytes):
> 90 bc 84 d5 00 88 ff ff 58 85 84 d5 00 88 ff ff ........X.......
> 88 ac 84 d5 00 88 ff ff e0 b1 84 d5 00 88 ff ff ................
> backtrace:
> [<ffffffff817b755e>] kmemleak_alloc+0x4e/0xb0
> [<ffffffff811d4898>] kmem_cache_alloc_trace+0x1c8/0x240
> [<ffffffff814ae286>] con_insert_unipair+0x86/0x170
> [<ffffffff814af107>] con_set_unimap+0x1b7/0x280
> [<ffffffff814a8d65>] vt_ioctl+0xe65/0x1170
> [<ffffffff8149b458>] tty_ioctl+0x208/0xca0
> [<ffffffff81207858>] do_vfs_ioctl+0x2f8/0x510
> [<ffffffff81207af1>] SyS_ioctl+0x81/0xa0
> [<ffffffff817ca2b2>] system_call_fastpath+0x16/0x75
> [<ffffffffffffffff>] 0xffffffffffffffff
>
> Signed-off-by: Dongxing Zhang <[email protected]>
> Signed-off-by: Xiaoming Wang <[email protected]>
> ---
> drivers/tty/vt/vt.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c index 4a24eb2..334c502
> 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -738,6 +738,8 @@ static void visual_init(struct vc_data *vc, int num, int init)
> __module_get(vc->vc_sw->owner);
> vc->vc_num = num;
> vc->vc_display_fg = &master_display_fg;
> + if (vc->vc_uni_pagedir_loc)
> + con_free_unimap(vc);
> vc->vc_uni_pagedir_loc = &vc->vc_uni_pagedir;
> vc->vc_uni_pagedir = NULL;
> vc->vc_hi_font_mask = 0;
> --
> 1.9.1

2015-06-28 20:34:24

by Peter Hurley

[permalink] [raw]
Subject: Re: [PATCH] tty/vt: Fix the memory leak in visual_init

On 06/10/2015 03:21 AM, Dongxing Zhang wrote:
> If vc->vc_uni_pagedir_loc is not NULL, its refcount needs to be
> decreased before vc_uni_pagedir_loc is re-assigned.

Reviewed-by: Peter Hurley <[email protected]>

2015-07-20 15:20:43

by Konstantin Khlebnikov

[permalink] [raw]
Subject: Re: [PATCH] tty/vt: Fix the memory leak in visual_init

On Wed, Jun 10, 2015 at 10:21 AM, Dongxing Zhang
<[email protected]> wrote:
> If vc->vc_uni_pagedir_loc is not NULL, its refcount needs to be
> decreased before vc_uni_pagedir_loc is re-assigned.
>
> unreferenced object 0xffff88002cdd13b0 (size 512):
> comm "setfont", pid 503, jiffies 4294896503 (age 722.828s)
> hex dump (first 32 bytes):
> 40 92 61 2b 00 88 ff ff 00 00 00 00 00 00 00 00 @.a+............
> 00 00 00 00 00 00 00 00 a0 ad 61 2b 00 88 ff ff ..........a+....
> backtrace:
> [<ffffffff817b755e>] kmemleak_alloc+0x4e/0xb0
> [<ffffffff811d4898>] kmem_cache_alloc_trace+0x1c8/0x240
> [<ffffffff814ae7d3>] con_do_clear_unimap.isra.2+0x83/0xe0
> [<ffffffff814ae9b2>] con_clear_unimap+0x22/0x40
> [<ffffffff814a8db8>] vt_ioctl+0xeb8/0x1170
> [<ffffffff8149b458>] tty_ioctl+0x208/0xca0
> [<ffffffff81207858>] do_vfs_ioctl+0x2f8/0x510
> [<ffffffff81207af1>] SyS_ioctl+0x81/0xa0
> [<ffffffff817ca2b2>] system_call_fastpath+0x16/0x75
> [<ffffffffffffffff>] 0xffffffffffffffff
> unreferenced object 0xffff88002b619240 (size 256):
> comm "setfont", pid 503, jiffies 4294896503 (age 722.828s)
> hex dump (first 32 bytes):
> 90 bc 84 d5 00 88 ff ff 58 85 84 d5 00 88 ff ff ........X.......
> 88 ac 84 d5 00 88 ff ff e0 b1 84 d5 00 88 ff ff ................
> backtrace:
> [<ffffffff817b755e>] kmemleak_alloc+0x4e/0xb0
> [<ffffffff811d4898>] kmem_cache_alloc_trace+0x1c8/0x240
> [<ffffffff814ae286>] con_insert_unipair+0x86/0x170
> [<ffffffff814af107>] con_set_unimap+0x1b7/0x280
> [<ffffffff814a8d65>] vt_ioctl+0xe65/0x1170
> [<ffffffff8149b458>] tty_ioctl+0x208/0xca0
> [<ffffffff81207858>] do_vfs_ioctl+0x2f8/0x510
> [<ffffffff81207af1>] SyS_ioctl+0x81/0xa0
> [<ffffffff817ca2b2>] system_call_fastpath+0x16/0x75
> [<ffffffffffffffff>] 0xffffffffffffffff
>
> Signed-off-by: Dongxing Zhang <[email protected]>
> Signed-off-by: Xiaoming Wang <[email protected]>

Looks like leak happens when kernel does this:
[ 5.010152] fb: switching to cirrusdrmfb from EFI VGA

Here:

[ 5.010438] [<ffffffff814bd5f9>] visual_init+0x149/0x150
[ 5.010441] [<ffffffff814bfaa9>] do_bind_con_driver+0x159/0x310
[ 5.010443] [<ffffffff814bffd6>] do_unbind_con_driver+0x1b6/0x210
[ 5.010462] [<ffffffff8143ff3b>] fbcon_event_notify+0x7cb/0x8b0
[ 5.010471] [<ffffffff810a2d85>] notifier_call_chain+0x65/0x90
[ 5.010475] [<ffffffff810a2e3e>] __blocking_notifier_call_chain+0x5e/0x90
[ 5.010478] [<ffffffff810a2e86>] blocking_notifier_call_chain+0x16/0x20
[ 5.010480] [<ffffffff8144309b>] fb_notifier_call_chain+0x1b/0x20
[ 5.010483] [<ffffffff81443f0c>] do_unregister_framebuffer+0x6c/0x130
[ 5.010487] [<ffffffff81444163>]
do_remove_conflicting_framebuffers+0x153/0x180
[ 5.010491] [<ffffffff814444df>] remove_conflicting_framebuffers+0x3f/0x60
[ 5.010495] [<ffffffffa016daa1>] cirrus_pci_probe+0x91/0xd0 [cirrus]
[ 5.010509] [<ffffffff8142382e>] local_pci_probe+0x4e/0xa0
[ 5.010512] [<ffffffff814244b3>] ? pci_match_device+0xe3/0x110
[ 5.010515] [<ffffffff814245f9>] pci_device_probe+0xd9/0x120
[ 5.010525] [<ffffffff81502155>] driver_probe_device+0x155/0x430
[ 5.010528] [<ffffffff815024d1>] __driver_attach+0xa1/0xb0
[ 5.010530] [<ffffffff81502430>] ? driver_probe_device+0x430/0x430
[ 5.010533] [<ffffffff8150023e>] bus_for_each_dev+0x5e/0x90
[ 5.010535] [<ffffffff815019be>] driver_attach+0x1e/0x20
[ 5.010536] [<ffffffff815016ee>] bus_add_driver+0x1be/0x290
[ 5.010538] [<ffffffff81503570>] driver_register+0x60/0xe0
[ 5.010540] [<ffffffff81423ba4>] __pci_register_driver+0x64/0x70
[ 5.010541] [<ffffffffa043f000>] ? 0xffffffffa043f000
[ 5.010553] [<ffffffffa00c8ad0>] drm_pci_init+0xe0/0x110 [drm]
[ 5.010556] [<ffffffffa043f000>] ? 0xffffffffa043f000
[ 5.010560] [<ffffffffa043f03c>] cirrus_init+0x3c/0x1000 [cirrus]
[ 5.010572] [<ffffffff8100212d>] do_one_initcall+0xad/0x1d0
[ 5.010588] [<ffffffff811e04d6>] ? kmem_cache_alloc_trace+0x2b6/0x300
[ 5.010601] [<ffffffff8110a9f4>] do_init_module+0x64/0x220
[ 5.010604] [<ffffffff8110cc4a>] load_module+0x130a/0x1990
[ 5.010606] [<ffffffff811083b0>] ? show_initstate+0x50/0x50
[ 5.010608] [<ffffffff8110d400>] SyS_init_module+0x130/0x180
[ 5.010616] [<ffffffff817b6bee>] entry_SYSCALL_64_fastpath+0x12/0x76


This patch helps. Thanks.

Tested-by: Konstantin Khlebnikov <[email protected]>

> ---
> drivers/tty/vt/vt.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 4a24eb2..334c502 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -738,6 +738,8 @@ static void visual_init(struct vc_data *vc, int num, int init)
> __module_get(vc->vc_sw->owner);
> vc->vc_num = num;
> vc->vc_display_fg = &master_display_fg;
> + if (vc->vc_uni_pagedir_loc)
> + con_free_unimap(vc);
> vc->vc_uni_pagedir_loc = &vc->vc_uni_pagedir;
> vc->vc_uni_pagedir = NULL;
> vc->vc_hi_font_mask = 0;
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/