Hi Jaegeuk, Yunlei,
On 2016/4/26 8:07, Jaegeuk Kim wrote:
> Let's consider a race condition between f2fs_add_regular_entry and
> find_target_dentry.
>
> 1.
> - f2fs_add_regular_entry updated len: 24 first.
> |
> Bits: 0 0 0 1
> Lens: 24 0 0 3 (name: foo)
> |->
> - find_target_dentry checks the first bit to find "foo", then ++pointer.
>
> 2.
> - f2fs_add_regular_entry updates bits.
> |>|>|
> Bits: 1 1 1 1
> Lens: 24 0 0 3 (name: foo)
> |
> - find_target_dentry is checking second bit, but it's len is zero, which
> makes the process being terminated.
As Pengyang reminded, there are no racing condition between find_target_dentry
and f2fs_add_regular_entry since i_mutex lock make each of operations being
atomical. So seems above condition can not happen.
But still we should handle dirent with zero-sized length correctly, as it may
cause deadloop. So how do you think of following patch?
From: Chao Yu <yuch...@huawei.com>
Subject: [PATCH] f2fs: be aware of invalid filename length
The filename length in dirent of may become zero-sized after random junk
data injection, once encounter such dirent, find_target_dentry or
f2fs_add_inline_entries will run into an infinite loop. So let f2fs being
aware of that to avoid deadloop.
Signed-off-by: Chao Yu <yuch...@huawei.com>
---
fs/f2fs/dir.c | 14 +++++---------
fs/f2fs/inline.c | 14 ++++++--------
2 files changed, 11 insertions(+), 17 deletions(-)
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index e90380d..3b1c14e 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -101,11 +101,6 @@ static struct f2fs_dir_entry *find_in_block(struct page
*dentry_page,
else
kunmap(dentry_page);
- /*
- * For the most part, it should be a bug when name_len is zero.
- * We stop here for figuring out where the bugs has occurred.
- */
- f2fs_bug_on(F2FS_P_SB(dentry_page), d.max < 0);
return de;
}
@@ -130,6 +125,11 @@ struct f2fs_dir_entry *find_target_dentry(struct
fscrypt_name *fname,
de = &d->dentry[bit_pos];
+ if (unlikely(!de->name_len)) {
+ bit_pos++;
+ continue;
+ }
+
/* encrypted case */
de_name.name = d->filename[bit_pos];
de_name.len = le16_to_cpu(de->name_len);
@@ -147,10 +147,6 @@ struct f2fs_dir_entry *find_target_dentry(struct
fscrypt_name *fname,
*max_slots = max_len;
max_len = 0;
- /* remain bug on condition */
- if (unlikely(!de->name_len))
- d->max = -1;
-
bit_pos += GET_DENTRY_SLOTS(le16_to_cpu(de->name_len));
}
diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 7720565..e61084c 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -303,11 +303,6 @@ struct f2fs_dir_entry *find_in_inline_dir(struct inode
*dir,
else
f2fs_put_page(ipage, 0);
- /*
- * For the most part, it should be a bug when name_len is zero.
- * We stop here for figuring out where the bugs has occurred.
- */
- f2fs_bug_on(sbi, d.max < 0);
return de;
}
@@ -437,6 +432,12 @@ static int f2fs_add_inline_entries(struct inode *dir,
}
de = &d.dentry[bit_pos];
+
+ if (unlikely(!de->name_len)) {
+ bit_pos++;
+ continue;
+ }
+
new_name.name = d.filename[bit_pos];
new_name.len = de->name_len;
@@ -448,9 +449,6 @@ static int f2fs_add_inline_entries(struct inode *dir,
if (err)
goto punch_dentry_pages;
- if (unlikely(!de->name_len))
- d.max = -1;
-
bit_pos += GET_DENTRY_SLOTS(le16_to_cpu(de->name_len));
}
return 0;
--
2.7.2
------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
