From f15e32cdb1223afa2ef279f90e89d339fcac0a82 Mon Sep 17 00:00:00 2001 From: Alexander Egorenkov Date: Fri, 3 Sep 2021 20:32:05 +0200 Subject: [PATCH] zdump/dfi: Fix illegal memory access in dfi_cpu_add_from_lc() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check that a CPU's lowcore address falls within a valid memory region before accessing it. This prevents potential illegal memory accesses in case a dump contains invalid CPU lowcore addresses. This bug was found with AFL fuzzing and ASAN. Starting program: /root/s390-tools/zdump/zgetdump -iVVVVV /root/zgetdump-fuzzing/findings/crashes/id:000004,sig:06,src:000005,op:flip32,pos:3055 [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib64/libthread_db.so.1". TRACE: DFI initialization DEBUG: DFI trying s390tape DEBUG: DFI s390tape returned with rc -19 DEBUG: DFI trying devmem DEBUG: DFI devmem returned with rc -19 DEBUG: DFI trying s390mv_ext DEBUG: DFI s390mv_ext returned with rc -19 DEBUG: DFI trying s390mv DEBUG: DFI s390mv returned with rc -19 DEBUG: DFI trying s390_ext DEBUG: DFI S390 extended initialization DEBUG: DFI s390_ext returned with rc -19 DEBUG: DFI trying s390 DEBUG: DFI S390 initialization INFO: DFI S390 version 5 DEBUG: DFI S390 mem_size 0x00000000000dbba0 DEBUG: DFI add vol mem chunk start 0x0000000000000000 size 0x00000000000dbba0 volnr 0 [snip] TRACE: DFI virt mem read addr 0x0000000000000000 size 0x0000000000002000 TRACE: DFI virt mem read addr 0x0000000000000000 size 0x0000000000002000 TRACE: DFI virt mem read addr 0x0000000000000000 size 0x0000000000002000 TRACE: DFI virt mem read addr 0x0000000000000000 size 0x0000000000002000 TRACE: DFI virt mem read addr 0x0000000000000000 size 0x0000000000002000 TRACE: DFI virt mem read addr 0x0000000000000000 size 0x0000000000002000 TRACE: DFI virt mem read addr 0x0000000000000000 size 0x0000000000002000 TRACE: DFI virt mem read addr 0x0000000000000000 size 0x0000000000002000 TRACE: DFI virt mem read addr 0x0000000000000000 size 0x0000000000002000 TRACE: DFI virt mem read addr 0x0000000000000000 size 0x0000000000002000 TRACE: DFI virt mem read addr 0x0000000000000000 size 0x0000000000002000 TRACE: DFI virt mem read addr 0x0000000000000000 size 0x0000000000002000 TRACE: DFI virt mem read addr 0x0000000000000000 size 0x0000000000002000 TRACE: DFI virt mem read addr 0x0000000000000000 size 0x0000000000002000 TRACE: DFI virt mem read addr 0x00000000000000ff size 0x0000000000002000 TRACE: DFI virt mem read addr 0x00000000ffffff00 size 0x0000000000002000 Program received signal SIGSEGV, Segmentation fault. 0x000000000100fbb0 in mem_read (cnt=, buf=0x3ffffffc7d0, addr=4294967040, mem=0x104b218 ) at dfi.c:339 339 size = MIN(cnt - copied, mem_chunk->end - addr + 1); Missing separate debuginfos, use: dnf debuginfo-install fuse-libs-2.9.9-11.fc34.s390x glibc-2.33-20.1.ibm.fc34.s390x libasan-11.0.1-0.3.1.ibm.fc34.s390x libgcc-11.0.1-0.3.1.ibm.fc34.s390x libstdc++-11.0.1-0.3.1.ibm.fc34.s390x zlib-1.2.11-26.fc34.s390x (gdb) bt Signed-off-by: Alexander Egorenkov Signed-off-by: Jan Höppner --- zdump/df_s390.c | 12 +++++++++--- zdump/df_s390.h | 2 +- zdump/dfi.c | 9 ++++++--- zdump/dfi.h | 2 +- zdump/dfi_lkcd.c | 17 ++++++++++++----- zdump/dfi_s390.c | 4 +++- zdump/dfi_s390mv.c | 6 +++++- zdump/dfi_s390tape.c | 4 +++- 8 files changed, 40 insertions(+), 16 deletions(-) diff --git a/zdump/df_s390.c b/zdump/df_s390.c index 834ec4d6..4c767486 100644 --- a/zdump/df_s390.c +++ b/zdump/df_s390.c @@ -37,9 +37,10 @@ static int check_addr_max(struct df_s390_hdr *hdr, u64 addr_max) /* * Convert lowcore information into internal CPU representation */ -void df_s390_cpu_info_add(struct df_s390_hdr *hdr, u64 addr_max) +int df_s390_cpu_info_add(struct df_s390_hdr *hdr, u64 addr_max) { unsigned int i; + int rc; if (hdr->version < 5 && hdr->magic == DF_S390_MAGIC) { /* No Prefix registers in header */ @@ -53,8 +54,13 @@ void df_s390_cpu_info_add(struct df_s390_hdr *hdr, u64 addr_max) dfi_cpu_info_init(DFI_CPU_CONTENT_ALL); } - for (i = 0; i < hdr->cpu_cnt; i++) - dfi_cpu_add_from_lc(hdr->lc_vec[i]); + for (i = 0; i < hdr->cpu_cnt; i++) { + rc = dfi_cpu_add_from_lc(hdr->lc_vec[i]); + if (rc) + return rc; + } + + return 0; } /* diff --git a/zdump/df_s390.h b/zdump/df_s390.h index 1cb3707f..1101e379 100644 --- a/zdump/df_s390.h +++ b/zdump/df_s390.h @@ -127,7 +127,7 @@ struct df_s390_dumper { */ extern void df_s390_hdr_add(struct df_s390_hdr *hdr); extern void df_s390_em_add(struct df_s390_em *em); -extern void df_s390_cpu_info_add(struct df_s390_hdr *hdr, u64 addr_max); +extern int df_s390_cpu_info_add(struct df_s390_hdr *hdr, u64 addr_max); extern int df_s390_em_verify(struct df_s390_em *em, struct df_s390_hdr *hdr); extern void df_s390_dumper_read(struct zg_fh *fh, int32_t blk_size, struct df_s390_dumper *dumper); diff --git a/zdump/dfi.c b/zdump/dfi.c index d9cce30d..433a58cd 100644 --- a/zdump/dfi.c +++ b/zdump/dfi.c @@ -935,7 +935,7 @@ static void lc2cpu_32(struct dfi_cpu_32 *cpu, struct dfi_lowcore_32 *lc) * Note: When this function is called, the memory chunks have to be already * defined by the DFI dump specific code. */ -void dfi_cpu_add_from_lc(u32 lc_addr) +int dfi_cpu_add_from_lc(u32 lc_addr) { struct dfi_cpu *cpu = dfi_cpu_alloc(); @@ -948,12 +948,14 @@ void dfi_cpu_add_from_lc(u32 lc_addr) if (l.arch == DFI_ARCH_32) { struct dfi_cpu_32 cpu_32; struct dfi_lowcore_32 lc; - dfi_mem_read(lc_addr, &lc, sizeof(lc)); + if (dfi_mem_read_rc(lc_addr, &lc, sizeof(lc))) + return -EINVAL; lc2cpu_32(&cpu_32, &lc); cpu_32_to_64(cpu, &cpu_32); } else { struct dfi_lowcore_64 lc; - dfi_mem_read(lc_addr, &lc, sizeof(lc)); + if (dfi_mem_read_rc(lc_addr, &lc, sizeof(lc))) + return -EINVAL; lc2cpu_64(cpu, &lc); } break; @@ -961,6 +963,7 @@ void dfi_cpu_add_from_lc(u32 lc_addr) ABORT("dfi_cpu_add_from_lc() called for CONTENT_NONE"); } dfi_cpu_add(cpu); + return 0; } /* diff --git a/zdump/dfi.h b/zdump/dfi.h index 5ccc274b..34a56bae 100644 --- a/zdump/dfi.h +++ b/zdump/dfi.h @@ -142,7 +142,7 @@ extern struct dfi_cpu *dfi_cpu(unsigned int cpu_nr); extern void dfi_cpu_add(struct dfi_cpu *cpu); extern unsigned int dfi_cpu_cnt(void); extern enum dfi_cpu_content dfi_cpu_content(void); -extern void dfi_cpu_add_from_lc(u32 lc_addr); +extern int dfi_cpu_add_from_lc(u32 lc_addr); #define DFI_VX_SA_SIZE (32 * 16) extern int dfi_cpu_lc_has_vx_sa(void *lc); diff --git a/zdump/dfi_lkcd.c b/zdump/dfi_lkcd.c index b0b14c6c..45de1787 100644 --- a/zdump/dfi_lkcd.c +++ b/zdump/dfi_lkcd.c @@ -267,19 +267,25 @@ static int mem_init(void) /* * Initialize CPU information */ -static void cpu_init(void) +static int cpu_init(void) { unsigned int i; + int rc; if (l.hdr_asm.magic != DF_LKCD_MAGIC_ASM) { /* Old LKCD dump without asm header */ dfi_cpu_info_init(DFI_CPU_CONTENT_NONE); - return; + return 0; } dfi_cpu_info_init(DFI_CPU_CONTENT_ALL); - for (i = 0; i < l.hdr_asm.cpu_cnt; i++) - dfi_cpu_add_from_lc(l.hdr_asm.lc_vec[i]); + for (i = 0; i < l.hdr_asm.cpu_cnt; i++) { + rc = dfi_cpu_add_from_lc(l.hdr_asm.lc_vec[i]); + if (rc) + return rc; + } + + return 0; } /* @@ -321,7 +327,8 @@ static int dfi_lkcd_init(void) return -ENODEV; if (mem_init() != 0) return -EINVAL; - cpu_init(); + if (cpu_init() != 0) + return -EINVAL; return 0; } diff --git a/zdump/dfi_s390.c b/zdump/dfi_s390.c index 73fabeab..c6e8e3b7 100644 --- a/zdump/dfi_s390.c +++ b/zdump/dfi_s390.c @@ -179,7 +179,9 @@ int dfi_s390_init_gen(bool extended) rc = mem_chunks_add_ext(); if (rc) return rc; - df_s390_cpu_info_add(&l.hdr, l.hdr.mem_size); + rc = df_s390_cpu_info_add(&l.hdr, l.hdr.mem_size); + if (rc) + return rc; zg_seek(g.fh, sizeof(l.hdr), ZG_CHECK); return 0; } diff --git a/zdump/dfi_s390mv.c b/zdump/dfi_s390mv.c index fe7a22c1..d936fd1e 100644 --- a/zdump/dfi_s390mv.c +++ b/zdump/dfi_s390mv.c @@ -574,6 +574,8 @@ static void set_magic_numbers(void) */ int dfi_s390mv_init_gen(bool extended) { + int rc; + l.extended = extended; set_magic_numbers(); if (open_dump() != 0) @@ -590,7 +592,9 @@ int dfi_s390mv_init_gen(bool extended) return -EINVAL; if (l.dump_incomplete) return -EINVAL; - df_s390_cpu_info_add(&l.hdr, l.hdr.mem_end); + rc = df_s390_cpu_info_add(&l.hdr, l.hdr.mem_end); + if (rc) + return rc; df_s390_em_add(&l.em); return 0; } diff --git a/zdump/dfi_s390tape.c b/zdump/dfi_s390tape.c index 96a1c026..c1a4c060 100644 --- a/zdump/dfi_s390tape.c +++ b/zdump/dfi_s390tape.c @@ -166,7 +166,9 @@ static int vol_init(void) /* Init memory read & CPU info */ mem_read_init(); - df_s390_cpu_info_add(&hdr, hdr.mem_size - 1); + rc = df_s390_cpu_info_add(&hdr, hdr.mem_size - 1); + if (rc) + return rc; return 0; }