Skip to contentSkip to conversation

xfs: small bugfixes

PATCH RESEND New Eric Sandeen 4 participants 6 messages Oct 1, 2014 XFS filesystem

lore
Share & tools
lore
#1 lore raw

PATCH 1/2 xfs: don't send null bp to xfs_trans_brelse()

In this case, if bp is NULL, error is set, and we send a NULL bp to xfs_trans_brelse, which will try to dereference it.

Test whether we actually have a buffer before we try to free it.

Coverity spotted this.

Signed-off-by Eric Sandeen <[email protected]>

---

fs/xfs/libxfs/xfs_da_btree.c +2 -1
index 2c42ae2..fd82753 100644
@@ -2563,7 +2563,8 @@ xfs_da_get_buf(
 				    mapp, nmap, 0);
 	error = bp ? bp->b_error : -EIO;
 	if (error) {
-		xfs_trans_brelse(trans, bp);
+		if (bp)
+			xfs_trans_brelse(trans, bp);
 		goto out_free;
 	}

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

Reviewed-by on v1 1/2
#2 lore raw

Re: [PATCH 1/2] xfs: don't send null bp to xfs_trans_brelse()

Quoted 9 lines · Eric Sandeen

In this case, if bp is NULL, error is set, and we send a NULL bp to xfs_trans_brelse, which will try to dereference it.

Test whether we actually have a buffer before we try to free it.

Coverity spotted this.

Signed-off-by Eric Sandeen <[email protected]>

Looks good,

Reviewed-by Christoph Hellwig <[email protected]>

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#3 lore raw

PATCH 2/2 xfs: fix crc field handling in xfs_sb_to/from_disk

In reply to Eric Sandeen

I discovered this in userspace, but the same change applies to the kernel.

If we xfs_mdrestore an image from a non-crc filesystem, lo and behold the restored image has gained a CRC:

# db/xfs_metadump.sh -o /dev/sdc1 - | xfs_mdrestore - test.img
# xfs_db -c "sb 0" -c "p crc" /dev/sdc1
crc = 0 (correct)
# xfs_db -c "sb 0" -c "p crc" test.img
crc = 0xb6f8d6a0 (correct)

This is because xfs_sb_from_disk doesn't fill in sb_crc, but xfs_sb_to_disk(XFS_SB_ALL_BITS) does write the in-memory CRC to disk - so we get uninitialized memory on disk.

Fix this by always initializing sb_crc to 0 when we read the superblock, and masking out the CRC bit from ALL_BITS when we write it.

Signed-off-by Eric Sandeen <[email protected]>

---

fs/xfs/libxfs/xfs_sb.c +5 -0
index 8426e5e..5f902fa 100644
@@ -445,6 +445,8 @@ __xfs_sb_from_disk(
 	to->sb_features_incompat = be32_to_cpu(from->sb_features_incompat);
 	to->sb_features_log_incompat =
 				be32_to_cpu(from->sb_features_log_incompat);
+	/* crc is only used on disk, not in memory; just init to 0 here. */
+	to->sb_crc = 0;
 	to->sb_pad = 0;
 	to->sb_pquotino = be64_to_cpu(from->sb_pquotino);
 	to->sb_lsn = be64_to_cpu(from->sb_lsn);

@@ -550,6 +552,9 @@ xfs_sb_to_disk(
 	if (!fields)
 		return;
 
+	/* We should never write the crc here, it's updated in the IO path */
+	fields &= ~XFS_SB_CRC;
+
 	xfs_sb_quota_to_disk(to, from, &fields);
 	while (fields) {
 		f = (xfs_sb_field_t)xfs_lowbit64((__uint64_t)fields);

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#5 lore raw

Re: [PATCH 2/2] xfs: fix crc field handling in xfs_sb_to/from_disk

Quoted 1 line · Christoph Hellwig

Looks good (although it will conflict with Dave's sb_to_disk rework..)

That's ok. I'm slipping that superblock logging change to the next merge window as I haven't had a chance to look at the problems you reported yet and it's too close to the next merge window to be debugging changes to on-disk format handling code...

Cheers,

Dave.

Signature · 7 lines
Dave Chinner
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs