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 1BCD1C2BB3F for ; Wed, 15 Nov 2023 19:04:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 91948280003; Wed, 15 Nov 2023 14:04:21 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8A231280001; Wed, 15 Nov 2023 14:04:21 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 74371280003; Wed, 15 Nov 2023 14:04:21 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 5D83C280001 for ; Wed, 15 Nov 2023 14:04:21 -0500 (EST) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 2A82C1407D7 for ; Wed, 15 Nov 2023 19:04:21 +0000 (UTC) X-FDA: 81461114322.29.A26FBCA Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf08.hostedemail.com (Postfix) with ESMTP id B69D416002A for ; Wed, 15 Nov 2023 19:04:18 +0000 (UTC) Authentication-Results: imf08.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b="coxdz/uM"; dmarc=none; spf=none (imf08.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1700075059; a=rsa-sha256; cv=none; b=fKNcTEgFtzyyLNxlJyw8kO3bsR8giNTjyalHTj8avUIQOfdXotLnF2mMUP0ymksvUj3YS9 TMWAvFbS8CL72DM3xDpueMMo1Ac8MwIuLRRenrr1NgEucOVBVR9YJdM50FVV9+8LyEc3Fl nYEZC8zY8qle6euZoftANwtN1emZ8SM= ARC-Authentication-Results: i=1; imf08.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b="coxdz/uM"; dmarc=none; spf=none (imf08.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1700075059; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=xR5jXjalkv+3xl2hvJEA3on0YsUVjM36wGHO7zEA1+8=; b=AVpaURhHRuRuG+MCTiHUmbjzZ1UHXLu1KIVbeu84gPdDijkBdTil9JIwb1lgmAap57cXuw 3+iHMpMKcdaW210bKJBSbfbRfGDjIo5NSy8MWlEcMaCXAJ1TAsN7EqNxxNd6Z9Yg4h8Kzd J4vq5Y/9P2uVNmELh1WK/Xp3mcMrZxk= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Transfer-Encoding: Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Sender:Reply-To:Content-ID:Content-Description; bh=xR5jXjalkv+3xl2hvJEA3on0YsUVjM36wGHO7zEA1+8=; b=coxdz/uMBpP2qrs6VvFvIrmOjB Rp0v2QygwbJhYQWMx0TaTg9q/pykgcpXK/n9As9QDhViNROUy2oaxm5e2SDDcN0hWeO+BhrO+AbIW owXkpwcxPFF5XAlpvXCZo4319GlmY18ZbJY//s2Kz9Z3oZ4P7XzZmmjCj6SSllxrEhgYrtAYVRVyC aaFDnxEp4rD+wb8o/nwrxGlsgSI6gmX37eumZP8QksJcILTlcrpDHTGCqNTY/VKwWRTig2V2TY0xG fK/xHNr7gDi6CwtjqwstXmKfQDuCddOg/1qcOM9pANa2sFaHWFEFSLnqnMHpcrYM1orcpXNlWoI/r GgEB84fQ==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1r3LBO-00Fruc-5G; Wed, 15 Nov 2023 19:04:14 +0000 Date: Wed, 15 Nov 2023 19:04:14 +0000 From: Matthew Wilcox To: =?iso-8859-1?Q?Jos=E9?= Pekkarinen Cc: akpm@linux-foundation.org, skhan@linuxfoundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-kernel-mentees@lists.linux.dev, syzbot+89edd67979b52675ddec@syzkaller.appspotmail.com, Hugh Dickins Subject: Re: [PATCH] mm/pgtable: return null if no ptl in __pte_offset_map_lock Message-ID: References: <20231115065506.19780-1-jose.pekkarinen@foxhound.fi> <1c4cb1959829ecf4f0c59691d833618c@foxhound.fi> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1c4cb1959829ecf4f0c59691d833618c@foxhound.fi> X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: B69D416002A X-Stat-Signature: h3r4cf11t59hnwp45qdtk3xjka9aztnj X-HE-Tag: 1700075058-770330 X-HE-Meta: U2FsdGVkX1/HLAxYQxPh9U9TgzctDCLlDzAC1ou9uNQ+hI6sKnFOSiSviwKrGhvAZ/5qD5mQyg0h2pxXSzNUah8Nc6Mft2lcApLYTS9E0BhCA/FkL/MdugsHKahN9BdwDLwhe753GotL65kg6yQ0E/cocK0IpqLQ9Ysjr5it+bG9EUq34UTHWZKip3Svkhu+HrK1V9QRwtoUQNj0q5ICrLG/kh3ajw8XXb2tIn1lDxrtbaGO1I3V0p5ukmnNlMzLzP75lcB2PgP6T2WaNuFOI1hROzLY7ROkKwot8pJ24TZybiDAFU1YZ5dgHsKq3ckqaTXU9GKIoTaLPjY6Do6qLqkI7wv8AMYAZCa/9Oa7iwP2O6DxzdfK9VGxeA5pv1wMg1s9i+MfxXMeILdquHfnPkj2mbuSQjrz1tpdL2UeBrvQP/lqKWjUSXp3dOH/9zRJYSMyTGH+5puLt/HwFLnxzq2xalOksX1NvnQo431/37QxcOKtNivAVcvH5Cc23w7ibDOVlRT7W/TkpN9VU17DSgKpcLadn4/wW47urhh3IsSyKBjkBa0UnBIuQrQJ3VefqUQy7ZImd13mXoDEyGoLdGRAmUyGyNJZJZ3zO1ULZ7pxsFmKZPmF5Cp+PxsFNatJFXXlSupYm5PaT5eAZfk4m+M+SEi3NMMdlSoUVdOjO0d3YJhdt/JS3qMFTyYJkbJA4yQA+7JGukjCYk9R0LBnQGBy4f2AAKvtkP85uF0eV4SY0d2Z6pu91Sjhpsi8OzDoO4bZqjpfjJlrM64yDdVqnfw/DUPExaiwEd587zeTEVmich54P9DTOCjPGWPaEeNNsZrS6R19frgaDceQe1jemNfaGwmGx1wpm22y5nz+IA0goAM8nAzrVdJuu8gdLiJ3IKDa4lRItp/Chck2V9WRY5F6BsfQ8hHJKDx+KDwYoI1It5KRN+SWK/ZRtm0iA9yyDs0TU6z8pb4YiRgAzN9 ZD5RyBjO mo3pO/LCVF0j2S7WgA5JSPzyhOXK86Wv60iOM8epsh+Aztkhkk73f9j7lFrJsSoliK6ZybAcWyrng2n4UQe2jSu6nDxCNsiyeJiUOBDEz3XRuaPXjhIpfMFZBkM+W5d8a4C28bhOxp4J5g9tCiHpUM/0rMlYSQqPoq4sliB/DLTQY3bdAAfcpzoLrgDERBk7CsViZJKjdK+tIOS7EuHrTYDWloKBcy7Ii/d/ftULhPx0banXLGeTdruYEiYkgiLeBQusBbKy8Rs6STfaCLhwDQPY4nl+luCYdCrNy4JdRHvT6NUS8+KUaQPFZNc08p0IdExfsEIfKWuWDQ9gBGtK8bg6RSEHdDuRQYIdKErCQkS6zSeJbciuxAMjxgmstknpFVYE21eyqA3ZEucrtMxZeY+16uXFXKLICWyfeJGbfuGi4GtFPCi+JQc/oPXrG5GfAsDxi8OWDkZMiPuwm6S9ypwMG8dFrCnmD6yNX6lHRft8IlRFu+EBJYn3MnOIo9A5CnyNh 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: List-Subscribe: List-Unsubscribe: On Wed, Nov 15, 2023 at 06:05:30PM +0200, José Pekkarinen wrote: > On 2023-11-15 16:19, Matthew Wilcox wrote: > > On Wed, Nov 15, 2023 at 08:55:05AM +0200, José Pekkarinen wrote: > > > Documentation of __pte_offset_map_lock suggest there is situations > > > where > > > > You should have cc'd Hugh who changed all this code recently. > > Hi, > > Sorry, he seems to be missing if I run get_maintainer.pl: > > $ ./scripts/get_maintainer.pl include/linux/mm.h > Andrew Morton (maintainer:MEMORY MANAGEMENT) > linux-mm@kvack.org (open list:MEMORY MANAGEMENT) > linux-kernel@vger.kernel.org (open list) That's a good example of why get_maintainer.pl is not great. It's just a stupid perl program. Ideally, you should research what changes have been made to that code recently and see who else might be implicated. Who introduced the exact code that you're fixing? In this specific instance, you can see Hugh already responded to it: https://lore.kernel.org/all/0000000000005e44550608a0806c@google.com/T/ Now, part of Hugh's response turns out to be incorrect; syzbot can reproduce this on a current mainline kernel. But, for some reason, syzbot has not done a bisect to track it down to a particular commit. I don't understand why it hasn't; maybe someone who knows syzbot better can explain why. > > > +++ b/include/linux/mm.h > > > @@ -2854,7 +2854,7 @@ void ptlock_free(struct ptdesc *ptdesc); > > > > > > static inline spinlock_t *ptlock_ptr(struct ptdesc *ptdesc) > > > { > > > - return ptdesc->ptl; > > > + return (likely(ptdesc)) ? ptdesc->ptl : NULL; > > > } > > > > I don't think we should be changing ptlock_ptr(). > > This is where the null ptr dereference originates, so the only > alternative I can think of is to protect the life cycle of the ptdesc > to prevent it to die between the pte check and the spin_unlock of > __pte_offset_map_lock. Would that work for you? Ah! I think I found the problem. If ALLOC_SPLIT_PTLOCKS is not set, there is no problem as ->ptl is embedded in the struct page. But if ALLOC_SPLIT_PTLOCKS is set (eg you have LOCKDEP enabled), we can _return_ a NULL pointer from ptlock_ptr. The NULL pointer dereference isn't in ptlock_ptr(), it's in __pte_offset_map_lock(). So, how to solve this? We can't just check the ptl against NULL; the memory that ptl points to may have been freed. We could grab a reference to the pmd_page, possibly conditionally on ALLOC_SPLIT_LOCK being set, but that will slow down everything. We could make page_ptl_cachep SLAB_TYPESAFE_BY_RCU, preventing the memory from being freed (even if the lock might not be associated with this page any more). Other ideas?