Thank you for the patch, HATAYAMA.

On Thu, Sep 7, 2023 at 6:55 PM <crash-utility-request@redhat.com> wrote:
Date: Thu,  7 Sep 2023 18:38:25 +0900
From: HATAYAMA Daisuke <d.hatayama@fujitsu.com>
To: crash-utility@redhat.com
Subject: [Crash-utility] [PATCH] diskdump: fix read_diskdump() returns
        successfully for illegal 0-size page descriptors
Message-ID: <20230907093825.515-1-d.hatayama@fujitsu.com>
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@fujitsu.com>
---
 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) {

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;
                }

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