Kexec Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] sadump: set info->page_size before cache_init()
@ 2017-05-23  2:52 Hatayama, Daisuke
  2017-05-25  7:52 ` Pratyush Anand
  0 siblings, 1 reply; 3+ messages in thread
From: Hatayama, Daisuke @ 2017-05-23  2:52 UTC (permalink / raw)
  To: 'ats-kumagai@wm.jp.nec.com'; +Cc: 'kexec@lists.infradead.org'

Currently, makedumpfile results in Segmentation fault on sadump dump
files as follows:

    # LANG=C makedumpfile -f --message-level=31 -ld31 -x vmlinux ./sadump_vmcore sadump_vmcore-ld31
    sadump: read dump device as single partition
    sadump: single partition configuration
    page_size    : 4096
    sadump: timezone information is missing
    Segmentation fault

By bisect, I found that this issue is caused by the following commit
that moves invocation of cache_init() in initial() a bit early:

    # git bisect bad
    8e2834bac4f62da3894da297f083068431be6d80 is the first bad commit
    commit 8e2834bac4f62da3894da297f083068431be6d80
    Author: Pratyush Anand <panand@redhat.com>
    Date:   Thu Mar 2 17:37:11 2017 +0900

        [PATCH v3 2/7] initial(): call cache_init() a bit early

        Call cache_init() before get_kcore_dump_loads(), because latter uses
        cache_search().

        Call path is like this :
        get_kcore_dump_loads() -> process_dump_load() -> vaddr_to_paddr() ->
        vtop4_x86_64() -> readmem() -> cache_search()

        Signed-off-by: Pratyush Anand <panand@redhat.com>

    :100644 100644 6942047199deb09dd1fff2121e264584dbb05587 3b8e9810468de26b0d8b73d456f0bd4f3d3aa2fe M      makedumpfile.c

In this timing, on sadump vmcores, info->page_size has not been
initialized yet so has 0. So, malloc() in cache_init() returns a chunk
of 0 size. A bit later, info->page_size is initialized with 4096.
Later processing on cache.c behaves assuming the chunk size is 8 *
4096. This destroys objects allocated after the chunk, resulting in
the above Segmentation fault.

To fix this issue, this commit moves setting info->page_size before
cache_init().

Signed-off-by: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
Cc: Pratyush Anand <panand@redhat.com>
---
 makedumpfile.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index 301772a..f300b19 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -3878,6 +3878,9 @@ initial(void)
 	if (!get_value_for_old_linux())
 		return FALSE;
 
+	if (info->flag_sadump && !set_page_size(sadump_page_size()))
+			return FALSE;
+
 	if (!is_xen_memory() && !cache_init())
 		return FALSE;
 
@@ -3906,9 +3909,6 @@ initial(void)
 			return FALSE;
 		}
 
-		if (!set_page_size(sadump_page_size()))
-			return FALSE;
-
 		if (!sadump_initialize_bitmap_memory())
 			return FALSE;
 
-- 
1.8.3.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH 1/2] sadump: set info->page_size before cache_init()
  2017-05-23  2:52 [PATCH 1/2] sadump: set info->page_size before cache_init() Hatayama, Daisuke
@ 2017-05-25  7:52 ` Pratyush Anand
  2017-05-26  7:46   ` Atsushi Kumagai
  0 siblings, 1 reply; 3+ messages in thread
From: Pratyush Anand @ 2017-05-25  7:52 UTC (permalink / raw)
  To: Hatayama, Daisuke, 'ats-kumagai@wm.jp.nec.com'
  Cc: 'kexec@lists.infradead.org'



On Tuesday 23 May 2017 08:22 AM, Hatayama, Daisuke wrote:
> Currently, makedumpfile results in Segmentation fault on sadump dump
> files as follows:
>
>     # LANG=C makedumpfile -f --message-level=31 -ld31 -x vmlinux ./sadump_vmcore sadump_vmcore-ld31
>     sadump: read dump device as single partition
>     sadump: single partition configuration
>     page_size    : 4096
>     sadump: timezone information is missing
>     Segmentation fault
>
> By bisect, I found that this issue is caused by the following commit
> that moves invocation of cache_init() in initial() a bit early:
>
>     # git bisect bad
>     8e2834bac4f62da3894da297f083068431be6d80 is the first bad commit
>     commit 8e2834bac4f62da3894da297f083068431be6d80
>     Author: Pratyush Anand <panand@redhat.com>
>     Date:   Thu Mar 2 17:37:11 2017 +0900
>
>         [PATCH v3 2/7] initial(): call cache_init() a bit early
>
>         Call cache_init() before get_kcore_dump_loads(), because latter uses
>         cache_search().
>
>         Call path is like this :
>         get_kcore_dump_loads() -> process_dump_load() -> vaddr_to_paddr() ->
>         vtop4_x86_64() -> readmem() -> cache_search()
>
>         Signed-off-by: Pratyush Anand <panand@redhat.com>
>
>     :100644 100644 6942047199deb09dd1fff2121e264584dbb05587 3b8e9810468de26b0d8b73d456f0bd4f3d3aa2fe M      makedumpfile.c
>
> In this timing, on sadump vmcores, info->page_size has not been
> initialized yet so has 0. So, malloc() in cache_init() returns a chunk
> of 0 size. A bit later, info->page_size is initialized with 4096.
> Later processing on cache.c behaves assuming the chunk size is 8 *
> 4096. This destroys objects allocated after the chunk, resulting in
> the above Segmentation fault.
>
> To fix this issue, this commit moves setting info->page_size before
> cache_init().
>
> Signed-off-by: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
> Cc: Pratyush Anand <panand@redhat.com>

For 1/2
Reviewed-by: Pratyush Anand <panand@redhat.com>


> ---
>  makedumpfile.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 301772a..f300b19 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -3878,6 +3878,9 @@ initial(void)
>  	if (!get_value_for_old_linux())
>  		return FALSE;
>
> +	if (info->flag_sadump && !set_page_size(sadump_page_size()))
> +			return FALSE;
> +
>  	if (!is_xen_memory() && !cache_init())
>  		return FALSE;
>
> @@ -3906,9 +3909,6 @@ initial(void)
>  			return FALSE;
>  		}
>
> -		if (!set_page_size(sadump_page_size()))
> -			return FALSE;
> -
>  		if (!sadump_initialize_bitmap_memory())
>  			return FALSE;
>
>

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply	[flat|nested] 3+ messages in thread
* RE: [PATCH 1/2] sadump: set info->page_size before cache_init()
  2017-05-25  7:52 ` Pratyush Anand
@ 2017-05-26  7:46   ` Atsushi Kumagai
  0 siblings, 0 replies; 3+ messages in thread
From: Atsushi Kumagai @ 2017-05-26  7:46 UTC (permalink / raw)
  To: Pratyush Anand, Hatayama, Daisuke; +Cc: 'kexec@lists.infradead.org'

>On Tuesday 23 May 2017 08:22 AM, Hatayama, Daisuke wrote:
>> Currently, makedumpfile results in Segmentation fault on sadump dump
>> files as follows:
>>
>>     # LANG=C makedumpfile -f --message-level=31 -ld31 -x vmlinux ./sadump_vmcore sadump_vmcore-ld31
>>     sadump: read dump device as single partition
>>     sadump: single partition configuration
>>     page_size    : 4096
>>     sadump: timezone information is missing
>>     Segmentation fault
>>
>> By bisect, I found that this issue is caused by the following commit
>> that moves invocation of cache_init() in initial() a bit early:
>>
>>     # git bisect bad
>>     8e2834bac4f62da3894da297f083068431be6d80 is the first bad commit
>>     commit 8e2834bac4f62da3894da297f083068431be6d80
>>     Author: Pratyush Anand <panand@redhat.com>
>>     Date:   Thu Mar 2 17:37:11 2017 +0900
>>
>>         [PATCH v3 2/7] initial(): call cache_init() a bit early
>>
>>         Call cache_init() before get_kcore_dump_loads(), because latter uses
>>         cache_search().
>>
>>         Call path is like this :
>>         get_kcore_dump_loads() -> process_dump_load() -> vaddr_to_paddr() ->
>>         vtop4_x86_64() -> readmem() -> cache_search()
>>
>>         Signed-off-by: Pratyush Anand <panand@redhat.com>
>>
>>     :100644 100644 6942047199deb09dd1fff2121e264584dbb05587 3b8e9810468de26b0d8b73d456f0bd4f3d3aa2fe M
>makedumpfile.c
>>
>> In this timing, on sadump vmcores, info->page_size has not been
>> initialized yet so has 0. So, malloc() in cache_init() returns a chunk
>> of 0 size. A bit later, info->page_size is initialized with 4096.
>> Later processing on cache.c behaves assuming the chunk size is 8 *
>> 4096. This destroys objects allocated after the chunk, resulting in
>> the above Segmentation fault.
>>
>> To fix this issue, this commit moves setting info->page_size before
>> cache_init().
>>
>> Signed-off-by: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
>> Cc: Pratyush Anand <panand@redhat.com>
>
>For 1/2
>Reviewed-by: Pratyush Anand <panand@redhat.com>

Thanks for your work, I'll merge this patch into v1.6.2.

Regards,
Atsushi Kumagai

>> ---
>>  makedumpfile.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/makedumpfile.c b/makedumpfile.c
>> index 301772a..f300b19 100644
>> --- a/makedumpfile.c
>> +++ b/makedumpfile.c
>> @@ -3878,6 +3878,9 @@ initial(void)
>>  	if (!get_value_for_old_linux())
>>  		return FALSE;
>>
>> +	if (info->flag_sadump && !set_page_size(sadump_page_size()))
>> +			return FALSE;
>> +
>>  	if (!is_xen_memory() && !cache_init())
>>  		return FALSE;
>>
>> @@ -3906,9 +3909,6 @@ initial(void)
>>  			return FALSE;
>>  		}
>>
>> -		if (!set_page_size(sadump_page_size()))
>> -			return FALSE;
>> -
>>  		if (!sadump_initialize_bitmap_memory())
>>  			return FALSE;
>>
>>
>
>_______________________________________________
>kexec mailing list
>kexec@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/kexec


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-05-26  7:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-23  2:52 [PATCH 1/2] sadump: set info->page_size before cache_init() Hatayama, Daisuke
2017-05-25  7:52 ` Pratyush Anand
2017-05-26  7:46   ` Atsushi Kumagai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox