Skip to content

Commit 5170711

Browse files
Dave ChinnerBen Myers
Dave Chinner
authored and
Ben Myers
committed
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 <[email protected]> Debugged-by: Michael L. Semon <[email protected]> Signed-off-by: Dave Chinner <[email protected]> Reviewed-by: Ben Myers <[email protected]> Signed-off-by: Ben Myers <[email protected]> (cherry picked from commit 8a1fd29)
1 parent 47ad2fc commit 5170711

File tree

2 files changed

+4
-2
lines changed

2 files changed

+4
-2
lines changed

fs/xfs/xfs_attr_leaf.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ struct xfs_attr3_leaf_hdr {
128128
__u8 holes;
129129
__u8 pad1;
130130
struct xfs_attr_leaf_map freemap[XFS_ATTR_LEAF_MAPSIZE];
131+
__be32 pad2; /* 64 bit alignment */
131132
};
132133

133134
#define XFS_ATTR3_LEAF_CRC_OFF (offsetof(struct xfs_attr3_leaf_hdr, info.crc))

fs/xfs/xfs_dir2_format.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,7 @@ struct xfs_dir3_blk_hdr {
266266
struct xfs_dir3_data_hdr {
267267
struct xfs_dir3_blk_hdr hdr;
268268
xfs_dir2_data_free_t best_free[XFS_DIR2_DATA_FD_COUNT];
269+
__be32 pad; /* 64 bit alignment */
269270
};
270271

271272
#define XFS_DIR3_DATA_CRC_OFF offsetof(struct xfs_dir3_data_hdr, hdr.crc)
@@ -477,7 +478,7 @@ struct xfs_dir3_leaf_hdr {
477478
struct xfs_da3_blkinfo info; /* header for da routines */
478479
__be16 count; /* count of entries */
479480
__be16 stale; /* count of stale entries */
480-
__be32 pad;
481+
__be32 pad; /* 64 bit alignment */
481482
};
482483

483484
struct xfs_dir3_icleaf_hdr {
@@ -715,7 +716,7 @@ struct xfs_dir3_free_hdr {
715716
__be32 firstdb; /* db of first entry */
716717
__be32 nvalid; /* count of valid entries */
717718
__be32 nused; /* count of used entries */
718-
__be32 pad; /* 64 bit alignment. */
719+
__be32 pad; /* 64 bit alignment */
719720
};
720721

721722
struct xfs_dir3_free {

0 commit comments

Comments
 (0)