From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id AE075C77B72 for ; Wed, 12 Apr 2023 15:14:41 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 17753900002; Wed, 12 Apr 2023 11:14:41 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 126C36B0075; Wed, 12 Apr 2023 11:14:41 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F30A7900002; Wed, 12 Apr 2023 11:14:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id E2E766B0074 for ; Wed, 12 Apr 2023 11:14:40 -0400 (EDT) Received: from smtpin26.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id BB5698013E for ; Wed, 12 Apr 2023 15:14:40 +0000 (UTC) X-FDA: 80673085920.26.520EEF9 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf12.hostedemail.com (Postfix) with ESMTP id 60A7940004 for ; Wed, 12 Apr 2023 15:14:37 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=XOLP+lIH; spf=none (imf12.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1681312478; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=l0xucI8HSm2rAbmozA4KK4Oskg2uJhGgLoTrZxLS5go=; b=rTELuhfsUJVJSxVGHbXwjJLWoCOx/VvRg6vZEDBbn2bbv3rCJnq1qLldz4l6PPhtZMnC0M ToKrw9W6lWjODSyGr6WifMYthWehobhVjNP04aiW19F0jXmoLG1jFQzvUfAl4sSBdvp1vk XBwzwMbotzqmZu09ZxMi95IdLu3G/2k= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=XOLP+lIH; spf=none (imf12.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1681312478; a=rsa-sha256; cv=none; b=LJjfCxfpy+USHUidjJw+IhPaVp9D8DeEQX79cDRRaX4Pdd4hJwJNeDeqHCOCSLJz7eXkNf 0cUQswtkvrnWDcgmLB5fNbPbzwkCCL6XhmE8q/WAE530MeOETOl8lJVR+COmeFYsw3MbYg jppihD/VLgz+KGiXxQYPb/6yAliH/mg= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=l0xucI8HSm2rAbmozA4KK4Oskg2uJhGgLoTrZxLS5go=; b=XOLP+lIHcnCQmM6hCFjaSb6rTI 7NYFAE1AyPDNlE0Ee/qPRKbHYFRwF7C41+PEjeAFtPFp4myX6Zb3gMwUZbRuNbZk3a0s8K6TW6olD NSS1hZUc0QIUAovWgGSsRWyOoiMEXnJ5tV5chsYrf454dY2hi6RW8smhKqGbj7b9yhRNdhEjov/7p v3PROueny297lJWTN6MXdDS6dfplewG3ow841KA29sRavbVGHShYjqymehKJrud+DsmgBzaU5SlSD 5pf+fmw9mL6OWowX3P38uoT5lIkq8FXVlhFz2p+NmNVR735zdPf/0NTm8HmFGHkLiYLCbOkeqKhsh 6+1KGDgQ==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1pmcAz-006yX9-3p; Wed, 12 Apr 2023 15:14:25 +0000 Date: Wed, 12 Apr 2023 16:14:25 +0100 From: Matthew Wilcox To: Andrew Morton Cc: "xiaosong.ma" , Alexander Viro , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Zhaoyang Huang , yuming.han@unisoc.com, ke.wang@unisoc.com Subject: Re: [PATCH V2] fs: perform the check when page without mapping but page->mapping contains junk or random bitscribble Message-ID: References: <1681091102-31907-1-git-send-email-Xiaosong.Ma@unisoc.com> <20230411171536.2e53b4b7507304d5618aa24e@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230411171536.2e53b4b7507304d5618aa24e@linux-foundation.org> X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 60A7940004 X-Rspam-User: X-Stat-Signature: 6q8erjzkc6cqtkzafy17apo45xycdxcq X-HE-Tag: 1681312477-215236 X-HE-Meta: U2FsdGVkX19L4lgNlyl+AmDfUm7bNfMBkt9zXz4n45TzCNv9Z75rGlKJaUdNPWyzcvWv/n2AnyhG2y76Ndczmf/s3251aKxDxfoX5pAAj7ZAeEGGge5pAG17/zYSUKJ4Thgc/vYqYqvU2S4itpbD+xMReDgIsLezY91p/PbtSczZlDJie1ELymEjI7EY4dj8jMi4ylavyUbIvP5fFXMBy8amUGcqctnNaVmJKLEk8lI6kyOf3iPMrvczsKFa+/0FL+rhWfNAZw1L2mUmmI638YaN0FNvfgKr1D+bX1PTfb3o2NNLke2BWu5H7FypUVVKGCGgA0IGW5spHAyB2KCLlPznc89EemP25+poWCqLc6Tp022FAeb7XDcbPqSshHLXmsWXU4YSBG6Aq6phim8EQuAtpZmiSwatGV1ypY9ic8aVUZpaJhGdMKoMVIfWLFABgMFeUOxwdGdkR4xo1nI1dpGs2/WdbYQIpWu/6qyhxF+AgleP2S1H0+qmo2WhNaPTKOKxdfyhjKeutYomhH9vsR6060gbkRDMJ6x+Ug6q4hgRTLFv7Db+ZOCMJ5vLMoQFYSw51prj1SMXg6y2ygR0agAhmPU3C2t1TOvRiK4+v2yyOwq8cReXnNCi3yMNc2Gn0z9p1umVNWwRNELajHybdu5AfbMNg3jCQHINK3DEQUnWt75Fe7MsQ42e83QC1wRpT2GGilPpCgJtZR9CusbuSmF0JAyL5NO5SJPAufgMXTPTxo2ufVjbGkPS0AJfIpu6DvqEwpR7L7ZE7netbAuYbTeCC3CwWfP0WzW+jHuypPKhKZamMK3u4d444CQV2et9ctG3gQLoXi8TWyqKQSj2zltqCX5Nx/YuTgimHpGIqRSdP1ElXN+IKUf/2CNlPFypehUakKN9uQuGhZNI+RdVCj6sHsKee9Uu/b7cvlnvTG+sX8f8xpn8/ZpGTyFfuXlwMtwdMIXVOv5tzpN0Z4x WDsW0OXI DC5g9hb9yfjLTW6SHJ9nXKS3wkMJMVU1o19JSLraGKwU+bWjNBYTs47wc2YVEax14pRXRwfnwAwCJ2O0yC33jvremLsNIAl9Jtch4iRu7nJNO3dsJxZP+tDYTsMZ7aXcW21JPnF6DBMUd0u2spysofCMVX79fP5J3kTlN11zUcfrXZQEh5Zk5BLOS96r4xemsU+qyp0SbccegDSmXzvSPV8bRzSS3WOatDss7AAYm18KR5fVUj2JD3a3/GNVxevKKFLauGp+XhLg0T9+xvd6ATcevrWWJ0HAkAz8rNeZcf7W1qESW0UNi3b+jLcESVHzo7n5zSXjxKH2XSQk= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Apr 11, 2023 at 05:15:36PM -0700, Andrew Morton wrote: > On Tue, 11 Apr 2023 13:16:18 +0100 Matthew Wilcox wrote: > > > 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(). > > um, yes. > > But if page->mapping is corrupted, where does __dump_page() dereference it? I don't see anywhere that it does, so I'm suspicious that we have the correct diagnosis here. > The initial patch > (https://lkml.kernel.org/r/1680587425-4683-1-git-send-email-Xiaosong.Ma@unisoc.com) > prevented __dump_page() from calling dump_mapping() if page->mapping is > bad, and that presumably fixed things. Right, but doesn't the _existing_ get_kernel_nofault(host, &mapping->host) already prevent us from blindly dereferencing a bad mapping pointer? > > > - 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. > > We're checking for mapping==junk, so this could be > > get_kernel_nofault(tmp, mapping) Why will that be better than get_kernel_nofault(host, &mapping->host)? I see no tangible difference between get_kernel_nofault(0x8000'0000) and get_kernel_nofault(0x8000'0084) (or whatever the offset is). > or go direct to copy_from_kernel_nofault(). We used to have a > probe_kernel_address() for this... > > So confusion reigns. I think making dump_mapping() tolerant of a wild > mapping pointer makes sense, but I don't think we actually know why the > reporter's kernel crashed. In my mind dump_mapping() is already tolerant of a wild page->mapping pointer. I think the problem is something entirely different.