diff --git a/SOURCES/v1.0.7_update.patch b/SOURCES/v1.0.7_update.patch new file mode 100644 index 0000000..dd8bcf0 --- /dev/null +++ b/SOURCES/v1.0.7_update.patch @@ -0,0 +1,223 @@ + +ptdump could fail with the following error message: + + ptdump: invalid size request: 0 type: "read page for write" + +This is because there is lack of consideration in function +write_buffer_wrapped() that there is possibility that current write +position in the corresponding ring buffer could be just on +page-aligned offset. Then, read size for the 3rd write operation +becomes 0 bytes and then readmem() accepting the 0 bytes in the 4th +argument results in the error with the above error message. + +More concretely, function write_buffer_wrapped() retrieves and writes +data on the corresponding ring buffer in 3 write operations as the +following picture: + + current write position + (2) (3) | (1) + <-----------> <---> v <-----------> + +-------------+---------+----------+ + | | | | + | | P | | + | | | | + +-------------+---------+----------+ + +The largest square is the corresponding ring buffer containing the +trace data collected by Intel PT. The downward arrow illustrates the +current write position, i.e. the offset of the write operation() at +the timing when system panic occurs and crash dump is collected. The +small square containing the letter 'P' is the page where the current +write position belongs to. ptdump retrieves and writes the data in +this ring buffer in the order of (1), (2) and (3), i.e. from old to +new data. + +Then, note that when the current write position is on the page-aligned +offset, there is no square containing 'P' as: + + current write position + (2) | (1) + <------------>v<-----------> + +-------------++------------+ + | || | + | || | + | || | + +-------------++------------+ + +and then the write size for the write operation (3) becomes 0 bytes, +meaning that the write operation (3) is unnecessary in this case. + + + +--- ptdump-1.0.3/ptdump.c.orig ++++ ptdump-1.0.3/ptdump.c +@@ -72,7 +72,7 @@ get_member(ulong addr, char *name, char + size = MEMBER_SIZE(name, member); + + +- if (!readmem(addr + offset, KVADDR, buf, size, name, FAULT_ON_ERROR)) ++ if (!readmem(addr + offset, KVADDR, buf, size, name, RETURN_ON_ERROR)) + return FALSE; + + return TRUE; +@@ -162,7 +162,7 @@ int init_pt_info(int cpu) + ulong page; + + if (!readmem(pgaddr, KVADDR, &page, sizeof(ulong), +- "struct page", FAULT_ON_ERROR)) ++ "struct page", RETURN_ON_ERROR)) + continue; + + pt_info_ptr->buffer_ptr[i] = page; +@@ -194,7 +194,7 @@ int init_pt_info(int cpu) + /* Read topa entry */ + if (!readmem((topa_base) + topa_idx*(sizeof(struct topa_entry)), + KVADDR, &topa, sizeof(topa), +- "struct topa_entry", FAULT_ON_ERROR)) { ++ "struct topa_entry", RETURN_ON_ERROR)) { + fprintf(fp, "Cannot read topa table\n"); + goto out_error; + } +@@ -230,7 +230,8 @@ int init_pt_info(int cpu) + out_error: + if (pt_info_ptr->buffer_ptr != NULL) + free(pt_info_ptr->buffer_ptr); +- return FALSE; ++ ++ return FALSE; + } + + static inline int is_zero_page(ulong page, int offset) +@@ -247,8 +248,11 @@ static inline int is_zero_page(ulong pag + + memset(buf, 0, PAGESIZE()); + dbgprintf(fp, "zero page chk: 0x%016lx, %lu\n", read_addr, read_size); +- readmem(read_addr, KVADDR, buf, read_size, "zero page check", +- FAULT_ON_ERROR); ++ if (!readmem(read_addr, KVADDR, buf, read_size, "zero page check", ++ RETURN_ON_ERROR)) { ++ free(buf); ++ return FALSE; ++ } + + for (i = 0; i < PAGESIZE() - offset; i++) { + if (buf[i]) { +@@ -312,8 +316,11 @@ int write_buffer_wrapped(int cpu, FILE * + page = pt_info_ptr->buffer_ptr[idx]; + len = PAGESIZE() - offset; + +- readmem(page + offset, KVADDR, buf, len, "read page for write", +- FAULT_ON_ERROR); ++ if (!readmem(page + offset, KVADDR, buf, len, "read page for write", ++ RETURN_ON_ERROR)) { ++ free(buf); ++ return FALSE; ++ } + + dbgprintf(fp, "[%d] R/W1 buff: p=0x%lx, i=%d, o=%lu, l=%d\n", + cpu, page + offset, idx, offset, len); +@@ -332,8 +339,11 @@ int write_buffer_wrapped(int cpu, FILE * + page = pt_info_ptr->buffer_ptr[idx]; + len = PAGESIZE() - offset; + +- readmem(page + offset, KVADDR, buf, len, "read page for write", +- FAULT_ON_ERROR); ++ if (!readmem(page + offset, KVADDR, buf, len, "read page for write", ++ RETURN_ON_ERROR)) { ++ free(buf); ++ return FALSE; ++ } + + dbgprintf(fp, "[%d] R/W2 buff: p=0x%lx, i=%d, o=%lu, l=%d\n", + cpu, page + offset, idx, offset, len); +@@ -351,8 +361,14 @@ int write_buffer_wrapped(int cpu, FILE * + offset = pt_info_ptr->output_off & mask; + len = offset; + +- readmem(page, KVADDR, buf, len, "read page for write", +- FAULT_ON_ERROR); ++ if (!len) ++ goto done; ++ ++ if (!readmem(page, KVADDR, buf, len, "read page for write", ++ RETURN_ON_ERROR)) { ++ free(buf); ++ return FALSE; ++ } + + dbgprintf(fp, "[%d] R/W3 buff: p=0x%lx, i=%d, o=%lu, l=%d\n", cpu, + page, idx, offset, len); +@@ -364,6 +380,7 @@ int write_buffer_wrapped(int cpu, FILE * + return FALSE; + } + ++done: + free(buf); + return TRUE; + } +@@ -388,8 +405,11 @@ int write_buffer_nowrapped(int cpu, FILE + page = pt_info_ptr->buffer_ptr[idx]; + len = PAGESIZE(); + +- readmem(page, KVADDR, buf, len, "read page for write", +- FAULT_ON_ERROR); ++ if (!readmem(page, KVADDR, buf, len, "read page for write", ++ RETURN_ON_ERROR)) { ++ free(buf); ++ return FALSE; ++ } + + dbgprintf(fp, "[%d] R/W1 buff: p=0x%lx, i=%d, o=%lu, l=%d\n", + cpu, page, idx, (ulong)0, len); +@@ -406,8 +426,14 @@ int write_buffer_nowrapped(int cpu, FILE + page = pt_info_ptr->buffer_ptr[idx]; + len = pt_info_ptr->output_off & mask; + +- readmem(page, KVADDR, buf, len, "read page for write", +- FAULT_ON_ERROR); ++ if (!len) ++ goto done; ++ ++ if (!readmem(page, KVADDR, buf, len, "read page for write", ++ RETURN_ON_ERROR)) { ++ free(buf); ++ return FALSE; ++ } + + dbgprintf(fp, "[%d] R/W2 buff: p=0x%lx, i=%d, o=%lu, l=%d\n", cpu, + page, idx, (ulong)0, len); +@@ -419,6 +445,7 @@ int write_buffer_nowrapped(int cpu, FILE + return FALSE; + } + ++done: + free(buf); + return TRUE; + } +@@ -491,6 +518,9 @@ cmd_ptdump(void) + if (argcnt != 2) + cmd_usage(pc->curcmd, SYNOPSIS); + ++ if (ACTIVE()) ++ error(FATAL, "not supported on a live system\n"); ++ + outdir = args[1]; + if ((ret = mkdir(outdir, mode))) { + fprintf(fp, "Cannot create directory %s: %d\n", outdir, ret); +@@ -502,12 +532,12 @@ cmd_ptdump(void) + return; + } + +- /* +- * Set the gdb scope to ensure that the appropriate ring_buffer +- * structure is selected. ++ /* ++ * Set the gdb scope to ensure that the appropriate ring_buffer ++ * structure is selected. + */ + if (kernel_symbol_exists("perf_mmap_to_page")) +- gdb_set_crash_scope(symbol_value("perf_mmap_to_page"), ++ gdb_set_crash_scope(symbol_value("perf_mmap_to_page"), + "perf_mmap_to_page"); + + online_cpus = get_cpus_online(); diff --git a/SPECS/crash-ptdump-command.spec b/SPECS/crash-ptdump-command.spec index a00af5b..6a16bf9 100644 --- a/SPECS/crash-ptdump-command.spec +++ b/SPECS/crash-ptdump-command.spec @@ -4,7 +4,7 @@ Summary: ptdump extension module for the crash utility Name: crash-ptdump-command Version: 1.0.3 -Release: 4%{?dist} +Release: 5%{?dist} License: GPLv2 Group: Development/Debuggers Source: ptdump-%{version}.tar.gz @@ -17,6 +17,7 @@ Requires: crash >= 5.1.5 Patch0: RPM_OPT_FLAGS.patch Patch1: ring_buffer_scope.patch Patch2: rhel8_build.patch +Patch3: v1.0.7_update.patch %description Retrieve and decode the log buffer generated by the Intel(R) Processor @@ -27,6 +28,7 @@ Trace facility %patch0 -p1 -b RPM_OPT_FLAGS.patch %patch1 -p1 -b ring_buffer_scope.patch %patch2 -p1 -b rhel8_build.patch +%patch3 -p1 -b v1.0.7_update.patch %build make -f ptdump.mk @@ -46,6 +48,12 @@ rm -Rf $RPM_BUILD_ROOT %doc COPYING %changelog +* Wed Jan 29 2020 Dave Anderson - 1.0.3-5 +- ptdump: fix build warning: warning: this ‘if’ clause does not guard +- ptdump: fix failure: ptdump: invalid size request: 0 type: "read page for write" +- ptdump: fix heap memory and fd leak when fault happens + Resolves: rhbz#1786497 + * Wed Sep 19 2018 Dave Anderson - 1.0.3-4 - Address annocheck link issue Resolves: rhbz#1630557