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 C69CBC4725D for ; Thu, 18 Jan 2024 02:12:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 607FC6B0081; Wed, 17 Jan 2024 21:12:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5B8366B0083; Wed, 17 Jan 2024 21:12:57 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 47FB96B0085; Wed, 17 Jan 2024 21:12:57 -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 3975E6B0081 for ; Wed, 17 Jan 2024 21:12:57 -0500 (EST) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 12F41A0117 for ; Thu, 18 Jan 2024 02:12:57 +0000 (UTC) X-FDA: 81690808794.07.AF08CC8 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf29.hostedemail.com (Postfix) with ESMTP id 77ADE120013 for ; Thu, 18 Jan 2024 02:12:53 +0000 (UTC) Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=qezhMimt; dmarc=none; spf=none (imf29.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=1705543975; 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=5l5efZNY73Qhq1tGWozeEdJ6d0fwUHOUFXbDlZJhdxc=; b=DCssqdP0dxj2PLbQQfcBob68VYdLwQ0W1nRJe4BMmmiFzno6F+YO4TVXn0gGiCC/gBsKOV tiyMr6i+qXq3UV90vXhs3ZGCS8lyEsmfpR8FrrJNzke3bdY2FANuN479p4aykQLgO9VS9x cTQ2CJVnfZRglfuqvSUa/YN56YdMdgs= ARC-Authentication-Results: i=1; imf29.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=qezhMimt; dmarc=none; spf=none (imf29.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=1705543975; a=rsa-sha256; cv=none; b=orAJbY/hdml5DBLIYoXBbX3SZ2vAg60OuABP2FbE4raJn0+TGN380w2GepC5+tdCiiPkvN 1z4WM+K7bcSipqaBwiGDEq/+0jYicHKZSfzmxno38e6uhn84ndlT29hGd2KY1OuWPmT57N cZdVdIDjnedoY0iT5Z5xYGPXr+Gub3M= 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=5l5efZNY73Qhq1tGWozeEdJ6d0fwUHOUFXbDlZJhdxc=; b=qezhMimtZvQP7WzzivmmK56dcC LcTSdSHXVcQALW0kaZxu3T7uptCQcWE5xmul1tCGRFyV6jAxXS5/Hw/azDIY60pi1K7zew8nDRFOG XCV2+DASY32Xkgll+ccpN+OipwjfHc8vcLoarGfreFT0YUqjDqtrw56/mM8QQcgEvCo01/w+ujiYu sfx6Nx/o1+OVUIOWlTdPWjm9/RbXbGlQOA8yzfxhpgaN/NHdBHucYJ6T5CSn9GNhP6Ko4bndYRP6a V4EiE+nxi5U+H4yYHWeMBUFea/gKEYQvjUan2Z7w2l7SmQPSDsWg7Ll56UgZFaHv6g3CgPQwh1ffD NKta2JaA==; Received: from willy by casper.infradead.org with local (Exim 4.97.1 #2 (Red Hat Linux)) id 1rQHtg-00000001EVK-1zNv; Thu, 18 Jan 2024 02:12:48 +0000 Date: Thu, 18 Jan 2024 02:12:48 +0000 From: Matthew Wilcox To: Al Viro Cc: Baolin Wang , akpm@linux-foundation.org, brauner@kernel.org, jack@suse.cz, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] fs: improve dump_mapping() robustness Message-ID: References: <937ab1f87328516821d39be672b6bc18861d9d3e.1705391420.git.baolin.wang@linux.alibaba.com> <20240118013857.GO1674809@ZenIV> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240118013857.GO1674809@ZenIV> X-Rspamd-Queue-Id: 77ADE120013 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: 7atffkrhd6faai8ofsdfrwaumuag9deu X-HE-Tag: 1705543973-702665 X-HE-Meta: U2FsdGVkX190LXsbDzTBJFD4w/YmfFDev4RiRxp+z1DXP9XA93qxrq4CzKp7sXfwfHcOkhFeTGPFYcqx09TNkpRIsmvkQgbbKUqNCkmYB0sgEBa0ICsk8rjXYdZZVxkZGfDDXb5QCDxJgLulXjSaMSXoE0DZtS/RR4ys7jYTA4m5tzpyMVO3GsAiPLlkDvNH7wsye4Xen2haJYTTYuBVsaFtp+0UENnL20fnmEiSUZK3EiR031VLsS5+XqA50c0+EA68OOYhdPc9+vdxyOc6bBpW/iWR7TLfUdGdrFDu/5jSKmYfyA3HshFyOV8bErV/DKsMPjyrJppClr7967oCkRDSZ72YH8FCs8FMFp2xqMFY9M0FRAiDoTycz/urvyTdsEhfaLWkUFhDM6VysTNd5acTVQ3To0WarPs5fkSwrESxOsKScJ9hbxrcS9xBfhTNDYflXMWSt3Vf0Fr431mobNEqUzHagEJmthoBDuzLJbexV2cjRH6LgRsT6Nkt9iMG4uvgbDvv0h/KVbUP1dn7lj8JjImL5HKmiWcaN3OSIZ8eCN/aEoiE6H+Lcie8MY2uuP1io01nfZjGOWDzktLd+PHlEuQEudM3l5bM3KV644f64EnvREvWaXH9/daV9h74itU8OHRcW8Wdi+5A7fW1/L3S96ctYb9/k05hvZbW8ASlSqsjJrbBcVxF5ilLa0HC/LnesbmZbkQh4AfDSR8fA2YCgzndcUd94etO3Ry6KXzhf55hiZkYBa8MVuHPuz4Gxc5KBw13nuclQ/k9pUlr4UfQM5/eYxK8Eon4yhz6FQ1S8KZE6zd/+l7XRvoV1Nsx8TjQoXrGEG8JdbNQkZHOyILpfh2HFIK5lg2AUilx9q8IQnUGBmaZQWESttvETmRRwkZcisqZZ8mIG+ghVSxH341wsBY6SPnmPgxBAJ3HKKRG64+7HmzLOQ2e9yVmoY4Az7PNTHAbcOj2GADIzd+ T5WJYmRG aDAdk7+y26J2YZnhIueYAWxhC40DrW8Pt+TnlXpRLdlCXrmWN+rZCYtDJT/eGp0TRK/TZeFNS6siTgsppSzkKi9emQ6vOBMexuOatQh5hI3KAPYsSc40ZwI81pzME0UIJzdlAVcZl/Z2YmXRU59AgNWuzRQbnmQ1JOSYrSR3pXhm0jemdaphdkIyRGg== 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 Thu, Jan 18, 2024 at 01:38:57AM +0000, Al Viro wrote: > On Tue, Jan 16, 2024 at 03:53:35PM +0800, Baolin Wang wrote: > > > With checking the 'dentry.parent' and 'dentry.d_name.name' used by > > dentry_name(), I can see dump_mapping() will output the invalid dentry > > instead of crashing the system when this issue is reproduced again. > > > dentry_ptr = container_of(dentry_first, struct dentry, d_u.d_alias); > > - if (get_kernel_nofault(dentry, dentry_ptr)) { > > + if (get_kernel_nofault(dentry, dentry_ptr) || > > + !dentry.d_parent || !dentry.d_name.name) { > > pr_warn("aops:%ps ino:%lx invalid dentry:%px\n", > > a_ops, ino, dentry_ptr); > > return; > > That's nowhere near enough. Your ->d_name.name can bloody well be pointing > to an external name that gets freed right under you. Legitimately so. > > Think what happens if dentry has a long name (longer than would fit into > the embedded array) and gets renamed name just after you copy it into > a local variable. Old name will get freed. Yes, freeing is RCU-delayed, > but I don't see anything that would prevent your thread losing CPU > and not getting it back until after the sucker's been freed. Agreed that it's not enough. It does usually work, and it's very helpful when it does. We've had it since 2018 (1c6fb1d89e73) and we've been gradually making it more robust over time. Part of my reason for splitting dump_mapping() out of dump_page() was so that it would get more review from people who understand the fs side of things ... and that seems to have worked. Can I trouble you to suggest a more robust solution? Bear in mind that dump_page() does get called on pointers which turn out not to even be pointers to struct page so this is all very much best-effort, and giving up and printing 'this is not a dentry" is always an option.