linux-ext4 - Re: [PATCH v5 27/28] ext4: xattr inode deduplication

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: <20170704183919.vpk7xuikjdh6s5gu@thunk.org>
Date:   Tue, 4 Jul 2017 14:39:19 -0400
From:   Theodore Ts'o <tytso@....edu>
To:     Tahsin Erdogan <tahsin@...gle.com>
Cc:     Andreas Dilger <adilger@...ger.ca>,
        "Darrick J . Wong" <darrick.wong@...cle.com>,
        linux-ext4@...r.kernel.org
Subject: Re: [PATCH v5 27/28] ext4: xattr inode deduplication

I'm going to fold in the following patch to the xattr inode
deduplicate patch to fix up some corner cases which
__ext4_xattr_set_credits() wasn't quite getting right.

				- Ted

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 4befc7369c0d..435a49811218 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -762,10 +762,11 @@ static void ext4_xattr_inode_free_quota(struct inode *inode, size_t len)
 	dquot_free_inode(inode);
 }
 
-static int __ext4_xattr_set_credits(struct super_block *sb,
+static int __ext4_xattr_set_credits(struct inode *inode,
 				    struct buffer_head *block_bh,
 				    size_t value_len)
 {
+	struct super_block *sb = inode->i_sb;
 	int credits;
 	int blocks;
 
@@ -775,12 +776,25 @@ static int __ext4_xattr_set_credits(struct super_block *sb,
 	 * 3) new xattr block
 	 * 4) block bitmap update for new xattr block
 	 * 5) group descriptor for new xattr block
+	 * 6) block bitmap update for old xattr block
+	 * 7) group descriptor for new old block
+	 *
+	 * 6 & 7 can happen if we have two racing threads T_a and T_b
+	 * which are each trying to set an xattr on inodes I_a and I_b
+	 * which were both initially sharing an xattr block.
 	 */
-	credits = 5;
+	credits = 7;
 
 	/* Quota updates. */
 	credits += EXT4_MAXQUOTAS_TRANS_BLOCKS(sb);
 
+	/*
+	 * In case of inline data, we may push out the data to a block,
+	 * so we need to reserve credits for this eventuality
+	 */
+	if (ext4_has_inline_data(inode))
+		credits += ext4_writepage_trans_blocks(inode) + 1;
+
 	/* We are done if ea_inode feature is not enabled. */
 	if (!ext4_has_feature_ea_inode(sb))
 		return credits;
@@ -2120,7 +2134,7 @@ ext4_xattr_set_handle(handle_t *handle, struct inode *inode, int name_index,
 			goto cleanup;
 		}
 
-		credits = __ext4_xattr_set_credits(inode->i_sb, bh, value_len);
+		credits = __ext4_xattr_set_credits(inode, bh, value_len);
 		brelse(bh);
 
 		if (!ext4_handle_has_enough_credits(handle, credits)) {
@@ -2243,7 +2257,7 @@ int ext4_xattr_set_credits(struct inode *inode, size_t value_len, int *credits)
 	if (IS_ERR(bh)) {
 		err = PTR_ERR(bh);
 	} else {
-		*credits = __ext4_xattr_set_credits(inode->i_sb, bh, value_len);
+		*credits = __ext4_xattr_set_credits(inode, bh, value_len);
 		brelse(bh);
 		err = 0;
 	}