Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 25 May 2020 08:33:15 -0700
From:   Matthew Wilcox <willy@infradead.org>
To:     Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Subject: Re: [PATCH] mm: dump_page: add debugfs file for dumping page state
 by pfn
Message-ID: <20200525153315.GC17206@bombadil.infradead.org>
References: <159041635119.987025.7321864888027213705.stgit@buzz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <159041635119.987025.7321864888027213705.stgit@buzz>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, May 25, 2020 at 05:19:11PM +0300, Konstantin Khlebnikov wrote:
> Tool 'page-types' could list pages mapped by process or file cache pages,
> but it shows only limited amount of state exported via procfs.
> 
> Let's employ existing helper dump_page() to reach remaining information:
> writing pfn into /sys/kernel/debug/dump_page dumps state into kernel log.
> 
> # echo 0x37c43c > /sys/kernel/debug/dump_page
> # dmesg | tail -6
>  page:ffffcb0b0df10f00 refcount:1 mapcount:0 mapping:000000007755d3d9 index:0x30
>  0xffffffffae4239e0 name:"libGeoIP.so.1.6.9"
>  flags: 0x200000000020014(uptodate|lru|mappedtodisk)
>  raw: 0200000000020014 ffffcb0b187fd288 ffffcb0b189e6248 ffff9528a04afe10
>  raw: 0000000000000030 0000000000000000 00000001ffffffff 0000000000000000
>  page dumped because: debugfs request

This makes me deeply uncomfortable.  We're using %px, and %lx
(for the 'raw' lines) so we actually get to see kernel addresses.
We've rationalised this in the past as being acceptable because you're
already in an "assert triggered" kind of situation.  Now you're adding
a way for any process with CAP_SYS_ADMIN to get kernel addresses dumped
into the syslog.

I think we need a different function for this, or we need to re-audit
dump_page() for exposing kernel pointers, and not expose the raw data
in struct page.