udf_next_aext() decodes each allocation descriptor straight into the caller's eloc, elen and etype, and follows a continuation descriptor into the next allocation extent. If that allocation extent holds no descriptors, it returns 0 for the end of the list, but by then eloc, elen and etype describe the continuation descriptor itself: the block of the empty allocation extent, one block long, type 3. The kernel creates such lists itself: udf_delete_aext() leaves the last allocation extent of a list empty when it removes its only descriptor, and udf_do_extend_file() and udf_extend_file() already handle a list that ends in an empty one. Two callers use the outputs after a return of 0. udf_discard_prealloc() walks to the last extent and, if it is a preallocation, deletes it with udf_delete_aext() and frees eloc/elen. When an empty allocation extent follows, udf_delete_aext() removes the continuation and frees the empty block, and udf_discard_prealloc() then frees that block a second time instead of the preallocated blocks. On a space bitmap the preallocated blocks are leaked and the free block count drifts. On an unallocated space table the second free adds a second free extent for the same block, which is later handed out twice: fsx as run by generic/091 and generic/263 ends up with two parts of its test file in one block and reads back bad data. udf_table_prealloc_blocks() can likewise take an empty allocation extent at the end of the table's own list for a free extent starting at the goal block. Only update the outputs once a descriptor other than a continuation has been found. The other callers use the outputs only on a positive return, or not at all, with one exception: when udf_table_free_blocks() appends a new extent, it keeps the partition reference of whatever eloc last held, which for a table whose list is a single continuation to an empty allocation extent would now be uninitialised. Take it from the freed blocks instead. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable@vger.kernel.org Signed-off-by: Matthias Goergens --- Reproducer, with fsx from fstests and mkudffs from udftools: truncate --size=2G udf.img mkudffs --blocksize=512 --space=unalloctable udf.img mount -t udf -o loop udf.img /mnt fsx -N 10000 -l 500000 -r 4096 -t 512 -w 512 -Z -R -W /mnt/junk Without this patch fsx stops with READ BAD DATA after about 9850 operations, in every run; with it, all 10000 operations complete. With --space=unallocbitmap fsx completes either way. fs/udf/balloc.c | 1 + fs/udf/inode.c | 21 +++++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c index 2ec577b4321c..d863ee201cbf 100644 --- a/fs/udf/balloc.c +++ b/fs/udf/balloc.c @@ -457,6 +457,7 @@ static void udf_table_free_blocks(struct super_block *sb, int adsize; + eloc.partitionReferenceNum = bloc->partitionReferenceNum; eloc.logicalBlockNum = start; elen = EXT_RECORDED_ALLOCATED | (count << sb->s_blocksize_bits); diff --git a/fs/udf/inode.c b/fs/udf/inode.c index 71386e7ac796..0e55f749bc48 100644 --- a/fs/udf/inode.c +++ b/fs/udf/inode.c @@ -2266,22 +2266,35 @@ void udf_write_aext(struct inode *inode, struct extent_position *epos, /* * Returns 1 on success, -errno on error, 0 on hit EOF. + * + * eloc, elen and etype are only updated when the next allocation descriptor + * was found. In particular, when a chain of indirect extents ends in an + * empty one, following the trailing CONTINUE descriptor and hitting EOF must + * not clobber them with the location and length of that CONTINUE: callers + * keep using the last real extent's values after a 0 return, e.g. to discard + * its preallocation. */ int udf_next_aext(struct inode *inode, struct extent_position *epos, struct kernel_lb_addr *eloc, uint32_t *elen, int8_t *etype, int inc) { + struct kernel_lb_addr tloc; + uint32_t tlen; + int8_t ttype; unsigned int indirections = 0; int ret = 0; udf_pblk_t block; while (1) { - ret = udf_current_aext(inode, epos, eloc, elen, - etype, inc); + ret = udf_current_aext(inode, epos, &tloc, &tlen, &ttype, inc); if (ret <= 0) return ret; - if (*etype != (EXT_NEXT_EXTENT_ALLOCDESCS >> 30)) + if (ttype != (EXT_NEXT_EXTENT_ALLOCDESCS >> 30)) { + *eloc = tloc; + *elen = tlen; + *etype = ttype; return ret; + } if (++indirections > UDF_MAX_INDIR_EXTS) { udf_err(inode->i_sb, @@ -2290,7 +2303,7 @@ int udf_next_aext(struct inode *inode, struct extent_position *epos, return -EFSCORRUPTED; } - epos->block = *eloc; + epos->block = tloc; epos->offset = sizeof(struct allocExtDesc); brelse(epos->bh); block = udf_get_lb_pblock(inode->i_sb, &epos->block, 0); -- 2.55.0