| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20131002121928.GC25126@quack.suse.cz>
Date: Wed, 2 Oct 2013 14:19:28 +0200
From: Jan Kara <jack@...e.cz>
To: Miklos Szeredi <miklos@...redi.hu>
Cc: linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
viro@...IV.linux.org.uk, torvalds@...ux-foundation.org,
hch@...radead.org, akpm@...ux-foundation.org, dhowells@...hat.com,
zab@...hat.com, jack@...e.cz, tytso@....edu, mszeredi@...e.cz
Subject: Re: [PATCH 6/7] ext4: rename: split out helper functions
On Tue 01-10-13 18:00:38, Miklos Szeredi wrote:
> From: Miklos Szeredi <mszeredi@...e.cz>
>
> Cross rename (exchange source and dest) will need to call some of these
> helpers for both source and dest, while overwriting rename currently only
> calls them for one or the other. This also makes the code easier to
> follow.
The patch looks good to me. You can add:
Reviewed-by: Jan Kara <jack@...e.cz>
Honza
>
> Signed-off-by: Miklos Szeredi <mszeredi@...e.cz>
> ---
> fs/ext4/namei.c | 199 +++++++++++++++++++++++++++++++++++---------------------
> 1 file changed, 126 insertions(+), 73 deletions(-)
>
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 1348251..fb0f1db 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -3014,6 +3014,125 @@ struct ext4_renament {
> int parent_inlined;
> };
>
> +static int ext4_rename_dir_prepare(handle_t *handle, struct ext4_renament *ent)
> +{
> + int retval;
> +
> + ent->dir_bh = ext4_get_first_dir_block(handle, ent->inode,
> + &retval, &ent->parent_de,
> + &ent->parent_inlined);
> + if (!ent->dir_bh)
> + return retval;
> + if (le32_to_cpu(ent->parent_de->inode) != ent->dir->i_ino)
> + return -EIO;
> + BUFFER_TRACE(ent->dir_bh, "get_write_access");
> + return ext4_journal_get_write_access(handle, ent->dir_bh);
> +}
> +
> +static int ext4_rename_dir_finish(handle_t *handle, struct ext4_renament *ent,
> + unsigned dir_ino)
> +{
> + int retval;
> +
> + ent->parent_de->inode = cpu_to_le32(dir_ino);
> + BUFFER_TRACE(ent->dir_bh, "call ext4_handle_dirty_metadata");
> + if (!ent->parent_inlined) {
> + if (is_dx(ent->inode)) {
> + retval = ext4_handle_dirty_dx_node(handle,
> + ent->inode,
> + ent->dir_bh);
> + } else {
> + retval = ext4_handle_dirty_dirent_node(handle,
> + ent->inode,
> + ent->dir_bh);
> + }
> + } else {
> + retval = ext4_mark_inode_dirty(handle, ent->inode);
> + }
> + if (retval) {
> + ext4_std_error(ent->dir->i_sb, retval);
> + return retval;
> + }
> + return 0;
> +}
> +
> +static int ext4_setent(handle_t *handle, struct ext4_renament *ent,
> + unsigned ino, unsigned file_type)
> +{
> + int retval;
> +
> + BUFFER_TRACE(ent->bh, "get write access");
> + retval = ext4_journal_get_write_access(handle, ent->bh);
> + if (retval)
> + return retval;
> + ent->de->inode = cpu_to_le32(ino);
> + if (EXT4_HAS_INCOMPAT_FEATURE(ent->dir->i_sb,
> + EXT4_FEATURE_INCOMPAT_FILETYPE))
> + ent->de->file_type = file_type;
> + ent->dir->i_version++;
> + ent->dir->i_ctime = ent->dir->i_mtime =
> + ext4_current_time(ent->dir);
> + ext4_mark_inode_dirty(handle, ent->dir);
> + BUFFER_TRACE(ent->bh, "call ext4_handle_dirty_metadata");
> + if (!ent->inlined) {
> + retval = ext4_handle_dirty_dirent_node(handle,
> + ent->dir, ent->bh);
> + if (unlikely(retval)) {
> + ext4_std_error(ent->dir->i_sb, retval);
> + return retval;
> + }
> + }
> + brelse(ent->bh);
> + ent->bh = NULL;
> +
> + return 0;
> +}
> +
> +static int ext4_find_delete_entry(handle_t *handle, struct inode *dir,
> + const struct qstr *d_name)
> +{
> + int retval = -ENOENT;
> + struct buffer_head *bh;
> + struct ext4_dir_entry_2 *de;
> +
> + bh = ext4_find_entry(dir, d_name, &de, NULL);
> + if (bh) {
> + retval = ext4_delete_entry(handle, dir, de, bh);
> + brelse(bh);
> + }
> + return retval;
> +}
> +
> +static void ext4_rename_delete(handle_t *handle, struct ext4_renament *ent)
> +{
> + int retval;
> + /*
> + * ent->de could have moved from under us during htree split, so make
> + * sure that we are deleting the right entry. We might also be pointing
> + * to a stale entry in the unused part of ent->bh so just checking inum
> + * and the name isn't enough.
> + */
> + if (le32_to_cpu(ent->de->inode) != ent->inode->i_ino ||
> + ent->de->name_len != ent->dentry->d_name.len ||
> + strncmp(ent->de->name, ent->dentry->d_name.name,
> + ent->de->name_len)) {
> + retval = ext4_find_delete_entry(handle, ent->dir,
> + &ent->dentry->d_name);
> + } else {
> + retval = ext4_delete_entry(handle, ent->dir, ent->de, ent->bh);
> + if (retval == -ENOENT) {
> + retval = ext4_find_delete_entry(handle, ent->dir,
> + &ent->dentry->d_name);
> + }
> + }
> +
> + if (retval) {
> + ext4_warning(ent->dir->i_sb,
> + "Deleting old file (%lu), %d, error=%d",
> + ent->dir->i_ino, ent->dir->i_nlink, retval);
> + }
> +}
> +
> /*
> * Anybody can rename anything with this: the permission checks are left to the
> * higher-level routines.
> @@ -3087,16 +3206,7 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> if (new.dir != old.dir && EXT4_DIR_LINK_MAX(new.dir))
> goto end_rename;
> }
> - retval = -EIO;
> - old.dir_bh = ext4_get_first_dir_block(handle, old.inode,
> - &retval, &old.parent_de,
> - &old.parent_inlined);
> - if (!old.dir_bh)
> - goto end_rename;
> - if (le32_to_cpu(old.parent_de->inode) != old.dir->i_ino)
> - goto end_rename;
> - BUFFER_TRACE(old.dir_bh, "get_write_access");
> - retval = ext4_journal_get_write_access(handle, old.dir_bh);
> + retval = ext4_rename_dir_prepare(handle, &old);
> if (retval)
> goto end_rename;
> }
> @@ -3105,29 +3215,10 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> if (retval)
> goto end_rename;
> } else {
> - BUFFER_TRACE(new.bh, "get write access");
> - retval = ext4_journal_get_write_access(handle, new.bh);
> + retval = ext4_setent(handle, &new,
> + old.inode->i_ino, old.de->file_type);
> if (retval)
> goto end_rename;
> - new.de->inode = cpu_to_le32(old.inode->i_ino);
> - if (EXT4_HAS_INCOMPAT_FEATURE(new.dir->i_sb,
> - EXT4_FEATURE_INCOMPAT_FILETYPE))
> - new.de->file_type = old.de->file_type;
> - new.dir->i_version++;
> - new.dir->i_ctime = new.dir->i_mtime =
> - ext4_current_time(new.dir);
> - ext4_mark_inode_dirty(handle, new.dir);
> - BUFFER_TRACE(new.bh, "call ext4_handle_dirty_metadata");
> - if (!new.inlined) {
> - retval = ext4_handle_dirty_dirent_node(handle,
> - new.dir, new.bh);
> - if (unlikely(retval)) {
> - ext4_std_error(new.dir->i_sb, retval);
> - goto end_rename;
> - }
> - }
> - brelse(new.bh);
> - new.bh = NULL;
> }
>
> /*
> @@ -3140,31 +3231,7 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> /*
> * ok, that's it
> */
> - if (le32_to_cpu(old.de->inode) != old.inode->i_ino ||
> - old.de->name_len != old.dentry->d_name.len ||
> - strncmp(old.de->name, old.dentry->d_name.name, old.de->name_len) ||
> - (retval = ext4_delete_entry(handle, old.dir,
> - old.de, old.bh)) == -ENOENT) {
> - /* old.de could have moved from under us during htree split, so
> - * make sure that we are deleting the right entry. We might
> - * also be pointing to a stale entry in the unused part of
> - * old.bh so just checking inum and the name isn't enough. */
> - struct buffer_head *old_bh2;
> - struct ext4_dir_entry_2 *old_de2;
> -
> - old_bh2 = ext4_find_entry(old.dir, &old.dentry->d_name,
> - &old_de2, NULL);
> - if (old_bh2) {
> - retval = ext4_delete_entry(handle, old.dir,
> - old_de2, old_bh2);
> - brelse(old_bh2);
> - }
> - }
> - if (retval) {
> - ext4_warning(old.dir->i_sb,
> - "Deleting old file (%lu), %d, error=%d",
> - old.dir->i_ino, old.dir->i_nlink, retval);
> - }
> + ext4_rename_delete(handle, &old);
>
> if (new.inode) {
> ext4_dec_count(handle, new.inode);
> @@ -3173,24 +3240,10 @@ static int ext4_rename(struct inode *old_dir, struct dentry *old_dentry,
> old.dir->i_ctime = old.dir->i_mtime = ext4_current_time(old.dir);
> ext4_update_dx_flag(old.dir);
> if (old.dir_bh) {
> - old.parent_de->inode = cpu_to_le32(new.dir->i_ino);
> - BUFFER_TRACE(old.dir_bh, "call ext4_handle_dirty_metadata");
> - if (!old.parent_inlined) {
> - if (is_dx(old.inode)) {
> - retval = ext4_handle_dirty_dx_node(handle,
> - old.inode,
> - old.dir_bh);
> - } else {
> - retval = ext4_handle_dirty_dirent_node(handle,
> - old.inode, old.dir_bh);
> - }
> - } else {
> - retval = ext4_mark_inode_dirty(handle, old.inode);
> - }
> - if (retval) {
> - ext4_std_error(old.dir->i_sb, retval);
> + retval = ext4_rename_dir_finish(handle, &old, new.dir->i_ino);
> + if (retval)
> goto end_rename;
> - }
> +
> ext4_dec_count(handle, old.dir);
> if (new.inode) {
> /* checked empty_dir above, can't have another parent,
> --
> 1.8.1.4
>
--
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists