From patchwork Mon Nov 15 02:24:22 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Ian Kent X-Patchwork-Id: 12618569 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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CF72FC433EF for ; Mon, 15 Nov 2021 02:24:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A67D261156 for ; Mon, 15 Nov 2021 02:24:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229742AbhKOC1s (ORCPT ); Sun, 14 Nov 2021 21:27:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33460 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229590AbhKOC1f (ORCPT ); Sun, 14 Nov 2021 21:27:35 -0500 Received: from smtp02.aussiebb.com.au (smtp02.aussiebb.com.au [IPv6:2403:5800:3:25::1002]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id EBFFEC061766; Sun, 14 Nov 2021 18:24:31 -0800 (PST) Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp02.aussiebb.com.au (Postfix) with ESMTP id 605BF102D6F; Mon, 15 Nov 2021 13:24:26 +1100 (AEDT) X-Virus-Scanned: Debian amavisd-new at smtp02.aussiebb.com.au Received: from smtp02.aussiebb.com.au ([127.0.0.1]) by localhost (smtp02.aussiebb.com.au [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id tkVJymr6ixVb; Mon, 15 Nov 2021 13:24:26 +1100 (AEDT) Received: by smtp02.aussiebb.com.au (Postfix, from userid 116) id 4D31F102D81; Mon, 15 Nov 2021 13:24:25 +1100 (AEDT) Received: from mickey.themaw.net (unknown [100.72.131.210]) by smtp02.aussiebb.com.au (Postfix) with ESMTP id 69FE7102D6F; Mon, 15 Nov 2021 13:24:22 +1100 (AEDT) Subject: [PATCH 2 1/2] vfs: check dentry is still valid in get_link() From: Ian Kent To: xfs , "Darrick J. Wong" , Christoph Hellwig Cc: Miklos Szeredi , Brian Foster , Al Viro , David Howells , linux-fsdevel , Kernel Mailing List Date: Mon, 15 Nov 2021 10:24:22 +0800 Message-ID: <163694306216.229789.2523433978081516384.stgit@mickey.themaw.net> In-Reply-To: <163694289979.229789.1176392639284347792.stgit@mickey.themaw.net> References: <163694289979.229789.1176392639284347792.stgit@mickey.themaw.net> User-Agent: StGit/0.23 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org When following a trailing symlink in rcu-walk mode it's possible for the dentry to become invalid between the last dentry seq lock check and getting the link (eg. an unlink) leading to a backtrace similar to this: crash> bt PID: 10964 TASK: ffff951c8aa92f80 CPU: 3 COMMAND: "TaniumCX" … #7 [ffffae44d0a6fbe0] page_fault at ffffffff8d6010fe [exception RIP: unknown or invalid address] RIP: 0000000000000000 RSP: ffffae44d0a6fc90 RFLAGS: 00010246 RAX: ffffffff8da3cc80 RBX: ffffae44d0a6fd30 RCX: 0000000000000000 RDX: ffffae44d0a6fd98 RSI: ffff951aa9af3008 RDI: 0000000000000000 RBP: 0000000000000000 R8: ffffae44d0a6fb94 R9: 0000000000000000 R10: ffff951c95d8c318 R11: 0000000000080000 R12: ffffae44d0a6fd98 R13: ffff951aa9af3008 R14: ffff951c8c9eb840 R15: 0000000000000000 ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018 #8 [ffffae44d0a6fc90] trailing_symlink at ffffffff8cf24e61 #9 [ffffae44d0a6fcc8] path_lookupat at ffffffff8cf261d1 #10 [ffffae44d0a6fd28] filename_lookup at ffffffff8cf2a700 #11 [ffffae44d0a6fe40] vfs_statx at ffffffff8cf1dbc4 #12 [ffffae44d0a6fe98] __do_sys_newstat at ffffffff8cf1e1f9 #13 [ffffae44d0a6ff38] do_syscall_64 at ffffffff8cc0420b Most of the time this is not a problem because the inode is unchanged while the rcu read lock is held. But xfs can re-use inodes which can result in the inode ->get_link() method becoming invalid (or NULL). This case needs to be checked for in fs/namei.c:get_link() and if detected the walk re-started. Signed-off-by: Ian Kent Reviewed-by: Brian Foster --- 0 files changed diff --git a/fs/namei.c b/fs/namei.c index 1946d9667790..9a48a6106516 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -1760,8 +1760,11 @@ static const char *pick_link(struct nameidata *nd, struct path *link, if (!res) { const char * (*get)(struct dentry *, struct inode *, struct delayed_call *); - get = inode->i_op->get_link; + get = READ_ONCE(inode->i_op->get_link); if (nd->flags & LOOKUP_RCU) { + /* Does the inode still match the associated dentry? */ + if (unlikely(read_seqcount_retry(&link->dentry->d_seq, last->seq))) + return ERR_PTR(-ECHILD); res = get(NULL, inode, &last->done); if (res == ERR_PTR(-ECHILD) && try_to_unlazy(nd)) res = get(link->dentry, inode, &last->done); From patchwork Mon Nov 15 02:24:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Kent X-Patchwork-Id: 12618573 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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 72003C433F5 for ; Mon, 15 Nov 2021 02:25:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 55DA260D42 for ; Mon, 15 Nov 2021 02:25:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230395AbhKOC1w (ORCPT ); Sun, 14 Nov 2021 21:27:52 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33512 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229686AbhKOC1q (ORCPT ); Sun, 14 Nov 2021 21:27:46 -0500 Received: from smtp02.aussiebb.com.au (smtp02.aussiebb.com.au [IPv6:2403:5800:3:25::1002]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A4C63C061746; Sun, 14 Nov 2021 18:24:46 -0800 (PST) Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp02.aussiebb.com.au (Postfix) with ESMTP id D3C1D102D82; Mon, 15 Nov 2021 13:24:29 +1100 (AEDT) X-Virus-Scanned: Debian amavisd-new at smtp02.aussiebb.com.au Received: from smtp02.aussiebb.com.au ([127.0.0.1]) by localhost (smtp02.aussiebb.com.au [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id UqFNLGMF9cvO; Mon, 15 Nov 2021 13:24:29 +1100 (AEDT) Received: by smtp02.aussiebb.com.au (Postfix, from userid 116) id B36D3102D70; Mon, 15 Nov 2021 13:24:29 +1100 (AEDT) Received: from mickey.themaw.net (unknown [100.72.131.210]) by smtp02.aussiebb.com.au (Postfix) with ESMTP id 3E3B6102D70; Mon, 15 Nov 2021 13:24:28 +1100 (AEDT) Subject: [PATCH 2 2/2] xfs: make sure link path does not go away at access From: Ian Kent To: xfs , "Darrick J. Wong" , Christoph Hellwig Cc: Miklos Szeredi , Brian Foster , Al Viro , David Howells , linux-fsdevel , Kernel Mailing List Date: Mon, 15 Nov 2021 10:24:28 +0800 Message-ID: <163694306800.229789.11812765289669370510.stgit@mickey.themaw.net> In-Reply-To: <163694289979.229789.1176392639284347792.stgit@mickey.themaw.net> References: <163694289979.229789.1176392639284347792.stgit@mickey.themaw.net> User-Agent: StGit/0.23 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org When following an inline symlink in rcu-walk mode it's possible to succeed in getting the ->get_link() method pointer but the link path string be deallocated while it's being used. This is becuase of the xfs inode reclaim mechanism. While rcu freeing the link path can prevent it from being freed during use the inode reclaim could assign a new value to the field at any time outside of the path walk and result in an invalid link path pointer being returned. Admittedly a very small race window but possible. The best way to mitigate this risk is to return -ECHILD to the VFS if the inline symlink method, ->get_link(), is called in rcu-walk mode so the VFS can switch to ref-walk mode or redo the walk if the inode has become invalid. If it's discovered that staying in rcu-walk mode gives a worth while performance improvement (unlikely) then the link path could be freed under rcu once potential side effects of the xfs inode reclaim sub-system have been analysed and dealt with if needed. Signed-off-by: Ian Kent Reviewed-by: Brian Foster --- fs/xfs/xfs_iops.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c index a607d6aca5c4..0a96183c5381 100644 --- a/fs/xfs/xfs_iops.c +++ b/fs/xfs/xfs_iops.c @@ -520,6 +520,9 @@ xfs_vn_get_link_inline( struct xfs_inode *ip = XFS_I(inode); char *link; + if (!dentry) + return ERR_PTR(-ECHILD); + ASSERT(ip->i_df.if_format == XFS_DINODE_FMT_LOCAL); /*