linux-kernel - Re: [PATCH V2] fs: perform the check when page without mapping but page->mapping contains junk or random bitscribble

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZDVPkljntjCr9/nX@casper.infradead.org>
Date:   Tue, 11 Apr 2023 13:16:18 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     "xiaosong.ma" <Xiaosong.Ma@...soc.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Alexander Viro <viro@...iv.linux.org.uk>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org,
        Zhaoyang Huang <huangzhaoyang@...il.com>,
        yuming.han@...soc.com, ke.wang@...soc.com
Subject: Re: [PATCH V2] fs: perform the check when page without mapping but
 page->mapping contains junk or random bitscribble

On Mon, Apr 10, 2023 at 09:45:02AM +0800, xiaosong.ma wrote:
> perform the check in dump_mapping() to print warning info and avoid crash with invalid non-NULL page->mapping.
> For example, a panic with following backtraces show dump_page will show wrong info and panic when the bad page
> is non-NULL mapping and page->mapping is 0x80000000000.
> 
>     crash_arm64> bt
>     PID: 232    TASK: ffffff80e8c2c340  CPU: 0   COMMAND: "Binder:232_2"
>      #0 [ffffffc013e5b080] sysdump_panic_event$b2bce43a479f4f7762201bfee02d7889 at ffffffc0108d7c2c
>      #1 [ffffffc013e5b0c0] atomic_notifier_call_chain at ffffffc010300228
>      #2 [ffffffc013e5b2c0] panic at ffffffc0102c926c
>      #3 [ffffffc013e5b370] die at ffffffc010267670
>      #4 [ffffffc013e5b3a0] die_kernel_fault at ffffffc0102808a4
>      #5 [ffffffc013e5b3d0] __do_kernel_fault at ffffffc010280820
>      #6 [ffffffc013e5b410] do_bad_area at ffffffc01028059c
>      #7 [ffffffc013e5b440] do_translation_fault$4df5decbea5d08a63349aa36f07426b2 at ffffffc0111149c8
>      #8 [ffffffc013e5b470] do_mem_abort at ffffffc0100a4488
>      #9 [ffffffc013e5b5e0] el1_ia at ffffffc0100a6c00
>      #10 [ffffffc013e5b5f0] __dump_page at ffffffc0104beecc

This doesn't show a crash in dump_mapping(), it shows a crash in
__dump_page().

> diff --git a/fs/inode.c b/fs/inode.c
> index f453eb5..c9021e5 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -564,7 +564,8 @@ void dump_mapping(const struct address_space *mapping)
>  	 * 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, &mapping) ||
> +	    get_kernel_nofault(host, &mapping->host) ||

This patch makes no sense.  Essentially, you're saying
	mapping = &mapping
which is obviously wrong.