linux-ext4 - [PATCH v2 02/19] fs: don't take the i_lock in inode_inc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20171216134656.15561-3-jlayton@kernel.org>
Date:   Sat, 16 Dec 2017 08:46:39 -0500
From:   Jeff Layton <jlayton@...nel.org>
To:     linux-fsdevel@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, viro@...iv.linux.org.uk,
        linux-nfs@...r.kernel.org, bfields@...ldses.org, neilb@...e.de,
        jack@...e.de, linux-ext4@...r.kernel.org, tytso@....edu,
        adilger.kernel@...ger.ca, linux-xfs@...r.kernel.org,
        darrick.wong@...cle.com, david@...morbit.com,
        linux-btrfs@...r.kernel.org, clm@...com, jbacik@...com,
        dsterba@...e.com, linux-integrity@...r.kernel.org,
        zohar@...ux.vnet.ibm.com, dmitry.kasatkin@...il.com,
        linux-afs@...ts.infradead.org, dhowells@...hat.com
Subject: [PATCH v2 02/19] fs: don't take the i_lock in inode_inc_iversion

From: Jeff Layton <jlayton@...hat.com>

The rationale for taking the i_lock when incrementing this value is
lost in antiquity. The readers of the field don't take it (at least
not universally), so my assumption is that it was only done here to
serialize incrementors.

If that is indeed the case, then we can drop the i_lock from this
codepath and treat it as a atomic64_t for the purposes of
incrementing it. This allows us to use inode_inc_iversion without
any danger of lock inversion.

Note that the read side is not fetched atomically with this change.
The assumption here is that that is not a critical issue since the
i_version is not fully synchronized with anything else anyway.

Signed-off-by: Jeff Layton <jlayton@...hat.com>
---
 include/linux/fs.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

I'm not dead-set on this patch, but I was a little leery of adding
the i_lock into all of the places that do inode->i_version++ today,
even as an interim step. We can probably drop this patch if it's too
ugly to live...

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 5001e77342fd..c234fac4bb77 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2136,9 +2136,9 @@ inode_set_iversion_queried(struct inode *inode, const u64 new)
 static inline bool
 inode_maybe_inc_iversion(struct inode *inode, bool force)
 {
-	spin_lock(&inode->i_lock);
-	inode->i_version++;
-	spin_unlock(&inode->i_lock);
+	atomic64_t *ivp = (atomic64_t *)&inode->i_version;
+
+	atomic64_inc(ivp);
 	return true;
 }

-- 
2.14.3