2020-06-03 06:37:00

by Zhang Yi

Subject: [PATCH v2 2/2] ext2: ext2_find_entry() return -ENOENT if no entry found

Almost all callers of ext2_find_entry() transform NULL return value to
-ENOENT, so just let ext2_find_entry() retuen -ENOENT instead of NULL
if no valid entry found, and also switch to check the return value of
ext2_inode_by_name() in ext2_lookup() and ext2_get_parent().

Signed-off-by: zhangyi (F) <[email protected]>
Suggested-by: Jan Kara <[email protected]>
---
fs/ext2/dir.c | 11 ++++++++---
fs/ext2/namei.c | 27 ++++++++-------------------
2 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/fs/ext2/dir.c b/fs/ext2/dir.c
index 95e4f0bd55a3..96c40fd7550e 100644
--- a/fs/ext2/dir.c
+++ b/fs/ext2/dir.c
@@ -393,7 +393,7 @@ struct ext2_dir_entry_2 *ext2_find_entry (struct inode *dir,
}
} while (n != start);
out:
- return NULL;
+ return ERR_PTR(-ENOENT);

found:
*res_page = page;
@@ -419,11 +419,16 @@ int ext2_inode_by_name(struct inode *dir, const struct qstr *child, ino_t *ino)
struct page *page;

de = ext2_find_entry(dir, child, &page);
- if (IS_ERR_OR_NULL(de))
+ if (IS_ERR(de))
return PTR_ERR(de);

- *ino = le32_to_cpu(de->inode);
ext2_put_page(page);
+ if (!de->inode) {
+ ext2_error(dir->i_sb, __func__, "bad inode number: %u",
+ le32_to_cpu(de->inode));
+ return -EFSCORRUPTED;
+ }
+ *ino = le32_to_cpu(de->inode);
return 0;
}

diff --git a/fs/ext2/namei.c b/fs/ext2/namei.c
index 4b38e558d477..9601d469c5b1 100644
--- a/fs/ext2/namei.c
+++ b/fs/ext2/namei.c
@@ -56,17 +56,18 @@ static inline int ext2_add_nondir(struct dentry *dentry, struct inode *inode)
static struct dentry *ext2_lookup(struct inode * dir, struct dentry *dentry, unsigned int flags)
{
struct inode * inode;
- ino_t ino = 0;
+ ino_t ino;
int res;

if (dentry->d_name.len > EXT2_NAME_LEN)
return ERR_PTR(-ENAMETOOLONG);

res = ext2_inode_by_name(dir, &dentry->d_name, &ino);
- if (res)
- return ERR_PTR(res);
- inode = NULL;
- if (ino) {
+ if (res) {
+ if (res != -ENOENT)
+ return ERR_PTR(res);
+ inode = NULL;
+ } else {
inode = ext2_iget(dir->i_sb, ino);
if (inode == ERR_PTR(-ESTALE)) {
ext2_error(dir->i_sb, __func__,
@@ -81,14 +82,13 @@ static struct dentry *ext2_lookup(struct inode * dir, struct dentry *dentry, uns
struct dentry *ext2_get_parent(struct dentry *child)
{
struct qstr dotdot = QSTR_INIT("..", 2);
- ino_t ino = 0;
+ ino_t ino;
int res;

res = ext2_inode_by_name(d_inode(child), &dotdot, &ino);
if (res)
return ERR_PTR(res);
- if (!ino)
- return ERR_PTR(-ENOENT);
+
return d_obtain_alias(ext2_iget(child->d_sb, ino));
}

@@ -289,10 +289,6 @@ static int ext2_unlink(struct inode * dir, struct dentry *dentry)
err = PTR_ERR(de);
goto out;
}
- if (!de) {
- err = -ENOENT;
- goto out;
- }

err = ext2_delete_entry (de, page);
if (err)
@@ -349,10 +345,6 @@ static int ext2_rename (struct inode * old_dir, struct dentry * old_dentry,
err = PTR_ERR(old_de);
goto out;
}
- if (!old_de) {
- err = -ENOENT;
- goto out;
- }

if (S_ISDIR(old_inode->i_mode)) {
err = -EIO;
@@ -369,14 +361,11 @@ static int ext2_rename (struct inode * old_dir, struct dentry * old_dentry,
if (dir_de && !ext2_empty_dir (new_inode))
goto out_dir;

- err = -ENOENT;
new_de = ext2_find_entry(new_dir, &new_dentry->d_name, &new_page);
if (IS_ERR(new_de)) {
err = PTR_ERR(new_de);
goto out_dir;
}
- if (!new_de)
- goto out_dir;
ext2_set_link(new_dir, new_de, new_page, old_inode, 1);
new_inode->i_ctime = current_time(new_inode);
if (dir_de)
--
2.25.4

2020-06-05 15:11:36

by Jan Kara

Subject: Re: [PATCH v2 2/2] ext2: ext2_find_entry() return -ENOENT if no entry found

On Wed 03-06-20 14:35:14, zhangyi (F) wrote:
> Almost all callers of ext2_find_entry() transform NULL return value to
> -ENOENT, so just let ext2_find_entry() retuen -ENOENT instead of NULL
> if no valid entry found, and also switch to check the return value of
> ext2_inode_by_name() in ext2_lookup() and ext2_get_parent().
>
> Signed-off-by: zhangyi (F) <[email protected]>
> Suggested-by: Jan Kara <[email protected]>

Thanks for the patch. Just one small nit below.

> @@ -419,11 +419,16 @@ int ext2_inode_by_name(struct inode *dir, const struct qstr *child, ino_t *ino)
> struct page *page;
>
> de = ext2_find_entry(dir, child, &page);
> - if (IS_ERR_OR_NULL(de))
> + if (IS_ERR(de))
> return PTR_ERR(de);
>
> - *ino = le32_to_cpu(de->inode);
> ext2_put_page(page);
> + if (!de->inode) {

ext2_find_entry() will not ever return de with de->inode == 0 because
ext2_match() never returns true for such entries. So I'd just remove this
condition...

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

2020-06-08 01:25:21

by Zhang Yi

Subject: Re: [PATCH v2 2/2] ext2: ext2_find_entry() return -ENOENT if no entry found

On 2020/6/5 23:11, Jan Kara wrote:
> On Wed 03-06-20 14:35:14, zhangyi (F) wrote:
>> Almost all callers of ext2_find_entry() transform NULL return value to
>> -ENOENT, so just let ext2_find_entry() retuen -ENOENT instead of NULL
>> if no valid entry found, and also switch to check the return value of
>> ext2_inode_by_name() in ext2_lookup() and ext2_get_parent().
>>
>> Signed-off-by: zhangyi (F) <[email protected]>
>> Suggested-by: Jan Kara <[email protected]>
>
> Thanks for the patch. Just one small nit below.
>
>> @@ -419,11 +419,16 @@ int ext2_inode_by_name(struct inode *dir, const struct qstr *child, ino_t *ino)
>> struct page *page;
>>
>> de = ext2_find_entry(dir, child, &page);
>> - if (IS_ERR_OR_NULL(de))
>> + if (IS_ERR(de))
>> return PTR_ERR(de);
>>
>> - *ino = le32_to_cpu(de->inode);
>> ext2_put_page(page);
>> + if (!de->inode) {
>
> ext2_find_entry() will not ever return de with de->inode == 0 because
> ext2_match() never returns true for such entries. So I'd just remove this
> condition...
>
Indeed, I missed this point, will do.

Thanks,
Yi.