Possible fix for kexec-tools dynamic range allocation
Subsystems (2)
PATCH 1/1 Possible fix for kexec-tools dynamic range allocation
Hi all,
The patch to dynamically allocate memory regions for ppc64 kexec-tools, ie. "ppc64: kexec memory ranges dynamic allocation" (d182ce5), has never worked AFAICT.
Chandru reported it as broken when it was posted:
http://lists.infradead.org/pipermail/kexec/2008-October/002751.html
Still, it's in now, and I'm trying to work out what's going wrong.
The symptom is as reported by Chandru, we end up not being able to allocate any memory (in locate_hole()). This is caused by the list of memory_ranges being empty.
The memory_ranges are empty because they have been realloc'ed (by the dynamic alloc code), and the generic code is still looking at the old version.
What I'm not clear on is why the ppc64 code is even calling setup_memory_ranges() a second time (in elf_ppc64_load()). It's already been called by get_memory_ranges() from my_load(). Or is there another route into elf_ppc64_load() that I'm not seeing?
And in fact if I just remove that call, then everything is peachy.
The following patch makes it work for me at least.
cheers
Quoted 1 line
From
40958d8f957876ca612b666f40bebf28ea335314Mon Sep 17 00:00:00 2001
From: Michael Ellerman <[email protected]>
Date: Wed, 7 Jan 2009 16:57:05 +1100
Subject: [PATCH] Don't call setup_memory_ranges() again
There's no need to call setup_memory_ranges() again. Furthermore it can lead to the memory_ranges being realloc'ed, which results in the generic code (locate_hole() etc.) using the free'd memory_ranges.
--- kexec/arch/ppc64/kexec-elf-ppc64.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
kexec/arch/ppc64/kexec-elf-ppc64.c +0 -2
@@ -151,8 +151,6 @@ int elf_ppc64_load(int argc, char **argv, const char *buf, off_t len,if (ramdisk && reuse_initrd)die ("Can't specify --ramdisk or --initrd with --reuseinitrd\n" ); -setup_memory_ranges (info->kexec_flags); -/* Need to append some command line parameters internally in case of * taking crash dumps. */
Signature · 1 line
1.5.3.7.1.g4e596e
Quoted 19 lines · Michael Ellerman
Hi all,
The patch to dynamically allocate memory regions for ppc64 kexec-tools, ie. "ppc64: kexec memory ranges dynamic allocation" (d182ce5), has never worked AFAICT.
Chandru reported it as broken when it was posted:
http://lists.infradead.org/pipermail/kexec/2008-October/002751.htmlStill, it's in now, and I'm trying to work out what's going wrong.
The symptom is as reported by Chandru, we end up not being able to allocate any memory (in
locate_hole()). This is caused by the list ofmemory_rangesbeing empty.The
memory_rangesare empty because they have been realloc'ed (by the dynamic alloc code), and the generic code is still looking at the old version.
Quoted 8 lines
What I'm not clear on is why the ppc64 code is even calling
setup_memory_ranges()a second time (inelf_ppc64_load()). It's already been called byget_memory_ranges()frommy_load(). Or is there another route intoelf_ppc64_load()that I'm not seeing?And in fact if I just remove that call, then everything is peachy.
The following patch makes it work for me at least.
Hi Michael,
I must confess that I don't have a complete understanding of this problem. Does Bernhard's recent patch (sorry that I applied it even though it came in after your patch) help this problem?
commit 95c74405638c786bc76fbca5e4e8427dfe26e907
Author: Bernhard Walle <[email protected]>
Date: Fri Jan 16 19:11:34 2009 +0100
Subject: Fix memory corruption when using realloc_memory_ranges()
Quoted 31 lines
Quoted 1 line
From
40958d8f957876ca612b666f40bebf28ea335314Mon Sep 17 00:00:00 2001From: Michael Ellerman <[email protected]>
Date: Wed, 7 Jan 2009 16:57:05 +1100
Subject: [PATCH] Don't callsetup_memory_ranges()againThere's no need to call
setup_memory_ranges()again. Furthermore it can lead to thememory_rangesbeing realloc'ed, which results in the generic code (locate_hole()etc.) using the free'dmemory_ranges.Signed-off-by Michael Ellerman <[email protected]>--- kexec/arch/ppc64/kexec-elf-ppc64.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)kexec/arch/ppc64/kexec-elf-ppc64.c +0
-2@@ -151,8 +151,6 @@ int elf_ppc64_load(int argc, char **argv, const char *buf, off_t len,if (ramdisk && reuse_initrd)die ("Can't specify --ramdisk or --initrd with --reuseinitrd\n" ); -setup_memory_ranges (info->kexec_flags); -/* Need to append some command line parameters internally in case of * taking crash dumps. */ Signature · 1 line
1.5.3.7.1.g4e596e
Signature · 3 lines
Simon Horman VA Linux Systems Japan K.K., Sydney, Australia Satellite Office H: www.vergenet.net/~horms/ W: www.valinux.co.jp/en
Quoted 27 lines · Simon Horman
Quoted 26 lines · Michael Ellerman
Hi all,
The patch to dynamically allocate memory regions for ppc64 kexec-tools, ie. "ppc64: kexec memory ranges dynamic allocation" (d182ce5), has never worked AFAICT.
Chandru reported it as broken when it was posted:
http://lists.infradead.org/pipermail/kexec/2008-October/002751.htmlStill, it's in now, and I'm trying to work out what's going wrong.
The symptom is as reported by Chandru, we end up not being able to allocate any memory (in
locate_hole()). This is caused by the list ofmemory_rangesbeing empty.The
memory_rangesare empty because they have been realloc'ed (by the dynamic alloc code), and the generic code is still looking at the old version.What I'm not clear on is why the ppc64 code is even calling
setup_memory_ranges()a second time (inelf_ppc64_load()). It's already been called byget_memory_ranges()frommy_load(). Or is there another route intoelf_ppc64_load()that I'm not seeing?And in fact if I just remove that call, then everything is peachy.
Quoted 7 lines
Quoted 1 line
The following patch makes it work for me at least.
Hi Michael,
I must confess that I don't have a complete understanding of this problem. Does Bernhard's recent patch (sorry that I applied it even though it came in after your patch) help this problem?
Hi Horms,
Well to be honest neither do I, I was hoping someone who'd written or helped write the original code would comment.
Bernhard's patch will help, but I think mine is a better solution.
Quoted 9 lines
commit
95c74405638c786bc76fbca5e4e8427dfe26e907
Author: Bernhard Walle <[email protected]>
Date: Fri Jan 16 19:11:34 2009 +0100
Subject: Fix memory corruption when usingrealloc_memory_ranges()Because
realloc_memory_ranges()makes the old memory invalid, and we return a pointer tomemory_rangeinget_memory_ranges(), we need to copy the contents inget_memory_ranges().
Quoted 8 lines
Some code that calls
realloc_memory_ranges()may be triggered byget_base_ranges()which is called afterget_memory_ranges().Yes, the memory needs to be deleted somewhere, but I don't know currently where it's the best, and since it's not in a loop and memory is deleted anyway after program termination I don't want to introduce unneccessary complexity. The problem is that
get_base_ranges()gets called from architecture independent code and that allocation is PPC64-specific here.
I don't see where get_base_ranges() is called other than from PPC64 code, so I'm confused about this comment.
What I see happening is:
* get_memory_ranges() is called early in kexec.c and saves a pointer to the memory ranges in "info".
* Any subsequent call which causes the memory ranges to be realloc'ed will screw that up, because now info will point at free'd memory.
* Later on in elf_ppc64_load() we call setup_memory_ranges() (again).
* That may cause the ranges to be realloc'ed, which would be bad.
* But the second call to setup_memory_ranges() is useless, because it doesn't update info, and info is what we keep using for allocations.
* So if setup_memory_ranges() found new ranges, they would never be used, even apart from the corruption issue. So we may as well not call it.
* If there are /other/ code paths where we can realloc memory ranges then maybe we /also/ need Bernhard's patch.
But that was only a 10 minute analysis, so maybe I'm wrong ;)
cheers
Signature · 8 lines
Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person
Quoted 10 lines · Simon Horman
Does Bernhard's recent patch (sorry that I applied it even though it came in after your patch) help this problem?
commit
95c74405638c786bc76fbca5e4e8427dfe26e907
Author: Bernhard Walle <[email protected]>
Date: Fri Jan 16 19:11:34 2009 +0100
Subject: Fix memory corruption when usingrealloc_memory_ranges()
'git clone' or 'git pull' isn't showing these patches. The last commit that I can see is : 6d4cdf4f94bf7ad2d1752b553ea948385881ff79 and search by that commit doesn't yield anything :(.
Thanks,
Chandru
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Quoted 11 lines · Chandru
Quoted 10 lines · Simon Horman
Does Bernhard's recent patch (sorry that I applied it even though it came in after your patch) help this problem?
commit
95c74405638c786bc76fbca5e4e8427dfe26e907
Author: Bernhard Walle <[email protected]>
Date: Fri Jan 16 19:11:34 2009 +0100
Subject: Fix memory corruption when usingrealloc_memory_ranges()
Quoted 3 lines
'git clone' or 'git pull' isn't showing these patches. The last commit that I can see is :
6d4cdf4f94bf7ad2d1752b553ea948385881ff79and search by that commit doesn't yield anything :(.
Sorry, I had forgotten to push. The changes should be there now.
Signature · 9 lines
Simon Horman VA Linux Systems Japan K.K., Sydney, Australia Satellite Office H: www.vergenet.net/~horms/ W: www.valinux.co.jp/en _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec