Re: [PATCH] mm: check mapping addr is correct when dump page

[Andrew Morton]

On Tue, 4 Apr 2023 13:50:25 +0800 "xiaosong.ma" <Xiaosong.Ma@xxxxxxxxxx> wrote:

> when we debug with slub_debug_on, the following backtraces show dump_page
> will show wrong info when the bad page is non-NULL mapping and page->mapping
> is 0x80000000000 so do virt_addr valid check is needed when dump mapping page.

How did this page get ->mapping=0x80000000?  I don't recall anywhere
where we deliberately set this state.

Maybe a random bitscribble?  I guess being defensive in __dump_page()
is sensible - we have reason to believe that the page is in some bad
state.

> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -109,7 +109,7 @@ static void __dump_page(struct page *page)
>                 type = "ksm ";
>         else if (PageAnon(page))
>                 type = "anon ";
> -       else if (mapping)
> +       else if (mapping && virt_addr_valid(mapping))
>                 dump_mapping(mapping);

I expect the user will be interested in knowing that ->mapping contains
junk, so perhaps we should print some information telling them this.

In which case, dump_mapping() would be a better place to perform the
check.

And lo, dump_mapping() already did this, so I think all we need is

--- a/fs/inode.c~a
+++ a/fs/inode.c
@@ -565,7 +565,8 @@ void dump_mapping(const struct address_s
 	 * If mapping is an invalid pointer, we don't want to crash
 	 * accessing it, so probe everything depending on it carefully.
 	 */
-	if (get_kernel_nofault(host, &mapping->host) ||
+	if (get_kernel_nofault(mapping) ||
+	    get_kernel_nofault(host, &mapping->host) ||
 	    get_kernel_nofault(a_ops, &mapping->a_ops)) {
 		pr_warn("invalid mapping:%px\n", mapping);
 		return;
_