Skip to content

Commit 27a0c41

Browse files
author
Darrick J. Wong
committed
xfs: strengthen attr leaf block freemap checking
Check for erroneous overlapping freemap regions and collisions between freemap regions and the xattr leaf entry array. Note that we must explicitly zero out the extra freemaps in xfs_attr3_leaf_compact so that the in-memory buffer has a correctly initialized freemap array to satisfy the new verification code, even if subsequent code changes the contents before unlocking the buffer. Signed-off-by: "Darrick J. Wong" <djwong@kernel.org> Reviewed-by: Christoph Hellwig <hch@lst.de>
1 parent a165f7e commit 27a0c41

1 file changed

Lines changed: 55 additions & 0 deletions

File tree

fs/xfs/libxfs/xfs_attr_leaf.c

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,49 @@ xfs_attr_leaf_entries_end(
8585
xfs_attr3_leaf_hdr_size(leaf);
8686
}
8787

88+
static inline bool
89+
ichdr_freemaps_overlap(
90+
const struct xfs_attr3_icleaf_hdr *ichdr,
91+
unsigned int x,
92+
unsigned int y)
93+
{
94+
const unsigned int xend =
95+
ichdr->freemap[x].base + ichdr->freemap[x].size;
96+
const unsigned int yend =
97+
ichdr->freemap[y].base + ichdr->freemap[y].size;
98+
99+
/* empty slots do not overlap */
100+
if (!ichdr->freemap[x].size || !ichdr->freemap[y].size)
101+
return false;
102+
103+
return ichdr->freemap[x].base < yend && xend > ichdr->freemap[y].base;
104+
}
105+
106+
static inline xfs_failaddr_t
107+
xfs_attr_leaf_ichdr_freemaps_verify(
108+
const struct xfs_attr3_icleaf_hdr *ichdr,
109+
const struct xfs_attr_leafblock *leaf)
110+
{
111+
unsigned int entries_end =
112+
xfs_attr_leaf_entries_end(ichdr->count, leaf);
113+
int i;
114+
115+
if (ichdr_freemaps_overlap(ichdr, 0, 1))
116+
return __this_address;
117+
if (ichdr_freemaps_overlap(ichdr, 0, 2))
118+
return __this_address;
119+
if (ichdr_freemaps_overlap(ichdr, 1, 2))
120+
return __this_address;
121+
122+
for (i = 0; i < XFS_ATTR_LEAF_MAPSIZE; i++) {
123+
if (ichdr->freemap[i].size > 0 &&
124+
ichdr->freemap[i].base < entries_end)
125+
return __this_address;
126+
}
127+
128+
return NULL;
129+
}
130+
88131
/*
89132
* attr3 block 'firstused' conversion helpers.
90133
*
@@ -228,6 +271,8 @@ xfs_attr3_leaf_hdr_to_disk(
228271
hdr3->freemap[i].base = cpu_to_be16(from->freemap[i].base);
229272
hdr3->freemap[i].size = cpu_to_be16(from->freemap[i].size);
230273
}
274+
275+
ASSERT(xfs_attr_leaf_ichdr_freemaps_verify(from, to) == NULL);
231276
return;
232277
}
233278
to->hdr.info.forw = cpu_to_be32(from->forw);
@@ -243,6 +288,8 @@ xfs_attr3_leaf_hdr_to_disk(
243288
to->hdr.freemap[i].base = cpu_to_be16(from->freemap[i].base);
244289
to->hdr.freemap[i].size = cpu_to_be16(from->freemap[i].size);
245290
}
291+
292+
ASSERT(xfs_attr_leaf_ichdr_freemaps_verify(from, to) == NULL);
246293
}
247294

248295
static xfs_failaddr_t
@@ -395,6 +442,10 @@ xfs_attr3_leaf_verify(
395442
return __this_address;
396443
}
397444

445+
fa = xfs_attr_leaf_ichdr_freemaps_verify(&ichdr, leaf);
446+
if (fa)
447+
return fa;
448+
398449
return NULL;
399450
}
400451

@@ -1664,6 +1715,10 @@ xfs_attr3_leaf_compact(
16641715
ichdr_dst->freemap[0].base = xfs_attr3_leaf_hdr_size(leaf_src);
16651716
ichdr_dst->freemap[0].size = ichdr_dst->firstused -
16661717
ichdr_dst->freemap[0].base;
1718+
ichdr_dst->freemap[1].base = 0;
1719+
ichdr_dst->freemap[2].base = 0;
1720+
ichdr_dst->freemap[1].size = 0;
1721+
ichdr_dst->freemap[2].size = 0;
16671722

16681723
/* write the header back to initialise the underlying buffer */
16691724
xfs_attr3_leaf_hdr_to_disk(args->geo, leaf_dst, ichdr_dst);

0 commit comments

Comments
 (0)