Thank you for the patch, HATAYAMA.
On Thu, Sep 7, 2023 at 6:55 PM <crash-utility-request@xxxxxxxxxx> wrote:
Date: Thu, 7 Sep 2023 18:38:25 +0900
From: HATAYAMA Daisuke <d.hatayama@xxxxxxxxxxx>
To: crash-utility@xxxxxxxxxx
Subject: [PATCH] diskdump: fix read_diskdump() returns
successfully for illegal 0-size page descriptors
Message-ID: <20230907093825.515-1-d.hatayama@xxxxxxxxxxx>
Content-Type: text/plain; charset="US-ASCII"; x-default=true
read_diskdump() returns successfully for illegal 0-size page
descriptors. Page descriptors are illegal if their size member holds 0
because makedumpfile never puts 0 there because any data never result
in 0 byte by compression. If page descriptors hold 0 in size member,
it means the crash dump file is corrupted for some reason.
The root cause of this is that sanity check of function cache_page()
doesn't focus on such 0-size page descriptors. Then, the 0-size page
descriptor is passed to pread(), pread() immediately returns 0
successfully because read data is 0 byte, and then read_diskdump()
returns successfully.
To fix this issue, let the sanity check take into account such 0-size
page descriptors and read_diskdump() result in READ_ERROR.
Signed-off-by: HATAYAMA Daisuke <d.hatayama@xxxxxxxxxxx>
---
diskdump.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/diskdump.c b/diskdump.c
index 2c284ff..2be7cc7 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -1210,7 +1210,7 @@ cache_page(physaddr_t paddr)
return ret;
/* sanity check */
- if (pd.size > block_size)
+ if (pd.size > block_size || !pd.size)
Actually this may not be a real read error instead of an incomplete page due to corruption or other reasons. Given that, I would suggest adding the sanity check as below:
- } else if (0 == pd.offset) {
+ } else if (0 == pd.offset || !pd.size) {
+ } else if (0 == pd.offset || !pd.size) {
It can help to print more information according to the code when fails on the page:
/*
* First check whether zero_excluded has been set.
*/
if (*diskdump_flags & ZERO_EXCLUDED) {
if (CRASHDEBUG(8))
fprintf(fp,
"read_diskdump/cache_page: zero-fill: "
"paddr/pfn: %llx/%lx\n",
(ulonglong)paddr, pfn);
memset(dd->compressed_page, 0, dd->block_size);
} else {
if (CRASHDEBUG(8))
fprintf(fp,
"read_diskdump/cache_page: "
"descriptor with zero offset found at "
"paddr/pfn/pos: %llx/%lx/%lx\n",
(ulonglong)paddr, pfn, desc_pos);
return PAGE_INCOMPLETE;
}
* First check whether zero_excluded has been set.
*/
if (*diskdump_flags & ZERO_EXCLUDED) {
if (CRASHDEBUG(8))
fprintf(fp,
"read_diskdump/cache_page: zero-fill: "
"paddr/pfn: %llx/%lx\n",
(ulonglong)paddr, pfn);
memset(dd->compressed_page, 0, dd->block_size);
} else {
if (CRASHDEBUG(8))
fprintf(fp,
"read_diskdump/cache_page: "
"descriptor with zero offset found at "
"paddr/pfn/pos: %llx/%lx/%lx\n",
(ulonglong)paddr, pfn, desc_pos);
return PAGE_INCOMPLETE;
}
I don't have such vmcores, and can not test it, just an idea.
Thanks.
Lianbo
return READ_ERROR;
/* read page data */
--
2.25.1
-- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility Contribution Guidelines: https://github.com/crash-utility/crash/wiki
