2023-01-16 08:56:38

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH] ext2: propagate errors from ext2_prepare_chunk

Propagate errors from ext2_prepare_chunk to the callers and handle them
there. While touching the prototype also turn update_times into a bool
from the current int used as bool.

Signed-off-by: Christoph Hellwig <[email protected]>
---
fs/ext2/dir.c | 13 ++++++++-----
fs/ext2/ext2.h | 5 +++--
fs/ext2/namei.c | 21 ++++++++++++---------
3 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index e5cbc27ba4595b..947527929c2672 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -461,9 +461,9 @@ static int ext2_handle_dirsync(struct inode *dir)
return err;
}

-void ext2_set_link(struct inode *dir, struct ext2_dir_entry_2 *de,
- struct page *page, void *page_addr, struct inode *inode,
- int update_times)
+int ext2_set_link(struct inode *dir, struct ext2_dir_entry_2 *de,
+ struct page *page, void *page_addr, struct inode *inode,
+ bool update_times)
{
loff_t pos = page_offset(page) +
(char *) de - (char *) page_addr;
@@ -472,7 +472,10 @@ void ext2_set_link(struct inode *dir, struct ext2_dir_entry_2 *de,

lock_page(page);
err = ext2_prepare_chunk(page, pos, len);
- BUG_ON(err);
+ if (err) {
+ unlock_page(page);
+ return err;
+ }
de->inode = cpu_to_le32(inode->i_ino);
ext2_set_de_type(de, inode);
ext2_commit_chunk(page, pos, len);
@@ -480,7 +483,7 @@ void ext2_set_link(struct inode *dir, struct ext2_dir_entry_2 *de,
dir->i_mtime = dir->i_ctime = current_time(dir);
EXT2_I(dir)->i_flags &= ~EXT2_BTREE_FL;
mark_inode_dirty(dir);
- ext2_handle_dirsync(dir);
+ return ext2_handle_dirsync(dir);
}

/*
diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 28de11a22e5f6c..f7f590aac55a00 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -734,8 +734,9 @@ extern int ext2_delete_entry(struct ext2_dir_entry_2 *dir, struct page *page,
char *kaddr);
extern int ext2_empty_dir (struct inode *);
extern struct ext2_dir_entry_2 *ext2_dotdot(struct inode *dir, struct page **p, void **pa);
-extern void ext2_set_link(struct inode *, struct ext2_dir_entry_2 *, struct page *, void *,
- struct inode *, int);
+int ext2_set_link(struct inode *dir, struct ext2_dir_entry_2 *de,
+ struct page *page, void *page_addr, struct inode *inode,
+ bool update_times);
static inline void ext2_put_page(struct page *page, void *page_addr)
{
kunmap_local(page_addr);
diff --git a/fs/ext2/namei.c b/fs/ext2/namei.c
index c056957221a225..18b3d5af77240b 100644
--- a/fs/ext2/namei.c
+++ b/fs/ext2/namei.c
@@ -370,7 +370,10 @@ static int ext2_rename (struct user_namespace * mnt_userns,
err = PTR_ERR(new_de);
goto out_dir;
}
- ext2_set_link(new_dir, new_de, new_page, page_addr, old_inode, 1);
+ err = ext2_set_link(new_dir, new_de, new_page, page_addr,
+ old_inode, true);
+ if (err)
+ goto out_dir;
ext2_put_page(new_page, page_addr);
new_inode->i_ctime = current_time(new_inode);
if (dir_de)
@@ -394,24 +397,24 @@ static int ext2_rename (struct user_namespace * mnt_userns,
ext2_delete_entry(old_de, old_page, old_page_addr);

if (dir_de) {
- if (old_dir != new_dir)
- ext2_set_link(old_inode, dir_de, dir_page,
- dir_page_addr, new_dir, 0);
+ if (old_dir != new_dir) {
+ err = ext2_set_link(old_inode, dir_de, dir_page,
+ dir_page_addr, new_dir, false);

+ }
ext2_put_page(dir_page, dir_page_addr);
inode_dec_link_count(old_dir);
}

+out_old:
ext2_put_page(old_page, old_page_addr);
- return 0;
+out:
+ return err;

out_dir:
if (dir_de)
ext2_put_page(dir_page, dir_page_addr);
-out_old:
- ext2_put_page(old_page, old_page_addr);
-out:
- return err;
+ goto out_old;
}

const struct inode_operations ext2_dir_inode_operations = {
--
2.39.0


2023-01-16 11:20:04

by Jan Kara

[permalink] [raw]
Subject: Re: [PATCH] ext2: propagate errors from ext2_prepare_chunk

On Mon 16-01-23 09:52:05, Christoph Hellwig wrote:
> Propagate errors from ext2_prepare_chunk to the callers and handle them
> there. While touching the prototype also turn update_times into a bool
> from the current int used as bool.
>
> Signed-off-by: Christoph Hellwig <[email protected]>


> diff --git a/fs/ext2/namei.c b/fs/ext2/namei.c
> index c056957221a225..18b3d5af77240b 100644
> --- a/fs/ext2/namei.c
> +++ b/fs/ext2/namei.c
> @@ -370,7 +370,10 @@ static int ext2_rename (struct user_namespace * mnt_userns,
> err = PTR_ERR(new_de);
> goto out_dir;
> }
> - ext2_set_link(new_dir, new_de, new_page, page_addr, old_inode, 1);
> + err = ext2_set_link(new_dir, new_de, new_page, page_addr,
> + old_inode, true);
> + if (err)
> + goto out_dir;
> ext2_put_page(new_page, page_addr);

AFAICT we need to call ext2_put_page(new_page, page_addr) also in case of
error here. I'll fix it up on commit. Thanks for the patch.

Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR

2023-01-16 16:12:29

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] ext2: propagate errors from ext2_prepare_chunk

On Mon, Jan 16, 2023 at 12:16:21PM +0100, Jan Kara wrote:
> > - ext2_set_link(new_dir, new_de, new_page, page_addr, old_inode, 1);
> > + err = ext2_set_link(new_dir, new_de, new_page, page_addr,
> > + old_inode, true);
> > + if (err)
> > + goto out_dir;
> > ext2_put_page(new_page, page_addr);
>
> AFAICT we need to call ext2_put_page(new_page, page_addr) also in case of
> error here. I'll fix it up on commit. Thanks for the patch.

Indeed. I was tricked by out_dir doing the cleanup, but that's for
the old entry.