afs: Fix directory entry size calculation

The number of dirent records used by an AFS directory entry should be
calculated using the assumption that there is a 16-byte name field in the
first block, rather than a 20-byte name field (which is actually the case).
This miscalculation is historic and effectively standard, so we have to use
it.

The calculation we need to use is:

	1 + (((strlen(name) + 1) + 15) >> 5)

where we are adding one to the strlen() result to account for the NUL
termination.

Fix this by the following means:

 (1) Create an inline function to do the calculation for a given name
     length.

 (2) Use the function to calculate the number of records used for a dirent
     in afs_dir_iterate_block().

     Use this to move the over-end check out of the loop since it only
     needs to be done once.

     Further use this to only go through the loop for the 2nd+ records
     composing an entry.  The only test there now is for if the record is
     allocated - and we already checked the first block at the top of the
     outer loop.

 (3) Add a max name length check in afs_dir_iterate_block().

 (4) Make afs_edit_dir_add() and afs_edit_dir_remove() use the function
     from (1) to calculate the number of blocks rather than doing it
     incorrectly themselves.

Fixes: 63a4681ff3 ("afs: Locally edit directory data for mkdir/create/unlink/...")
Fixes: ^1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: David Howells <dhowells@redhat.com>
Tested-by: Marc Dionne <marc.dionne@auristor.com>
This commit is contained in:
David Howells 2020-12-23 10:39:57 +00:00
parent 26982a89ca
commit 366911cd76
4 changed files with 43 additions and 28 deletions

View File

@ -350,7 +350,7 @@ static int afs_dir_iterate_block(struct afs_vnode *dvnode,
unsigned blkoff) unsigned blkoff)
{ {
union afs_xdr_dirent *dire; union afs_xdr_dirent *dire;
unsigned offset, next, curr; unsigned offset, next, curr, nr_slots;
size_t nlen; size_t nlen;
int tmp; int tmp;
@ -363,13 +363,12 @@ static int afs_dir_iterate_block(struct afs_vnode *dvnode,
offset < AFS_DIR_SLOTS_PER_BLOCK; offset < AFS_DIR_SLOTS_PER_BLOCK;
offset = next offset = next
) { ) {
next = offset + 1;
/* skip entries marked unused in the bitmap */ /* skip entries marked unused in the bitmap */
if (!(block->hdr.bitmap[offset / 8] & if (!(block->hdr.bitmap[offset / 8] &
(1 << (offset % 8)))) { (1 << (offset % 8)))) {
_debug("ENT[%zu.%u]: unused", _debug("ENT[%zu.%u]: unused",
blkoff / sizeof(union afs_xdr_dir_block), offset); blkoff / sizeof(union afs_xdr_dir_block), offset);
next = offset + 1;
if (offset >= curr) if (offset >= curr)
ctx->pos = blkoff + ctx->pos = blkoff +
next * sizeof(union afs_xdr_dirent); next * sizeof(union afs_xdr_dirent);
@ -381,35 +380,39 @@ static int afs_dir_iterate_block(struct afs_vnode *dvnode,
nlen = strnlen(dire->u.name, nlen = strnlen(dire->u.name,
sizeof(*block) - sizeof(*block) -
offset * sizeof(union afs_xdr_dirent)); offset * sizeof(union afs_xdr_dirent));
if (nlen > AFSNAMEMAX - 1) {
_debug("ENT[%zu]: name too long (len %u/%zu)",
blkoff / sizeof(union afs_xdr_dir_block),
offset, nlen);
return afs_bad(dvnode, afs_file_error_dir_name_too_long);
}
_debug("ENT[%zu.%u]: %s %zu \"%s\"", _debug("ENT[%zu.%u]: %s %zu \"%s\"",
blkoff / sizeof(union afs_xdr_dir_block), offset, blkoff / sizeof(union afs_xdr_dir_block), offset,
(offset < curr ? "skip" : "fill"), (offset < curr ? "skip" : "fill"),
nlen, dire->u.name); nlen, dire->u.name);
/* work out where the next possible entry is */ nr_slots = afs_dir_calc_slots(nlen);
for (tmp = nlen; tmp > 15; tmp -= sizeof(union afs_xdr_dirent)) { next = offset + nr_slots;
if (next >= AFS_DIR_SLOTS_PER_BLOCK) { if (next > AFS_DIR_SLOTS_PER_BLOCK) {
_debug("ENT[%zu.%u]:" _debug("ENT[%zu.%u]:"
" %u travelled beyond end dir block" " %u extends beyond end dir block"
" (len %u/%zu)", " (len %zu)",
blkoff / sizeof(union afs_xdr_dir_block),
offset, next, nlen);
return afs_bad(dvnode, afs_file_error_dir_over_end);
}
/* Check that the name-extension dirents are all allocated */
for (tmp = 1; tmp < nr_slots; tmp++) {
unsigned int ix = offset + tmp;
if (!(block->hdr.bitmap[ix / 8] & (1 << (ix % 8)))) {
_debug("ENT[%zu.u]:"
" %u unmarked extension (%u/%u)",
blkoff / sizeof(union afs_xdr_dir_block), blkoff / sizeof(union afs_xdr_dir_block),
offset, next, tmp, nlen); offset, tmp, nr_slots);
return afs_bad(dvnode, afs_file_error_dir_over_end);
}
if (!(block->hdr.bitmap[next / 8] &
(1 << (next % 8)))) {
_debug("ENT[%zu.%u]:"
" %u unmarked extension (len %u/%zu)",
blkoff / sizeof(union afs_xdr_dir_block),
offset, next, tmp, nlen);
return afs_bad(dvnode, afs_file_error_dir_unmarked_ext); return afs_bad(dvnode, afs_file_error_dir_unmarked_ext);
} }
_debug("ENT[%zu.%u]: ext %u/%zu",
blkoff / sizeof(union afs_xdr_dir_block),
next, tmp, nlen);
next++;
} }
/* skip if starts before the current position */ /* skip if starts before the current position */

View File

@ -215,8 +215,7 @@ void afs_edit_dir_add(struct afs_vnode *vnode,
} }
/* Work out how many slots we're going to need. */ /* Work out how many slots we're going to need. */
need_slots = round_up(12 + name->len + 1 + 4, AFS_DIR_DIRENT_SIZE); need_slots = afs_dir_calc_slots(name->len);
need_slots /= AFS_DIR_DIRENT_SIZE;
meta_page = kmap(page0); meta_page = kmap(page0);
meta = &meta_page->blocks[0]; meta = &meta_page->blocks[0];
@ -393,8 +392,7 @@ void afs_edit_dir_remove(struct afs_vnode *vnode,
} }
/* Work out how many slots we're going to discard. */ /* Work out how many slots we're going to discard. */
need_slots = round_up(12 + name->len + 1 + 4, AFS_DIR_DIRENT_SIZE); need_slots = afs_dir_calc_slots(name->len);
need_slots /= AFS_DIR_DIRENT_SIZE;
meta_page = kmap(page0); meta_page = kmap(page0);
meta = &meta_page->blocks[0]; meta = &meta_page->blocks[0];

View File

@ -58,7 +58,8 @@ union afs_xdr_dirent {
/* When determining the number of dirent slots needed to /* When determining the number of dirent slots needed to
* represent a directory entry, name should be assumed to be 16 * represent a directory entry, name should be assumed to be 16
* bytes, due to a now-standardised (mis)calculation, but it is * bytes, due to a now-standardised (mis)calculation, but it is
* in fact 20 bytes in size. * in fact 20 bytes in size. afs_dir_calc_slots() should be
* used for this.
* *
* For names longer than (16 or) 20 bytes, extra slots should * For names longer than (16 or) 20 bytes, extra slots should
* be annexed to this one using the extended_name format. * be annexed to this one using the extended_name format.
@ -101,4 +102,15 @@ struct afs_xdr_dir_page {
union afs_xdr_dir_block blocks[AFS_DIR_BLOCKS_PER_PAGE]; union afs_xdr_dir_block blocks[AFS_DIR_BLOCKS_PER_PAGE];
}; };
/*
* Calculate the number of dirent slots required for any given name length.
* The calculation is made assuming the part of the name in the first slot is
* 16 bytes, rather than 20, but this miscalculation is now standardised.
*/
static inline unsigned int afs_dir_calc_slots(size_t name_len)
{
name_len++; /* NUL-terminated */
return 1 + ((name_len + 15) / AFS_DIR_DIRENT_SIZE);
}
#endif /* XDR_FS_H */ #endif /* XDR_FS_H */

View File

@ -231,6 +231,7 @@ enum afs_file_error {
afs_file_error_dir_bad_magic, afs_file_error_dir_bad_magic,
afs_file_error_dir_big, afs_file_error_dir_big,
afs_file_error_dir_missing_page, afs_file_error_dir_missing_page,
afs_file_error_dir_name_too_long,
afs_file_error_dir_over_end, afs_file_error_dir_over_end,
afs_file_error_dir_small, afs_file_error_dir_small,
afs_file_error_dir_unmarked_ext, afs_file_error_dir_unmarked_ext,
@ -488,6 +489,7 @@ enum afs_cb_break_reason {
EM(afs_file_error_dir_bad_magic, "DIR_BAD_MAGIC") \ EM(afs_file_error_dir_bad_magic, "DIR_BAD_MAGIC") \
EM(afs_file_error_dir_big, "DIR_BIG") \ EM(afs_file_error_dir_big, "DIR_BIG") \
EM(afs_file_error_dir_missing_page, "DIR_MISSING_PAGE") \ EM(afs_file_error_dir_missing_page, "DIR_MISSING_PAGE") \
EM(afs_file_error_dir_name_too_long, "DIR_NAME_TOO_LONG") \
EM(afs_file_error_dir_over_end, "DIR_ENT_OVER_END") \ EM(afs_file_error_dir_over_end, "DIR_ENT_OVER_END") \
EM(afs_file_error_dir_small, "DIR_SMALL") \ EM(afs_file_error_dir_small, "DIR_SMALL") \
EM(afs_file_error_dir_unmarked_ext, "DIR_UNMARKED_EXT") \ EM(afs_file_error_dir_unmarked_ext, "DIR_UNMARKED_EXT") \