Skip to content

Commit 5689d23

Browse files
Darrick J. Wonggregkh
authored andcommitted
xfs: enforce one namespace per attribute
commit ea0b3e8 upstream. [backport: fix conflicts due to various xattr refactoring] Create a standardized helper function to enforce one namespace bit per extended attribute, and refactor all the open-coded hweight logic. This function is not a static inline to avoid porting hassles in userspace. Signed-off-by: Darrick J. Wong <djwong@kernel.org> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com> Acked-by: Darrick J. Wong <djwong@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 9716cdc commit 5689d23

File tree

6 files changed

+39
-15
lines changed

6 files changed

+39
-15
lines changed

fs/xfs/libxfs/xfs_attr.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1565,12 +1565,23 @@ xfs_attr_node_get(
15651565
return error;
15661566
}
15671567

1568+
/* Enforce that there is at most one namespace bit per attr. */
1569+
inline bool xfs_attr_check_namespace(unsigned int attr_flags)
1570+
{
1571+
return hweight32(attr_flags & XFS_ATTR_NSP_ONDISK_MASK) < 2;
1572+
}
1573+
15681574
/* Returns true if the attribute entry name is valid. */
15691575
bool
15701576
xfs_attr_namecheck(
1577+
unsigned int attr_flags,
15711578
const void *name,
15721579
size_t length)
15731580
{
1581+
/* Only one namespace bit allowed. */
1582+
if (!xfs_attr_check_namespace(attr_flags))
1583+
return false;
1584+
15741585
/*
15751586
* MAXNAMELEN includes the trailing null, but (name/length) leave it
15761587
* out, so use >= for the length check.

fs/xfs/libxfs/xfs_attr.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,9 @@ int xfs_attr_get(struct xfs_da_args *args);
547547
int xfs_attr_set(struct xfs_da_args *args);
548548
int xfs_attr_set_iter(struct xfs_attr_intent *attr);
549549
int xfs_attr_remove_iter(struct xfs_attr_intent *attr);
550-
bool xfs_attr_namecheck(const void *name, size_t length);
550+
bool xfs_attr_check_namespace(unsigned int attr_flags);
551+
bool xfs_attr_namecheck(unsigned int attr_flags, const void *name,
552+
size_t length);
551553
int xfs_attr_calc_size(struct xfs_da_args *args, int *local);
552554
void xfs_init_attr_trans(struct xfs_da_args *args, struct xfs_trans_res *tres,
553555
unsigned int *total);

fs/xfs/libxfs/xfs_attr_leaf.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -984,6 +984,10 @@ xfs_attr_shortform_to_leaf(
984984
nargs.hashval = xfs_da_hashname(sfe->nameval,
985985
sfe->namelen);
986986
nargs.attr_filter = sfe->flags & XFS_ATTR_NSP_ONDISK_MASK;
987+
if (!xfs_attr_check_namespace(sfe->flags)) {
988+
error = -EFSCORRUPTED;
989+
goto out;
990+
}
987991
error = xfs_attr3_leaf_lookup_int(bp, &nargs); /* set a->index */
988992
ASSERT(error == -ENOATTR);
989993
error = xfs_attr3_leaf_add(bp, &nargs);
@@ -1105,7 +1109,7 @@ xfs_attr_shortform_verify(
11051109
* one namespace flag per xattr, so we can just count the
11061110
* bits (i.e. hweight) here.
11071111
*/
1108-
if (hweight8(sfep->flags & XFS_ATTR_NSP_ONDISK_MASK) > 1)
1112+
if (!xfs_attr_check_namespace(sfep->flags))
11091113
return __this_address;
11101114

11111115
sfep = next_sfep;

fs/xfs/scrub/attr.c

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -193,14 +193,8 @@ xchk_xattr_listent(
193193
return;
194194
}
195195

196-
/* Only one namespace bit allowed. */
197-
if (hweight32(flags & XFS_ATTR_NSP_ONDISK_MASK) > 1) {
198-
xchk_fblock_set_corrupt(sx->sc, XFS_ATTR_FORK, args.blkno);
199-
goto fail_xref;
200-
}
201-
202196
/* Does this name make sense? */
203-
if (!xfs_attr_namecheck(name, namelen)) {
197+
if (!xfs_attr_namecheck(flags, name, namelen)) {
204198
xchk_fblock_set_corrupt(sx->sc, XFS_ATTR_FORK, args.blkno);
205199
goto fail_xref;
206200
}
@@ -501,6 +495,10 @@ xchk_xattr_rec(
501495
xchk_da_set_corrupt(ds, level);
502496
return 0;
503497
}
498+
if (!xfs_attr_check_namespace(ent->flags)) {
499+
xchk_da_set_corrupt(ds, level);
500+
return 0;
501+
}
504502

505503
if (ent->flags & XFS_ATTR_LOCAL) {
506504
lentry = (struct xfs_attr_leaf_name_local *)

fs/xfs/xfs_attr_item.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,10 @@ xfs_attri_validate(
522522
if (attrp->alfi_attr_filter & ~XFS_ATTRI_FILTER_MASK)
523523
return false;
524524

525+
if (!xfs_attr_check_namespace(attrp->alfi_attr_filter &
526+
XFS_ATTR_NSP_ONDISK_MASK))
527+
return false;
528+
525529
/* alfi_op_flags should be either a set or remove */
526530
switch (op) {
527531
case XFS_ATTRI_OP_FLAGS_SET:
@@ -572,7 +576,8 @@ xfs_attri_item_recover(
572576
*/
573577
attrp = &attrip->attri_format;
574578
if (!xfs_attri_validate(mp, attrp) ||
575-
!xfs_attr_namecheck(nv->name.i_addr, nv->name.i_len))
579+
!xfs_attr_namecheck(attrp->alfi_attr_filter, nv->name.i_addr,
580+
nv->name.i_len))
576581
return -EFSCORRUPTED;
577582

578583
error = xlog_recover_iget(mp, attrp->alfi_ino, &ip);
@@ -772,7 +777,8 @@ xlog_recover_attri_commit_pass2(
772777
}
773778

774779
attr_name = item->ri_buf[i].i_addr;
775-
if (!xfs_attr_namecheck(attr_name, attri_formatp->alfi_name_len)) {
780+
if (!xfs_attr_namecheck(attri_formatp->alfi_attr_filter, attr_name,
781+
attri_formatp->alfi_name_len)) {
776782
XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
777783
attri_formatp, len);
778784
return -EFSCORRUPTED;

fs/xfs/xfs_attr_list.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ xfs_attr_shortform_list(
8282
(dp->i_af.if_bytes + sf->hdr.count * 16) < context->bufsize)) {
8383
for (i = 0, sfe = &sf->list[0]; i < sf->hdr.count; i++) {
8484
if (XFS_IS_CORRUPT(context->dp->i_mount,
85-
!xfs_attr_namecheck(sfe->nameval,
85+
!xfs_attr_namecheck(sfe->flags,
86+
sfe->nameval,
8687
sfe->namelen)))
8788
return -EFSCORRUPTED;
8889
context->put_listent(context,
@@ -120,7 +121,8 @@ xfs_attr_shortform_list(
120121
for (i = 0, sfe = &sf->list[0]; i < sf->hdr.count; i++) {
121122
if (unlikely(
122123
((char *)sfe < (char *)sf) ||
123-
((char *)sfe >= ((char *)sf + dp->i_af.if_bytes)))) {
124+
((char *)sfe >= ((char *)sf + dp->i_af.if_bytes)) ||
125+
!xfs_attr_check_namespace(sfe->flags))) {
124126
XFS_CORRUPTION_ERROR("xfs_attr_shortform_list",
125127
XFS_ERRLEVEL_LOW,
126128
context->dp->i_mount, sfe,
@@ -174,7 +176,7 @@ xfs_attr_shortform_list(
174176
cursor->offset = 0;
175177
}
176178
if (XFS_IS_CORRUPT(context->dp->i_mount,
177-
!xfs_attr_namecheck(sbp->name,
179+
!xfs_attr_namecheck(sbp->flags, sbp->name,
178180
sbp->namelen))) {
179181
error = -EFSCORRUPTED;
180182
goto out;
@@ -465,7 +467,8 @@ xfs_attr3_leaf_list_int(
465467
}
466468

467469
if (XFS_IS_CORRUPT(context->dp->i_mount,
468-
!xfs_attr_namecheck(name, namelen)))
470+
!xfs_attr_namecheck(entry->flags, name,
471+
namelen)))
469472
return -EFSCORRUPTED;
470473
context->put_listent(context, entry->flags,
471474
name, namelen, valuelen);

0 commit comments

Comments
 (0)