This patchset fixes various issues found working with the removal of the zero page mapping for SPARC32/PPC in order to catch NULL pointer dereferences.
With all patches applied, I see no regressions in any of my SPARC32 or PPC test images.
Mark Cave-Ayland (7): SPARC32: Add debugging output for calls to the romvec obp_memalloc() and obp_dumb_memalloc() functions. OFMEM: Fix selection of reusable memory areas from the internal malloc() freelist. Remove /chosen "memory" property from the default device tree. OFMEM: Fix bad alignments passed into the OFMEM claim words SPARC32: Remove manual fix for bad alignments passed into the romvec malloc() functions SPARC32: Remove zero page mapping from MMU to enable detection of NULL pointer dereferences PPC: Remove zero page mapping from MMU to enable detection of NULL pointer dereferences
openbios-devel/arch/ppc/qemu/ofmem.c | 11 ++++++--- openbios-devel/arch/sparc32/init.fs | 2 +- openbios-devel/arch/sparc32/lib.c | 34 ++++++++++----------------- openbios-devel/arch/sparc32/ofmem_sparc32.c | 3 +++ openbios-devel/forth/device/tree.fs | 1 - openbios-devel/libopenbios/ofmem_common.c | 22 +++++++++++++---- 6 files changed, 42 insertions(+), 31 deletions(-)
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk --- openbios-devel/arch/sparc32/lib.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/openbios-devel/arch/sparc32/lib.c b/openbios-devel/arch/sparc32/lib.c index c0df654..f8fa55a 100644 --- a/openbios-devel/arch/sparc32/lib.c +++ b/openbios-devel/arch/sparc32/lib.c @@ -269,6 +269,8 @@ char *obp_memalloc(char *va, unsigned int size, unsigned int align) phys_addr_t phys; ucell virt;
+ DPRINTF("obp_memalloc: virta 0x%x, sz %d, align %d\n", (unsigned int)va, size, align); + /* Claim physical memory */ phys = ofmem_claim_phys(-1, size, align);
@@ -286,6 +288,8 @@ char *obp_dumb_memalloc(char *va, unsigned int size) unsigned long align; int i;
+ DPRINTF("obp_dumb_memalloc: virta 0x%x, sz %d\n", (unsigned int)va, size); + /* Solaris seems to assume that the returned value is physically aligned to size. For example, not having this here causes the Solaris 8 kernel to fault because the IOMMU page table base address is calculated incorrectly. */
The existing code would incorrectly allow freelist memory to be reused if the requested size were 0x1000 greater than the freelist item size, rather than the freelist item size being 0x1000 greater than the requested size.
Since internal memory allocations could be smaller than requested, it would be possible for a caller to clobber over the internal memory heap causing a crash or internal memory corruption.
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk --- openbios-devel/libopenbios/ofmem_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/openbios-devel/libopenbios/ofmem_common.c b/openbios-devel/libopenbios/ofmem_common.c index bae0732..4c464c8 100644 --- a/openbios-devel/libopenbios/ofmem_common.c +++ b/openbios-devel/libopenbios/ofmem_common.c @@ -107,7 +107,7 @@ int ofmem_posix_memalign( void **memptr, size_t alignment, size_t size ) }
/* waste at most 4K by taking an entry from the freelist */ - if( *pp && (**pp).size < size + 0x1000 ) { + if( *pp && (**pp).size > size + 0x1000 ) { /* Alignment should be on physical not virtual address */ pa = va2pa((uintptr_t)*pp + sizeof(alloc_desc_t)); pa = align_ptr(pa, alignment);
Similar to the earlier commit for "mmu", do the same for /chosen "memory" property. Hence all architectures that want the "pretty" memory properties can provide suitable ihandles if required, and those that don't will not fail with a NULL pointer dereference from a zero ihandle.
While we're here, correct the SPARC32 initialiser to point to the correct /virtual-memory node for "mmu" so that once again we can get "pretty" memory properties on SPARC32.
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk --- openbios-devel/arch/sparc32/init.fs | 2 +- openbios-devel/forth/device/tree.fs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/openbios-devel/arch/sparc32/init.fs b/openbios-devel/arch/sparc32/init.fs index 7306eab..26ffa05 100644 --- a/openbios-devel/arch/sparc32/init.fs +++ b/openbios-devel/arch/sparc32/init.fs @@ -32,7 +32,7 @@ \ preopen device nodes (and store the ihandles under /chosen) :noname " memory" " /memory" preopen - " mmu" " /cpus/@0" preopen + " mmu" " /virtual-memory" preopen ; SYSTEM-initializer
device-end diff --git a/openbios-devel/forth/device/tree.fs b/openbios-devel/forth/device/tree.fs index 00d12ed..04f85b5 100644 --- a/openbios-devel/forth/device/tree.fs +++ b/openbios-devel/forth/device/tree.fs @@ -53,7 +53,6 @@ new-device 0 encode-int " stdout" property \ " hda1:/boot/vmunix" encode-string " bootpath" property \ " -as" encode-string " bootargs" property - 0 encode-int " memory" property finish-device
\ END
If a memory claim is made with an alignment that is not an exact power of 2, round up to the nearest power of 2 as per the IEEE1275 specification rather than switching to a 4K default.
Also we make sure that the minimum alignment is equivalent to PAGE_SIZE.
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk --- openbios-devel/libopenbios/ofmem_common.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/openbios-devel/libopenbios/ofmem_common.c b/openbios-devel/libopenbios/ofmem_common.c index 4c464c8..ee8ee82 100644 --- a/openbios-devel/libopenbios/ofmem_common.c +++ b/openbios-devel/libopenbios/ofmem_common.c @@ -425,13 +425,27 @@ static ucell find_area( ucell align, ucell size, range_t *r, { phys_addr_t base = min; range_t *r2; + ucell old_align; + int i;
if( (align & (align-1)) ) { - OFMEM_TRACE("bad alignment " FMT_ucell "\n", align); - align = 0x1000; + + /* As per IEEE1275 specification, round up to the nearest power of 2 */ + old_align = align; + if (old_align <= PAGE_SIZE) { + align = PAGE_SIZE; + } else { + align--; + for (i = 1; i < sizeof(ucell) * 8; i<<=1) { + align |= align >> i; + } + align++; + } + + OFMEM_TRACE("warning: bad alignment " FMT_ucellx " rounded up to " FMT_ucellx "\n", old_align, align); } if( !align ) - align = 0x1000; + align = PAGE_SIZE;
base = reverse ? max - size : min; r2 = reverse ? NULL : r;
Since the handling of bad alignments is now internal to OFMEM, there is no need for the SPARC32 malloc() functions to have to do this any more.
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk --- openbios-devel/arch/sparc32/lib.c | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-)
diff --git a/openbios-devel/arch/sparc32/lib.c b/openbios-devel/arch/sparc32/lib.c index f8fa55a..350cd08 100644 --- a/openbios-devel/arch/sparc32/lib.c +++ b/openbios-devel/arch/sparc32/lib.c @@ -285,28 +285,14 @@ char *obp_memalloc(char *va, unsigned int size, unsigned int align)
char *obp_dumb_memalloc(char *va, unsigned int size) { - unsigned long align; - int i; + unsigned long align = size;
DPRINTF("obp_dumb_memalloc: virta 0x%x, sz %d\n", (unsigned int)va, size);
- /* Solaris seems to assume that the returned value is physically aligned to size. For - example, not having this here causes the Solaris 8 kernel to fault because the - IOMMU page table base address is calculated incorrectly. */ - - /* Enforce a minimum alignment of CONFIG_OFMEM_MALLOC_ALIGN, and choose an alignment - which is the next power of 2 higher than the specified size */ - align = size; - if (align <= CONFIG_OFMEM_MALLOC_ALIGN) { - align = CONFIG_OFMEM_MALLOC_ALIGN; - } else { - align--; - for (i = 1; i < sizeof(unsigned long) * 8; i<<=1) { - align |= align >> i; - } - align++; - } - + /* Solaris seems to assume that the returned value is physically aligned to size. + e.g. it is used for setting up page tables. Fortunately this is now handled by + ofmem_claim_phys() above. */ + return obp_memalloc(va, size, align); }
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk Acked-by: Artyom Tarasenko atar4qemu@gmail.com --- openbios-devel/arch/sparc32/lib.c | 6 +++--- openbios-devel/arch/sparc32/ofmem_sparc32.c | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/openbios-devel/arch/sparc32/lib.c b/openbios-devel/arch/sparc32/lib.c index 350cd08..81d30ad 100644 --- a/openbios-devel/arch/sparc32/lib.c +++ b/openbios-devel/arch/sparc32/lib.c @@ -393,9 +393,9 @@ init_mmu_swift(void) ofmem_arch_map_pages(pa, va, size, ofmem_arch_default_translation_mode(pa)); ofmem_map_page_range(pa, va, size, ofmem_arch_default_translation_mode(pa));
- // 1:1 mapping for RAM - ofmem_arch_map_pages(0, 0, LOWMEMSZ, ofmem_arch_default_translation_mode(0)); - ofmem_map_page_range(0, 0, LOWMEMSZ, ofmem_arch_default_translation_mode(0)); + // 1:1 mapping for RAM (don't map page 0 to allow catching of NULL dereferences) + ofmem_arch_map_pages(PAGE_SIZE, PAGE_SIZE, LOWMEMSZ - PAGE_SIZE, ofmem_arch_default_translation_mode(0)); + ofmem_map_page_range(PAGE_SIZE, PAGE_SIZE, LOWMEMSZ - PAGE_SIZE, ofmem_arch_default_translation_mode(0));
/* * Flush cache diff --git a/openbios-devel/arch/sparc32/ofmem_sparc32.c b/openbios-devel/arch/sparc32/ofmem_sparc32.c index 54cb766..2767b7b 100644 --- a/openbios-devel/arch/sparc32/ofmem_sparc32.c +++ b/openbios-devel/arch/sparc32/ofmem_sparc32.c @@ -238,6 +238,9 @@ void ofmem_init( void ) memset(&s_ofmem_data, 0, sizeof(s_ofmem_data)); s_ofmem_data.ofmem.ramsize = qemu_mem_size; + /* Mark the first page as non-free */ + ofmem_claim_virt(0, PAGE_SIZE, 0); + /* Claim reserved physical addresses at top of RAM */ ofmem_claim_phys(ofmem_arch_get_phys_top(), s_ofmem_data.ofmem.ramsize - ofmem_arch_get_phys_top(), 0);
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk --- openbios-devel/arch/ppc/qemu/ofmem.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/openbios-devel/arch/ppc/qemu/ofmem.c b/openbios-devel/arch/ppc/qemu/ofmem.c index 0161a17..20e9a1a 100644 --- a/openbios-devel/arch/ppc/qemu/ofmem.c +++ b/openbios-devel/arch/ppc/qemu/ofmem.c @@ -549,9 +549,14 @@ ofmem_init(void) { ofmem_t *ofmem = ofmem_arch_get_private();
- ofmem_claim_phys(0, get_ram_bottom(), 0); - ofmem_claim_virt(0, get_ram_bottom(), 0); - ofmem_map(0, 0, get_ram_bottom(), 0); + /* Map the memory (don't map page 0 to allow catching of NULL dereferences) */ + ofmem_claim_phys(PAGE_SIZE, get_ram_bottom() - PAGE_SIZE, 0); + ofmem_claim_virt(PAGE_SIZE, get_ram_bottom() - PAGE_SIZE, 0); + ofmem_map(PAGE_SIZE, PAGE_SIZE, get_ram_bottom() - PAGE_SIZE, 0); + + /* Mark the first page as non-free */ + ofmem_claim_phys(0, PAGE_SIZE, 0); + ofmem_claim_virt(0, PAGE_SIZE, 0);
/* Map everything at the top of physical RAM 1:1, minus the OpenBIOS ROM in RAM copy */ ofmem_claim_phys(get_ram_top(), get_hash_base() + HASH_SIZE - get_ram_top(), 0);
On Fri, Apr 12, 2013 at 12:57 PM, Mark Cave-Ayland mark.cave-ayland@ilande.co.uk wrote:
This patchset fixes various issues found working with the removal of the zero page mapping for SPARC32/PPC in order to catch NULL pointer dereferences.
With all patches applied, I see no regressions in any of my SPARC32 or PPC test images.
I don't see any problems in my tests either.
Mark Cave-Ayland (7): SPARC32: Add debugging output for calls to the romvec obp_memalloc() and obp_dumb_memalloc() functions. OFMEM: Fix selection of reusable memory areas from the internal malloc() freelist. Remove /chosen "memory" property from the default device tree. OFMEM: Fix bad alignments passed into the OFMEM claim words SPARC32: Remove manual fix for bad alignments passed into the romvec malloc() functions SPARC32: Remove zero page mapping from MMU to enable detection of NULL pointer dereferences PPC: Remove zero page mapping from MMU to enable detection of NULL pointer dereferences
openbios-devel/arch/ppc/qemu/ofmem.c | 11 ++++++--- openbios-devel/arch/sparc32/init.fs | 2 +- openbios-devel/arch/sparc32/lib.c | 34 ++++++++++----------------- openbios-devel/arch/sparc32/ofmem_sparc32.c | 3 +++ openbios-devel/forth/device/tree.fs | 1 - openbios-devel/libopenbios/ofmem_common.c | 22 +++++++++++++---- 6 files changed, 42 insertions(+), 31 deletions(-)
-- 1.7.10.4
-- OpenBIOS http://openbios.org/ Mailinglist: http://lists.openbios.org/mailman/listinfo Free your System - May the Forth be with you
-
Blue Swirl -
Mark Cave-Ayland