From patchwork Thu Nov 11 03:39:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Ian Kent X-Patchwork-Id: 12613999 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 2C5B3C433F5 for ; Thu, 11 Nov 2021 03:39:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 06A5B61261 for ; Thu, 11 Nov 2021 03:39:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232091AbhKKDmX (ORCPT ); Wed, 10 Nov 2021 22:42:23 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52366 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230185AbhKKDmW (ORCPT ); Wed, 10 Nov 2021 22:42:22 -0500 Received: from smtp01.aussiebb.com.au (smtp01.aussiebb.com.au [IPv6:2403:5800:3:25::1001]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3863EC061766; Wed, 10 Nov 2021 19:39:34 -0800 (PST) Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp01.aussiebb.com.au (Postfix) with ESMTP id 0D8D710004F; Thu, 11 Nov 2021 14:39:27 +1100 (AEDT) X-Virus-Scanned: Debian amavisd-new at smtp01.aussiebb.com.au Received: from smtp01.aussiebb.com.au ([127.0.0.1]) by localhost (smtp01.aussiebb.com.au [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id vT_5Af2SkO8e; Thu, 11 Nov 2021 14:39:26 +1100 (AEDT) Received: by smtp01.aussiebb.com.au (Postfix, from userid 116) id EB611100070; Thu, 11 Nov 2021 14:39:26 +1100 (AEDT) Received: from mickey.themaw.net (unknown [100.72.131.210]) by smtp01.aussiebb.com.au (Postfix) with ESMTP id 3885410027F; Thu, 11 Nov 2021 14:39:25 +1100 (AEDT) Subject: [PATCH 1/2] vfs: check dentry is still valid in get_link() From: Ian Kent To: xfs , "Darrick J. Wong" Cc: Miklos Szeredi , Brian Foster , Al Viro , David Howells , linux-fsdevel , Kernel Mailing List Date: Thu, 11 Nov 2021 11:39:19 +0800 Message-ID: <163660195990.22525.6041281669106537689.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 --- fs/namei.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 Thu Nov 11 03:39:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Kent X-Patchwork-Id: 12614001 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 1F760C433FE for ; Thu, 11 Nov 2021 03:39:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 059266124D for ; Thu, 11 Nov 2021 03:39:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232674AbhKKDm0 (ORCPT ); Wed, 10 Nov 2021 22:42:26 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52370 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232426AbhKKDmX (ORCPT ); Wed, 10 Nov 2021 22:42:23 -0500 Received: from smtp01.aussiebb.com.au (smtp01.aussiebb.com.au [IPv6:2403:5800:3:25::1001]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1A0B7C061766; Wed, 10 Nov 2021 19:39:35 -0800 (PST) Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp01.aussiebb.com.au (Postfix) with ESMTP id AF1721002A7; Thu, 11 Nov 2021 14:39:33 +1100 (AEDT) X-Virus-Scanned: Debian amavisd-new at smtp01.aussiebb.com.au Received: from smtp01.aussiebb.com.au ([127.0.0.1]) by localhost (smtp01.aussiebb.com.au [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id lhI9-2cYJ_S0; Thu, 11 Nov 2021 14:39:33 +1100 (AEDT) Received: by smtp01.aussiebb.com.au (Postfix, from userid 116) id 9E14B1002A9; Thu, 11 Nov 2021 14:39:33 +1100 (AEDT) Received: from mickey.themaw.net (unknown [100.72.131.210]) by smtp01.aussiebb.com.au (Postfix) with ESMTP id F089F100299; Thu, 11 Nov 2021 14:39:30 +1100 (AEDT) Subject: [PATCH 2/2] xfs: make sure link path does not go away at access From: Ian Kent To: xfs , "Darrick J. Wong" Cc: Miklos Szeredi , Brian Foster , Al Viro , David Howells , linux-fsdevel , Kernel Mailing List Date: Thu, 11 Nov 2021 11:39:30 +0800 Message-ID: <163660197073.22525.11235124150551283676.stgit@mickey.themaw.net> In-Reply-To: <163660195990.22525.6041281669106537689.stgit@mickey.themaw.net> References: <163660195990.22525.6041281669106537689.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 to succeed in getting the ->get_link() method pointer but the link path string be deallocated while it's being used. Utilize the rcu mechanism to mitigate this risk. Suggested-by: Miklos Szeredi Signed-off-by: Ian Kent Reported-by: kernel test robot --- fs/xfs/kmem.h | 4 ++++ fs/xfs/xfs_inode.c | 4 ++-- fs/xfs/xfs_iops.c | 10 ++++++++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/fs/xfs/kmem.h b/fs/xfs/kmem.h index 54da6d717a06..c1bd1103b340 100644 --- a/fs/xfs/kmem.h +++ b/fs/xfs/kmem.h @@ -61,6 +61,10 @@ static inline void kmem_free(const void *ptr) { kvfree(ptr); } +static inline void kmem_free_rcu(const void *ptr) +{ + kvfree_rcu(ptr); +} static inline void * diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index a4f6f034fb81..aaa1911e61ed 100644 --- a/fs/xfs/xfs_inode.c +++ b/fs/xfs/xfs_inode.c @@ -2650,8 +2650,8 @@ xfs_ifree( * already been freed by xfs_attr_inactive. */ if (ip->i_df.if_format == XFS_DINODE_FMT_LOCAL) { - kmem_free(ip->i_df.if_u1.if_data); - ip->i_df.if_u1.if_data = NULL; + kmem_free_rcu(ip->i_df.if_u1.if_data); + RCU_INIT_POINTER(ip->i_df.if_u1.if_data, NULL); ip->i_df.if_bytes = 0; } diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c index a607d6aca5c4..2977e19da7b7 100644 --- a/fs/xfs/xfs_iops.c +++ b/fs/xfs/xfs_iops.c @@ -524,11 +524,17 @@ xfs_vn_get_link_inline( /* * The VFS crashes on a NULL pointer, so return -EFSCORRUPTED if - * if_data is junk. + * if_data is junk. Also, if the path walk is in rcu-walk mode + * and the inode link path has gone away due inode re-use we have + * no choice but to tell the VFS to redo the lookup. */ - link = ip->i_df.if_u1.if_data; + link = rcu_dereference(ip->i_df.if_u1.if_data); + if (!dentry && !link) + return ERR_PTR(-ECHILD); + if (XFS_IS_CORRUPT(ip->i_mount, !link)) return ERR_PTR(-EFSCORRUPTED); + return link; }