aboutsummaryrefslogtreecommitdiffstats
path: root/fs/xfs
diff options
context:
space:
mode:
authorDave Chinner <dchinner@redhat.com>2013-06-11 22:19:07 -0400
committerBen Myers <bpm@sgi.com>2013-06-14 16:59:16 -0400
commit5170711df79b284cf95b3924322e8ac4c0fd6c76 (patch)
tree31395c9ee299cb7a21e51ab2a2b2971f83a7e9b9 /fs/xfs
parent47ad2fcba9ddd0630acccb13c71f19a818947751 (diff)
xfs: fix implicit padding in directory and attr CRC formats
Michael L. Semon has been testing CRC patches on a 32 bit system and been seeing assert failures in the directory code from xfs/080. Thanks to Michael's heroic efforts with printk debugging, we found that the problem was that the last free space being left in the directory structure was too small to fit a unused tag structure and it was being corrupted and attempting to log a region out of bounds. Hence the assert failure looked something like: ..... #5 calling xfs_dir2_data_log_unused() 36 32 #1 4092 4095 4096 #2 8182 8183 4096 XFS: Assertion failed: first <= last && last < BBTOB(bp->b_length), file: fs/xfs/xfs_trans_buf.c, line: 568 Where #1 showed the first region of the dup being logged (i.e. the last 4 bytes of a directory buffer) and #2 shows the corrupt values being calculated from the length of the dup entry which overflowed the size of the buffer. It turns out that the problem was not in the logging code, nor in the freespace handling code. It is an initial condition bug that only shows up on 32 bit systems. When a new buffer is initialised, where's the freespace that is set up: [ 172.316249] calling xfs_dir2_leaf_addname() from xfs_dir_createname() [ 172.316346] #9 calling xfs_dir2_data_log_unused() [ 172.316351] #1 calling xfs_trans_log_buf() 60 63 4096 [ 172.316353] #2 calling xfs_trans_log_buf() 4094 4095 4096 Note the offset of the first region being logged? It's 60 bytes into the buffer. Once I saw that, I pretty much knew that the bug was going to be caused by this. Essentially, all direct entries are rounded to 8 bytes in length, and all entries start with an 8 byte alignment. This means that we can decode inplace as variables are naturally aligned. With the directory data supposedly starting on a 8 byte boundary, and all entries padded to 8 bytes, the minimum freespace in a directory block is supposed to be 8 bytes, which is large enough to fit a unused data entry structure (6 bytes in size). The fact we only have 4 bytes of free space indicates a directory data block alignment problem. And what do you know - there's an implicit hole in the directory data block header for the CRC format, which means the header is 60 byte on 32 bit intel systems and 64 bytes on 64 bit systems. Needs padding. And while looking at the structures, I found the same problem in the attr leaf header. Fix them both. Note that this only affects 32 bit systems with CRCs enabled. Everything else is just fine. Note that CRC enabled filesystems created before this fix on such systems will not be readable with this fix applied. Reported-by: Michael L. Semon <mlsemon35@gmail.com> Debugged-by: Michael L. Semon <mlsemon35@gmail.com> Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Ben Myers <bpm@sgi.com> Signed-off-by: Ben Myers <bpm@sgi.com> (cherry picked from commit 8a1fd2950e1fe267e11fc8c85dcaa6b023b51b60)
Diffstat (limited to 'fs/xfs')
-rw-r--r--fs/xfs/xfs_attr_leaf.h1
-rw-r--r--fs/xfs/xfs_dir2_format.h5
2 files changed, 4 insertions, 2 deletions
diff --git a/fs/xfs/xfs_attr_leaf.h b/fs/xfs/xfs_attr_leaf.h
index f9d7846097e2..444a7704596c 100644
--- a/fs/xfs/xfs_attr_leaf.h
+++ b/fs/xfs/xfs_attr_leaf.h
@@ -128,6 +128,7 @@ struct xfs_attr3_leaf_hdr {
128 __u8 holes; 128 __u8 holes;
129 __u8 pad1; 129 __u8 pad1;
130 struct xfs_attr_leaf_map freemap[XFS_ATTR_LEAF_MAPSIZE]; 130 struct xfs_attr_leaf_map freemap[XFS_ATTR_LEAF_MAPSIZE];
131 __be32 pad2; /* 64 bit alignment */
131}; 132};
132 133
133#define XFS_ATTR3_LEAF_CRC_OFF (offsetof(struct xfs_attr3_leaf_hdr, info.crc)) 134#define XFS_ATTR3_LEAF_CRC_OFF (offsetof(struct xfs_attr3_leaf_hdr, info.crc))
diff --git a/fs/xfs/xfs_dir2_format.h b/fs/xfs/xfs_dir2_format.h
index 995f1f505a52..7826782b8d78 100644
--- a/fs/xfs/xfs_dir2_format.h
+++ b/fs/xfs/xfs_dir2_format.h
@@ -266,6 +266,7 @@ struct xfs_dir3_blk_hdr {
266struct xfs_dir3_data_hdr { 266struct xfs_dir3_data_hdr {
267 struct xfs_dir3_blk_hdr hdr; 267 struct xfs_dir3_blk_hdr hdr;
268 xfs_dir2_data_free_t best_free[XFS_DIR2_DATA_FD_COUNT]; 268 xfs_dir2_data_free_t best_free[XFS_DIR2_DATA_FD_COUNT];
269 __be32 pad; /* 64 bit alignment */
269}; 270};
270 271
271#define XFS_DIR3_DATA_CRC_OFF offsetof(struct xfs_dir3_data_hdr, hdr.crc) 272#define XFS_DIR3_DATA_CRC_OFF offsetof(struct xfs_dir3_data_hdr, hdr.crc)
@@ -477,7 +478,7 @@ struct xfs_dir3_leaf_hdr {
477 struct xfs_da3_blkinfo info; /* header for da routines */ 478 struct xfs_da3_blkinfo info; /* header for da routines */
478 __be16 count; /* count of entries */ 479 __be16 count; /* count of entries */
479 __be16 stale; /* count of stale entries */ 480 __be16 stale; /* count of stale entries */
480 __be32 pad; 481 __be32 pad; /* 64 bit alignment */
481}; 482};
482 483
483struct xfs_dir3_icleaf_hdr { 484struct xfs_dir3_icleaf_hdr {
@@ -715,7 +716,7 @@ struct xfs_dir3_free_hdr {
715 __be32 firstdb; /* db of first entry */ 716 __be32 firstdb; /* db of first entry */
716 __be32 nvalid; /* count of valid entries */ 717 __be32 nvalid; /* count of valid entries */
717 __be32 nused; /* count of used entries */ 718 __be32 nused; /* count of used entries */
718 __be32 pad; /* 64 bit alignment. */ 719 __be32 pad; /* 64 bit alignment */
719}; 720};
720 721
721struct xfs_dir3_free { 722struct xfs_dir3_free {