Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit dac7a4b4 authored by Theodore Ts'o's avatar Theodore Ts'o
Browse files

ext4: lock the xattr block before checksuming it 

We must lock the xattr block before calculating or verifying the
checksum in order to avoid spurious checksum failures.

https://bugzilla.kernel.org/show_bug.cgi?id=193661



Reported-by: default avatarColin Ian King <colin.king@canonical.com>
Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
Cc: stable@vger.kernel.org
parent cd9cb405

fs/ext4/xattr.c

+31 −34
Original line number Diff line number Diff line
@@ -131,31 +131,26 @@ static __le32 ext4_xattr_block_csum(struct inode *inode, 
}



static int ext4_xattr_block_csum_verify(struct inode *inode,

					sector_t block_nr,

					struct ext4_xattr_header *hdr)

{

	if (ext4_has_metadata_csum(inode->i_sb) &&

	    (hdr->h_checksum != ext4_xattr_block_csum(inode, block_nr, hdr)))

		return 0;

	return 1;

}



static void ext4_xattr_block_csum_set(struct inode *inode,

				      sector_t block_nr,

				      struct ext4_xattr_header *hdr)

					struct buffer_head *bh)

{

	if (!ext4_has_metadata_csum(inode->i_sb))

		return;

	struct ext4_xattr_header *hdr = BHDR(bh);

	int ret = 1;



	hdr->h_checksum = ext4_xattr_block_csum(inode, block_nr, hdr);

	if (ext4_has_metadata_csum(inode->i_sb)) {

		lock_buffer(bh);

		ret = (hdr->h_checksum == ext4_xattr_block_csum(inode,

							bh->b_blocknr, hdr));

		unlock_buffer(bh);

	}

	return ret;

}



static inline int ext4_handle_dirty_xattr_block(handle_t *handle,

						struct inode *inode,

static void ext4_xattr_block_csum_set(struct inode *inode,

				      struct buffer_head *bh)

{

	ext4_xattr_block_csum_set(inode, bh->b_blocknr, BHDR(bh));

	return ext4_handle_dirty_metadata(handle, inode, bh);

	if (ext4_has_metadata_csum(inode->i_sb))

		BHDR(bh)->h_checksum = ext4_xattr_block_csum(inode,

						bh->b_blocknr, BHDR(bh));

}



static inline const struct xattr_handler *
@@ -233,7 +228,7 @@ ext4_xattr_check_block(struct inode *inode, struct buffer_head *bh) 
	if (BHDR(bh)->h_magic != cpu_to_le32(EXT4_XATTR_MAGIC) ||

	    BHDR(bh)->h_blocks != cpu_to_le32(1))

		return -EFSCORRUPTED;

	if (!ext4_xattr_block_csum_verify(inode, bh->b_blocknr, BHDR(bh)))

	if (!ext4_xattr_block_csum_verify(inode, bh))

		return -EFSBADCRC;

	error = ext4_xattr_check_names(BFIRST(bh), bh->b_data + bh->b_size,

				       bh->b_data);
@@ -618,23 +613,22 @@ ext4_xattr_release_block(handle_t *handle, struct inode *inode, 
			}

		}



		ext4_xattr_block_csum_set(inode, bh);

		/*

		 * Beware of this ugliness: Releasing of xattr block references

		 * from different inodes can race and so we have to protect

		 * from a race where someone else frees the block (and releases

		 * its journal_head) before we are done dirtying the buffer. In

		 * nojournal mode this race is harmless and we actually cannot

		 * call ext4_handle_dirty_xattr_block() with locked buffer as

		 * call ext4_handle_dirty_metadata() with locked buffer as

		 * that function can call sync_dirty_buffer() so for that case

		 * we handle the dirtying after unlocking the buffer.

		 */

		if (ext4_handle_valid(handle))

			error = ext4_handle_dirty_xattr_block(handle, inode,

							      bh);

			error = ext4_handle_dirty_metadata(handle, inode, bh);

		unlock_buffer(bh);

		if (!ext4_handle_valid(handle))

			error = ext4_handle_dirty_xattr_block(handle, inode,

							      bh);

			error = ext4_handle_dirty_metadata(handle, inode, bh);

		if (IS_SYNC(inode))

			ext4_handle_sync(handle);

		dquot_free_block(inode, EXT4_C2B(EXT4_SB(inode->i_sb), 1));
@@ -863,11 +857,12 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode, 
				ext4_xattr_cache_insert(ext4_mb_cache,

					bs->bh);

			}

			ext4_xattr_block_csum_set(inode, bs->bh);

			unlock_buffer(bs->bh);

			if (error == -EFSCORRUPTED)

				goto bad_block;

			if (!error)

				error = ext4_handle_dirty_xattr_block(handle,

				error = ext4_handle_dirty_metadata(handle,

								   inode,

								   bs->bh);

			if (error)
@@ -967,8 +962,9 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode, 
					ce->e_reusable = 0;

				ea_bdebug(new_bh, "reusing; refcount now=%d",

					  ref);

				ext4_xattr_block_csum_set(inode, new_bh);

				unlock_buffer(new_bh);

				error = ext4_handle_dirty_xattr_block(handle,

				error = ext4_handle_dirty_metadata(handle,

								   inode,

								   new_bh);

				if (error)
@@ -1020,11 +1016,12 @@ ext4_xattr_block_set(handle_t *handle, struct inode *inode, 
				goto getblk_failed;

			}

			memcpy(new_bh->b_data, s->base, new_bh->b_size);

			ext4_xattr_block_csum_set(inode, new_bh);

			set_buffer_uptodate(new_bh);

			unlock_buffer(new_bh);

			ext4_xattr_cache_insert(ext4_mb_cache, new_bh);

			error = ext4_handle_dirty_xattr_block(handle,

							      inode, new_bh);

			error = ext4_handle_dirty_metadata(handle, inode,

							   new_bh);

			if (error)

				goto cleanup;

		}