April 2013 - OpenBIOS - mail.coreboot.org

[commit] r1123 - trunk/openbios-devel/arch/sparc32
by repository service April 19, 2013

April 19, 2013

Author: mcayland Date: Fri Apr 19 09:04:32 2013 New Revision: 1123 URL: http://tracker.coreboot.org/trac/openbios/changeset/1123 Log: SPARC32: Remove zero page mapping from MMU to enable detection of NULL pointer dereferences Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> Acked-by: Artyom Tarasenko <atar4qemu(a)gmail.com> Modified: trunk/openbios-devel/arch/sparc32/lib.c trunk/openbios-devel/arch/sparc32/ofmem_sparc32.c Modified: trunk/openbios-devel/arch/sparc32/lib.c ============================================================================== --- trunk/openbios-devel/arch/sparc32/lib.c Fri Apr 19 09:04:25 2013 (r1122) +++ trunk/openbios-devel/arch/sparc32/lib.c Fri Apr 19 09:04:32 2013 (r1123) @@ -393,9 +393,9 @@ 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 Modified: trunk/openbios-devel/arch/sparc32/ofmem_sparc32.c ============================================================================== --- trunk/openbios-devel/arch/sparc32/ofmem_sparc32.c Fri Apr 19 09:04:25 2013 (r1122) +++ trunk/openbios-devel/arch/sparc32/ofmem_sparc32.c Fri Apr 19 09:04:32 2013 (r1123) @@ -238,6 +238,9 @@ 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);

1 0

[commit] r1122 - trunk/openbios-devel/arch/sparc32
by repository service April 19, 2013

April 19, 2013

Author: mcayland Date: Fri Apr 19 09:04:25 2013 New Revision: 1122 URL: http://tracker.coreboot.org/trac/openbios/changeset/1122 Log: SPARC32: Remove manual fix for bad alignments passed into the romvec malloc() functions 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(a)ilande.co.uk> Modified: trunk/openbios-devel/arch/sparc32/lib.c Modified: trunk/openbios-devel/arch/sparc32/lib.c ============================================================================== --- trunk/openbios-devel/arch/sparc32/lib.c Fri Apr 19 09:03:59 2013 (r1121) +++ trunk/openbios-devel/arch/sparc32/lib.c Fri Apr 19 09:04:25 2013 (r1122) @@ -285,28 +285,14 @@ 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); }

1 0

[commit] r1121 - trunk/openbios-devel/libopenbios
by repository service April 19, 2013

April 19, 2013

Author: mcayland Date: Fri Apr 19 09:03:59 2013 New Revision: 1121 URL: http://tracker.coreboot.org/trac/openbios/changeset/1121 Log: OFMEM: Fix bad alignments passed into the OFMEM claim words 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(a)ilande.co.uk> Modified: trunk/openbios-devel/libopenbios/ofmem_common.c Modified: trunk/openbios-devel/libopenbios/ofmem_common.c ============================================================================== --- trunk/openbios-devel/libopenbios/ofmem_common.c Fri Apr 19 09:03:38 2013 (r1120) +++ trunk/openbios-devel/libopenbios/ofmem_common.c Fri Apr 19 09:03:59 2013 (r1121) @@ -425,13 +425,27 @@ { 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;

1 0

[commit] r1120 - in trunk/openbios-devel: arch/sparc32 forth/device
by repository service April 19, 2013

April 19, 2013

Author: mcayland Date: Fri Apr 19 09:03:38 2013 New Revision: 1120 URL: http://tracker.coreboot.org/trac/openbios/changeset/1120 Log: Remove /chosen "memory" property from the default device tree. 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(a)ilande.co.uk> Modified: trunk/openbios-devel/arch/sparc32/init.fs trunk/openbios-devel/forth/device/tree.fs Modified: trunk/openbios-devel/arch/sparc32/init.fs ============================================================================== --- trunk/openbios-devel/arch/sparc32/init.fs Fri Apr 19 09:03:26 2013 (r1119) +++ trunk/openbios-devel/arch/sparc32/init.fs Fri Apr 19 09:03:38 2013 (r1120) @@ -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 Modified: trunk/openbios-devel/forth/device/tree.fs ============================================================================== --- trunk/openbios-devel/forth/device/tree.fs Fri Apr 19 09:03:26 2013 (r1119) +++ trunk/openbios-devel/forth/device/tree.fs Fri Apr 19 09:03:38 2013 (r1120) @@ -53,7 +53,6 @@ 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

1 0

[commit] r1119 - trunk/openbios-devel/libopenbios
by repository service April 19, 2013

April 19, 2013

Author: mcayland Date: Fri Apr 19 09:03:26 2013 New Revision: 1119 URL: http://tracker.coreboot.org/trac/openbios/changeset/1119 Log: OFMEM: Fix selection of reusable memory areas from the internal malloc() freelist. 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(a)ilande.co.uk> Modified: trunk/openbios-devel/libopenbios/ofmem_common.c Modified: trunk/openbios-devel/libopenbios/ofmem_common.c ============================================================================== --- trunk/openbios-devel/libopenbios/ofmem_common.c Fri Apr 19 09:03:20 2013 (r1118) +++ trunk/openbios-devel/libopenbios/ofmem_common.c Fri Apr 19 09:03:26 2013 (r1119) @@ -107,7 +107,7 @@ } /* 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);

1 0

[commit] r1118 - trunk/openbios-devel/arch/sparc32
by repository service April 19, 2013

April 19, 2013

Author: mcayland Date: Fri Apr 19 09:03:20 2013 New Revision: 1118 URL: http://tracker.coreboot.org/trac/openbios/changeset/1118 Log: SPARC32: Add debugging output for calls to the romvec obp_memalloc() and obp_dumb_memalloc() functions. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> Modified: trunk/openbios-devel/arch/sparc32/lib.c Modified: trunk/openbios-devel/arch/sparc32/lib.c ============================================================================== --- trunk/openbios-devel/arch/sparc32/lib.c Tue Apr 9 21:25:48 2013 (r1117) +++ trunk/openbios-devel/arch/sparc32/lib.c Fri Apr 19 09:03:20 2013 (r1118) @@ -269,6 +269,8 @@ 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 @@ 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. */

1 0

[PATCH 0/7] Assorted OpenBIOS memory fixes
by Mark Cave-Ayland April 14, 2013

April 14, 2013

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(-) -- 1.7.10.4

2 8

Re: [OpenBIOS] [PATCH 2/2] SPARC64: Remove limit on kernel command line length caused by using a static buffer
by Mark Cave-Ayland April 13, 2013

April 13, 2013

On 13/04/13 10:07, Blue Swirl wrote: >> Instead use strdup() so that we can dynamically allocate the string ourselves. As > > Actually there's no call to strdup()... > >> reported by Rob Landley<rob(a)landley.net>. >> >> Signed-off-by: Mark Cave-Ayland<mark.cave-ayland(a)ilande.co.uk> >> --- >> openbios-devel/arch/sparc64/openbios.c | 11 ++++------- >> 1 file changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/openbios-devel/arch/sparc64/openbios.c b/openbios-devel/arch/sparc64/openbios.c >> index 3b372b9..440e505 100644 >> --- a/openbios-devel/arch/sparc64/openbios.c >> +++ b/openbios-devel/arch/sparc64/openbios.c >> @@ -42,9 +42,6 @@ >> #define NVRAM_OB_START (0) >> #define NVRAM_OB_SIZE ((0x1fd0 - NVRAM_OB_START)& ~15) >> >> -#define OBIO_CMDLINE_MAX 256 >> -static char obio_cmdline[OBIO_CMDLINE_MAX]; >> - >> static uint8_t idprom[NVRAM_IDPROM_SIZE]; >> >> struct hwdef { >> @@ -370,6 +367,7 @@ static uint8_t qemu_uuid[16]; >> >> void arch_nvram_get(char *data) >> { >> + char *obio_cmdline = { '\0' }; > > Isn't this allocated in the stack? So ... > >> uint32_t size = 0; >> const struct cpudef *cpu; >> char buf[256]; >> @@ -401,12 +399,11 @@ void arch_nvram_get(char *data) >> kernel_image = fw_cfg_read_i64(FW_CFG_KERNEL_ADDR); >> >> size = fw_cfg_read_i32(FW_CFG_CMDLINE_SIZE); >> - if (size> OBIO_CMDLINE_MAX - 1) >> - size = OBIO_CMDLINE_MAX - 1; >> if (size) { >> + obio_cmdline = (char *)malloc(size + 1); >> fw_cfg_read(FW_CFG_CMDLINE_DATA, obio_cmdline, size); >> - } >> - obio_cmdline[size] = '\0'; >> + obio_cmdline[size] = '\0'; >> + } > > ... I'd add an 'else' case here with something like obio_cmdline = strdup(""). > >> qemu_cmdline = (uint64_t)obio_cmdline; >> cmdline_size = size; >> boot_device = fw_cfg_read_i16(FW_CFG_BOOT_DEVICE); Thanks for the feedback - I've just posted a revised v2 patch to the list based upon your comments. ATB, Mark.

1 0

[v2 1/2] SPARC32: Remove limit on kernel command line length caused by using a static buffer
by Mark Cave-Ayland April 13, 2013

April 13, 2013

Instead use strdup() so that we can dynamically allocate the string ourselves. As reported by Rob Landley <rob(a)landley.net>. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> --- openbios-devel/arch/sparc32/openbios.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/openbios-devel/arch/sparc32/openbios.c b/openbios-devel/arch/sparc32/openbios.c index 95a5d37..de23a6f 100644 --- a/openbios-devel/arch/sparc32/openbios.c +++ b/openbios-devel/arch/sparc32/openbios.c @@ -835,8 +835,7 @@ static void init_memory(void) static void arch_init( void ) { - static char cmdline[128]; - int size = 0; + char *cmdline; const char *kernel_cmdline; uint32_t temp; uint16_t machine_id; @@ -897,11 +896,11 @@ arch_init( void ) kernel_cmdline = (const char *) fw_cfg_read_i32(FW_CFG_KERNEL_CMDLINE); if (kernel_cmdline) { - size = strlen(kernel_cmdline); - memcpy(cmdline, kernel_cmdline, size); + cmdline = strdup(kernel_cmdline); obp_arg.argv[1] = cmdline; - } - cmdline[size] = '\0'; + } else { + cmdline = strdup(""); + } qemu_cmdline = (uint32_t)cmdline; /* Setup nvram variables */ -- 1.7.10.4

1 1

[PATCH 1/2] SPARC32: Remove limit on kernel command line length caused by using a static buffer
by Mark Cave-Ayland April 13, 2013

April 13, 2013

Instead use strdup() so that we can dynamically allocate the string ourselves. As reported by Rob Landley <rob(a)landley.net>. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland(a)ilande.co.uk> --- openbios-devel/arch/sparc32/openbios.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/openbios-devel/arch/sparc32/openbios.c b/openbios-devel/arch/sparc32/openbios.c index 95a5d37..56bafe8 100644 --- a/openbios-devel/arch/sparc32/openbios.c +++ b/openbios-devel/arch/sparc32/openbios.c @@ -835,8 +835,7 @@ static void init_memory(void) static void arch_init( void ) { - static char cmdline[128]; - int size = 0; + char *cmdline = { '\0' }; const char *kernel_cmdline; uint32_t temp; uint16_t machine_id; @@ -897,11 +896,9 @@ arch_init( void ) kernel_cmdline = (const char *) fw_cfg_read_i32(FW_CFG_KERNEL_CMDLINE); if (kernel_cmdline) { - size = strlen(kernel_cmdline); - memcpy(cmdline, kernel_cmdline, size); + cmdline = strdup(kernel_cmdline); obp_arg.argv[1] = cmdline; } - cmdline[size] = '\0'; qemu_cmdline = (uint32_t)cmdline; /* Setup nvram variables */ -- 1.7.10.4

2 2