affs_extent_file_ofs() grows OFS mmu_private during write_begin. On allocation failure the error path still set i_size and mmu_private to newsize, so a partially extended hole left a too-large i_size. Set both to the bytes actually extended, and release the previous data block held across the failed getzeroblk_ino(). Signed-off-by: Xixin Liu --- fs/affs/file.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fs/affs/file.c b/fs/affs/file.c index 144b17482d12..3265b6b6bf9f 100644 --- a/fs/affs/file.c +++ b/fs/affs/file.c @@ -569,6 +569,7 @@ affs_extent_file_ofs(struct inode *inode, u32 newsize) pr_debug("%s(%llu, %d)\n", __func__, inode->i_ino, newsize); bsize = AFFS_SB(sb)->s_data_blksize; bh = NULL; + prev_bh = NULL; size = AFFS_I(inode)->mmu_private; bidx = size / bsize; boff = size % bsize; @@ -625,7 +626,9 @@ affs_extent_file_ofs(struct inode *inode, u32 newsize) return 0; out: - inode->i_size = AFFS_I(inode)->mmu_private = newsize; + affs_brelse(prev_bh); + /* Keep i_size/mmu_private at how far we actually extended. */ + inode->i_size = AFFS_I(inode)->mmu_private = size; return PTR_ERR(bh); } @@ -662,9 +665,6 @@ static int affs_write_begin_ofs(const struct kiocb *iocb, pr_debug("%s(%llu, %llu, %llu)\n", __func__, inode->i_ino, pos, pos + len); if (pos > AFFS_I(inode)->mmu_private) { - /* XXX: this probably leaves a too-big i_size in case of - * failure. Should really be updating i_size at write_end time - */ err = affs_extent_file_ofs(inode, pos); if (err) return err; -- 2.43.0 affs_getzeroblk() and affs_getemptyblk() pass sb_getblk() to lock_buffer()/wait_on_buffer() without a NULL check, so allocation failure crashes in the helpers before callers can handle it. Return NULL when sb_getblk() fails. Signed-off-by: Xixin Liu --- fs/affs/affs.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/affs/affs.h b/fs/affs/affs.h index 44a3f69d275f..651d61b3213a 100644 --- a/fs/affs/affs.h +++ b/fs/affs/affs.h @@ -250,6 +250,8 @@ affs_getzeroblk(struct super_block *sb, int block) pr_debug("%s: %d\n", __func__, block); if (affs_validblock(sb, block)) { bh = sb_getblk(sb, block); + if (!bh) + return NULL; lock_buffer(bh); memset(bh->b_data, 0 , sb->s_blocksize); set_buffer_uptodate(bh); @@ -265,6 +267,8 @@ affs_getemptyblk(struct super_block *sb, int block) pr_debug("%s: %d\n", __func__, block); if (affs_validblock(sb, block)) { bh = sb_getblk(sb, block); + if (!bh) + return NULL; wait_on_buffer(bh); set_buffer_uptodate(bh); return bh; -- 2.43.0 affs_rename() removes the object from the old directory hash before inserting it under the new name. If insert_hash() fails, the object keeps the new name but is linked in neither directory. On failure, restore the original name from old_dentry and re-insert into the old directory. Still return the insert error; warn if rollback fails. Apply the same best-effort rollback to affs_xrename(). Signed-off-by: Xixin Liu --- fs/affs/namei.c | 93 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 71 insertions(+), 22 deletions(-) diff --git a/fs/affs/namei.c b/fs/affs/namei.c index c3c6532da4b0..08edfdaeefc3 100644 --- a/fs/affs/namei.c +++ b/fs/affs/namei.c @@ -401,6 +401,38 @@ affs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *dentry) return affs_add_entry(dir, inode, dentry, ST_LINKFILE); } +/* Write dentry name into header and insert into dir (success or rollback). */ +static int affs_reinsert_with_name(struct inode *dir, struct buffer_head *bh, + struct dentry *dentry) +{ + struct super_block *sb = dir->i_sb; + int err; + + affs_copy_name(AFFS_TAIL(sb, bh)->name, dentry); + affs_fix_checksum(sb, bh); + affs_lock_dir(dir); + err = affs_insert_hash(dir, bh); + affs_unlock_dir(dir); + return err; +} + +/* Put both exchange objects back under their original names/dirs. */ +static void affs_xrename_restore(struct inode *old_dir, struct inode *new_dir, + struct buffer_head *bh_old, + struct buffer_head *bh_new, + struct dentry *old_dentry, + struct dentry *new_dentry) +{ + if (affs_reinsert_with_name(old_dir, bh_old, old_dentry)) + affs_warning(old_dir->i_sb, "xrename", + "failed to restore inode %lu", + (unsigned long)bh_old->b_blocknr); + if (affs_reinsert_with_name(new_dir, bh_new, new_dentry)) + affs_warning(new_dir->i_sb, "xrename", + "failed to restore inode %lu", + (unsigned long)bh_new->b_blocknr); +} + static int affs_rename(struct inode *old_dir, struct dentry *old_dentry, struct inode *new_dir, struct dentry *new_dentry) @@ -434,13 +466,15 @@ affs_rename(struct inode *old_dir, struct dentry *old_dentry, if (retval) goto done; - /* And insert it into the new directory with the new name. */ - affs_copy_name(AFFS_TAIL(sb, bh)->name, new_dentry); - affs_fix_checksum(sb, bh); - affs_lock_dir(new_dir); - retval = affs_insert_hash(new_dir, bh); - affs_unlock_dir(new_dir); - /* TODO: move it back to old_dir, if error? */ + /* Insert into the new directory with the new name. */ + retval = affs_reinsert_with_name(new_dir, bh, new_dentry); + if (retval) { + /* old_dentry still has the original name; put the object back. */ + if (affs_reinsert_with_name(old_dir, bh, old_dentry)) + affs_warning(sb, "rename", + "failed to restore inode %lu", + (unsigned long)bh->b_blocknr); + } done: mmb_mark_buffer_dirty(bh, @@ -453,11 +487,10 @@ static int affs_xrename(struct inode *old_dir, struct dentry *old_dentry, struct inode *new_dir, struct dentry *new_dentry) { - struct super_block *sb = old_dir->i_sb; struct buffer_head *bh_old = NULL; struct buffer_head *bh_new = NULL; - int retval; + int retval, retval2; bh_old = affs_bread(sb, d_inode(old_dentry)->i_ino); if (!bh_old) @@ -480,25 +513,41 @@ affs_xrename(struct inode *old_dir, struct dentry *old_dentry, affs_lock_dir(new_dir); retval = affs_remove_hash(new_dir, bh_new); affs_unlock_dir(new_dir); - if (retval) + if (retval) { + /* Names unchanged; only put old back into old_dir. */ + if (affs_reinsert_with_name(old_dir, bh_old, old_dentry)) + affs_warning(sb, "xrename", + "failed to restore inode %lu", + (unsigned long)bh_old->b_blocknr); goto done; + } /* Insert old into the new directory with the new name. */ - affs_copy_name(AFFS_TAIL(sb, bh_old)->name, new_dentry); - affs_fix_checksum(sb, bh_old); - affs_lock_dir(new_dir); - retval = affs_insert_hash(new_dir, bh_old); - affs_unlock_dir(new_dir); + retval = affs_reinsert_with_name(new_dir, bh_old, new_dentry); + if (retval) { + affs_xrename_restore(old_dir, new_dir, bh_old, bh_new, + old_dentry, new_dentry); + goto done; + } /* Insert new into the old directory with the old name. */ - affs_copy_name(AFFS_TAIL(sb, bh_new)->name, old_dentry); - affs_fix_checksum(sb, bh_new); - affs_lock_dir(old_dir); - retval = affs_insert_hash(old_dir, bh_new); - affs_unlock_dir(old_dir); + retval = affs_reinsert_with_name(old_dir, bh_new, old_dentry); + if (retval) { + /* Undo the first insert, then restore both originals. */ + affs_lock_dir(new_dir); + retval2 = affs_remove_hash(new_dir, bh_old); + affs_unlock_dir(new_dir); + if (retval2) + affs_warning(sb, "xrename", + "undo insert of inode %lu failed (%d)", + (unsigned long)bh_old->b_blocknr, retval2); + affs_xrename_restore(old_dir, new_dir, bh_old, bh_new, + old_dentry, new_dentry); + } + done: - mmb_mark_buffer_dirty(bh_old, &AFFS_I(new_dir)->i_metadata_bhs); - mmb_mark_buffer_dirty(bh_new, &AFFS_I(old_dir)->i_metadata_bhs); + mmb_mark_buffer_dirty(bh_old, &AFFS_I(retval ? old_dir : new_dir)->i_metadata_bhs); + mmb_mark_buffer_dirty(bh_new, &AFFS_I(retval ? new_dir : old_dir)->i_metadata_bhs); affs_brelse(bh_old); affs_brelse(bh_new); return retval; -- 2.43.0