Received: by 2002:ab2:69cc:0:b0:1fd:c486:4f03 with SMTP id n12csp114088lqp; Mon, 10 Jun 2024 21:16:08 -0700 (PDT) X-Forwarded-Encrypted: i=3; AJvYcCXcaaB+H0043CHbcgMCs+WmoNqLHDnzEmvJATnkxCN39ViZLvdES+iGZ0HRpOPVlPz9eAVPqfFKe46Xk/tb7df34W8wMH1TGrSEoLjHtw== X-Google-Smtp-Source: AGHT+IFnFeX4bDDSOvxDhE1k/kQOxyrSH7UEuRG59xRx9Fst26AxBBhiGwiepZMqq9CJdrATzICR X-Received: by 2002:a17:90a:6b43:b0:2bd:dce4:8f90 with SMTP id 98e67ed59e1d1-2c2bcadfcb5mr10409941a91.6.1718079368683; Mon, 10 Jun 2024 21:16:08 -0700 (PDT) ARC-Seal: i=2; a=rsa-sha256; t=1718079368; cv=pass; d=google.com; s=arc-20160816; b=Gv1hbzycK0Q8pkL4GK3h06ynYIyW4UyInVH5veEhVWQviqVR+tpMPRi9OfWpPvkJce 2pg969/Bd5wzOLTZr7U+MdD0Ervfw91zzT6gmtbopM6zOor14WgRaNHQOq3pXFtKsBgf 2dVHOkJTGpJbsEgwdBcVQ70v6uiw+jLuKxJg3VbtdWVDkrkY2TFU6uJUyoQHp3uYB5RO /wairiWa6L86D4lGS8WRxIJnFBhYOkNydC8DjjE7BXm772pap/juCDTklPXNqfYjg/n2 RCGMo+2bgYmu656CFu1Qkbc9wiaDz11j2vZBr6Te5dga85GGWSETNxNmlC9J4xkl4zRI ndEQ== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=cuWqGCjonoMFuxbnfFMq9beCr3z21M9jL/dtMf8LQx8=; fh=epcu9BdD+QZh42KnIqKfCj8VU/RlaW6CfaDIPRbZYOE=; b=R+LslerKVvIw3NMDvAxyz1S+UOTp2t64atBWMS9w8ve3ezC45LzSFYQhUdDiQUWgo4 c7FQRmR4R0XD964TcwueRKPh3yhmrpQ+kuw4fkf2Ht2sFu6YI+cbJ/1qjRapOaMINK7R 4r36nHcZNgpx81eycEAcu5XDzCf2ZWMlXIBMRcUsv0ryZQhcNH1WU5H3bBnsW0vyGyoi sQueI0vnnWNqFXCcVCuOXSTz3VGT1V+yVkHkggl8GAvJorjr8hSLmO0QKIGBYMYdbuAo UybCUP/gW6CdhqfSRlmGCVpaYPP2XITK4DNUbQlHoz5JEhJjp28RwdoZiLC434QTCJEu Gowg==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=VzH4xQIz; arc=pass (i=1 spf=pass spfdomain=gmail.com dkim=pass dkdomain=gmail.com dmarc=pass fromdomain=gmail.com); spf=pass (google.com: domain of linux-kernel+bounces-209242-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-209242-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [2604:1380:45e3:2400::1]) by mx.google.com with ESMTPS id 98e67ed59e1d1-2c2d0ff21e5si6069638a91.115.2024.06.10.21.16.08 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 10 Jun 2024 21:16:08 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel+bounces-209242-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=VzH4xQIz; arc=pass (i=1 spf=pass spfdomain=gmail.com dkim=pass dkdomain=gmail.com dmarc=pass fromdomain=gmail.com); spf=pass (google.com: domain of linux-kernel+bounces-209242-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-209242-linux.lists.archive=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 4A6732834EB for ; Tue, 11 Jun 2024 04:16:08 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 00AB316FF29; Tue, 11 Jun 2024 04:15:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="VzH4xQIz" Received: from mail-wm1-f53.google.com (mail-wm1-f53.google.com [209.85.128.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 73DA92570; Tue, 11 Jun 2024 04:15:56 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.53 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718079358; cv=none; b=SmuMXwmu5ermsYssFLX32GwO4y+AA7SUO8xL64/NVhdMconMVbBEsk9zidM8vcY6mraskDrClBD3AFBeHy+9o1k8uqSLO00ud+57b40uMgM9GlKAJKFeSDMBLRJH9HW+IRtfJEeapel8JyjnCmLw7WDXCAyY3OvfmDYbSeIyWDc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718079358; c=relaxed/simple; bh=N5EBSpcZkFiMtrh5336xOXwbPXQGG7sXkoPFqYbcyZA=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=PmZLiDN//PWxFVYH6bHUyPJk/8Wmb/AT0TvN7GMV+co6gkE1T9E78yuQPYAyCI4n64OF/8ukD+NaG4tHChvr3GRpcWmngzpkK3XUQ8Po0YxeZgoOlS9uWwYUfgI/+cdthsllVApFuV1T9iWMkconGTy842A96sPzchnunGoI1l0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=VzH4xQIz; arc=none smtp.client-ip=209.85.128.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wm1-f53.google.com with SMTP id 5b1f17b1804b1-42121d27861so45000995e9.0; Mon, 10 Jun 2024 21:15:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1718079355; x=1718684155; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=cuWqGCjonoMFuxbnfFMq9beCr3z21M9jL/dtMf8LQx8=; b=VzH4xQIz0mAY1EP+ubbMyWQaMvwO1D4E46tAja1dShdSiNIX04/iP3Wzf/zzQBgxqd ftbg1MeXHBWvY5HM+EX0RKAGOppie64+c+HCbqP7a8FrpFtoHtNp0Pr1icgbaufF1Evt +jKOmgDTaoy3iEo9rp/KUoq97vgYlHhLRnP7XOw0iVDXwKNuJatpJbNiw2m6CvwCG8QU j6zFd85TL8U9JXbx34mJiQp4fA+Yp/RGUhXC6qz/j1k8ebrqfBWqSSiRkHckpf0wPdAv eAJqwHmssgIfA5PVAbI4KZyqcHxVJ+4SCcwtlU6QhacCtYoNb6S8a3uY10pysD4QhRFZ khMQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718079355; x=1718684155; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=cuWqGCjonoMFuxbnfFMq9beCr3z21M9jL/dtMf8LQx8=; b=cdJYdw/ba331nVBd3DAgVl0HqXfbg2BIVliMH7Jac1CTJ9xVxPJNgDHWscQmMv3a2H pPWVV+cHcArdMl6XtKVm5GBd0DQylQ7eBp/MBVDxcCmSMRnrR1PepqMUPK0tbg58qqgX Pf7boO9qNlhIUrKIw4yBIndPKiEeGrxppBi+0ynkLzCi4F0zmApebAD+fhQAoZqSKB43 /LK7QEepiIKNKuUNfDSOhOBq4yw9U7uWPAV2S1u/Yfjso9cZ+QSKMk1TeokX2uL9sqjg ZWixo4FIRFH+KSUOqgFpoy4HOgUHRTU7zgg9eqICp7wCcPyuvGkJaQ6yiLepSFkTxPN7 bpzA== X-Forwarded-Encrypted: i=1; AJvYcCVL6SNyO/G/cHx5Q0oL0JnQhjIeh7OrGNN5REml3oCPp0/R319DG8fRQ9y6L2x9mpqdh7AMNa1n3ft2k7qeBXMtAssis2n7ctRLZIgqbkO3MRJZTVu4emnsnDYM9uqGJnVqA74e59pFu58H/A== X-Gm-Message-State: AOJu0YyEOmKocfjblAF5cDlyty+G6YF0iSKIqWeEMm7Mp2sU3t77v//Z WdtfK9PjpO3mItezb2ozPWodJkTbqA2BVMfr+g7zaC9kKeZcD8e7 X-Received: by 2002:a05:600c:1ca3:b0:421:6b79:8900 with SMTP id 5b1f17b1804b1-4216b798c9cmr64041135e9.41.1718079354540; Mon, 10 Jun 2024 21:15:54 -0700 (PDT) Received: from f.. (cst-prg-65-249.cust.vodafone.cz. [46.135.65.249]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-42158149008sm194405165e9.29.2024.06.10.21.15.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 10 Jun 2024 21:15:54 -0700 (PDT) From: Mateusz Guzik To: brauner@kernel.org Cc: viro@zeniv.linux.org.uk, jack@suse.cz, dave@fromorbit.com, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, Mateusz Guzik Subject: [PATCH] vfs: partially sanitize i_state zeroing on inode creation Date: Tue, 11 Jun 2024 06:15:40 +0200 Message-ID: <20240611041540.495840-1-mjguzik@gmail.com> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit new_inode used to have the following: spin_lock(&inode_lock); inodes_stat.nr_inodes++; list_add(&inode->i_list, &inode_in_use); list_add(&inode->i_sb_list, &sb->s_inodes); inode->i_ino = ++last_ino; inode->i_state = 0; spin_unlock(&inode_lock); over time things disappeared, got moved around or got replaced (global inode lock with a per-inode lock), eventually this got reduced to: spin_lock(&inode->i_lock); inode->i_state = 0; spin_unlock(&inode->i_lock); But the lock acquire here does not synchronize against anyone. Additionally iget5_locked performs i_state = 0 assignment without any locks to begin with and the two combined look confusing at best. It looks like the current state is a leftover which was not cleaned up. Ideally it would be an invariant that i_state == 0 to begin with, but achieving that would require dealing with all filesystem alloc handlers one by one. In the meantime drop the misleading locking and move i_state zeroing to alloc_inode so that others don't need to deal with it by hand. Signed-off-by: Mateusz Guzik --- I diffed this against fs-next + my inode hash patch as it adds one i_state = 0 case. Should that patch not be accepted this bit can be easily dropped from this one. I brought the entire thing up quite some time ago [1] and Dave Chinner noted that perhaps the lock has a side effect of providing memory barriers which otherwise would not be there and which are needed by someone. For new_inode and alloc_inode consumers all fences are already there anyway due to immediate lock usage. Arguably new_inode_pseudo escape without it but I don't find the code at hand to be affected in any meanignful way -- the only 2 consumers (get_pipe_inode and sock_alloc) perform numerous other stores to the inode immediately after. By the time it gets added to anything looking at i_state, flushing that should be handled by whatever thing which adds it. Mentioning this just in case. [1] https://lore.kernel.org/all/CAGudoHF_Y0shcU+AMRRdN5RQgs9L_HHvBH8D4K=7_0X72kYy2g@mail.gmail.com/ fs/inode.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/fs/inode.c b/fs/inode.c index 149adf8ab0ea..3967e68311a6 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -276,6 +276,10 @@ static struct inode *alloc_inode(struct super_block *sb) return NULL; } + /* + * FIXME: the code should be able to assert i_state == 0 instead. + */ + inode->i_state = 0; return inode; } @@ -1023,14 +1027,7 @@ EXPORT_SYMBOL(get_next_ino); */ struct inode *new_inode_pseudo(struct super_block *sb) { - struct inode *inode = alloc_inode(sb); - - if (inode) { - spin_lock(&inode->i_lock); - inode->i_state = 0; - spin_unlock(&inode->i_lock); - } - return inode; + return alloc_inode(sb); } /** @@ -1254,7 +1251,6 @@ struct inode *iget5_locked(struct super_block *sb, unsigned long hashval, struct inode *new = alloc_inode(sb); if (new) { - new->i_state = 0; inode = inode_insert5(new, hashval, test, set, data); if (unlikely(inode != new)) destroy_inode(new); @@ -1297,7 +1293,6 @@ struct inode *iget5_locked_rcu(struct super_block *sb, unsigned long hashval, new = alloc_inode(sb); if (new) { - new->i_state = 0; inode = inode_insert5(new, hashval, test, set, data); if (unlikely(inode != new)) destroy_inode(new); -- 2.43.0