* [PATCH v2] f2fs: fix to check inline_xattr_size boundary correctly
@ 2019-02-14 16:08 Chao Yu
2019-03-04 6:39 ` Chao Yu
0 siblings, 1 reply; 7+ messages in thread
From: Chao Yu @ 2019-02-14 16:08 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, Chao Yu
From: Chao Yu <yuchao0@huawei.com>
We use below condition to check inline_xattr_size boundary:
if (!F2FS_OPTION(sbi).inline_xattr_size ||
F2FS_OPTION(sbi).inline_xattr_size >=
DEF_ADDRS_PER_INODE -
F2FS_TOTAL_EXTRA_ATTR_SIZE -
DEF_INLINE_RESERVED_SIZE -
DEF_MIN_INLINE_SIZE)
There is there problems in that check:
- we should allow inline_xattr_size equaling to min size of inline
{data,dentry} area.
- F2FS_TOTAL_EXTRA_ATTR_SIZE and inline_xattr_size are based on
different size unit, previous one is 4 bytes, latter one is 1 bytes.
- DEF_MIN_INLINE_SIZE only indicate min size of inline data area,
however, we need to consider min size of inline dentry area as well,
minimal inline dentry should at least contain two entries: '.' and
'..', so that min inline_dentry size is 40 bytes.
.bitmap 1 * 1 = 1
.reserved 1 * 1 = 1
.dentry 11 * 2 = 22
.filename 8 * 2 = 16
total 40
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
v2:
- fix lower bound check, inline xattr size should be larger than
xattr_header's size at least.
fs/f2fs/f2fs.h | 1 -
fs/f2fs/super.c | 13 +++++++------
include/linux/f2fs_fs.h | 13 +++++++------
3 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 8c928cd72b61..83df26b427b1 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -459,7 +459,6 @@ struct f2fs_flush_device {
/* for inline stuff */
#define DEF_INLINE_RESERVED_SIZE 1
-#define DEF_MIN_INLINE_SIZE 1
static inline int get_extra_isize(struct inode *inode);
static inline int get_inline_xattr_addrs(struct inode *inode);
#define MAX_INLINE_DATA(inode) (sizeof(__le32) * \
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 83bbe7424fc1..be8be445c6ed 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -834,12 +834,13 @@ static int parse_options(struct super_block *sb, char *options)
"set with inline_xattr option");
return -EINVAL;
}
- if (!F2FS_OPTION(sbi).inline_xattr_size ||
- F2FS_OPTION(sbi).inline_xattr_size >=
- DEF_ADDRS_PER_INODE -
- F2FS_TOTAL_EXTRA_ATTR_SIZE -
- DEF_INLINE_RESERVED_SIZE -
- DEF_MIN_INLINE_SIZE) {
+ if (F2FS_OPTION(sbi).inline_xattr_size <
+ sizeof(struct f2fs_xattr_header) / sizeof(__le32) ||
+ F2FS_OPTION(sbi).inline_xattr_size >
+ DEF_ADDRS_PER_INODE -
+ F2FS_TOTAL_EXTRA_ATTR_SIZE / sizeof(__le32) -
+ DEF_INLINE_RESERVED_SIZE -
+ MIN_INLINE_DENTRY_SIZE / sizeof(__le32)) {
f2fs_msg(sb, KERN_ERR,
"inline xattr size is out of range");
return -EINVAL;
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index 8d57aaee8166..666db8eb71e0 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -490,12 +490,12 @@ typedef __le32 f2fs_hash_t;
/*
* space utilization of regular dentry and inline dentry (w/o extra reservation)
- * regular dentry inline dentry
- * bitmap 1 * 27 = 27 1 * 23 = 23
- * reserved 1 * 3 = 3 1 * 7 = 7
- * dentry 11 * 214 = 2354 11 * 182 = 2002
- * filename 8 * 214 = 1712 8 * 182 = 1456
- * total 4096 3488
+ * regular dentry inline dentry (def) inline dentry (min)
+ * bitmap 1 * 27 = 27 1 * 23 = 23 1 * 1 = 1
+ * reserved 1 * 3 = 3 1 * 7 = 7 1 * 1 = 1
+ * dentry 11 * 214 = 2354 11 * 182 = 2002 11 * 2 = 22
+ * filename 8 * 214 = 1712 8 * 182 = 1456 8 * 2 = 16
+ * total 4096 3488 40
*
* Note: there are more reserved space in inline dentry than in regular
* dentry, when converting inline dentry we should handle this carefully.
@@ -507,6 +507,7 @@ typedef __le32 f2fs_hash_t;
#define SIZE_OF_RESERVED (PAGE_SIZE - ((SIZE_OF_DIR_ENTRY + \
F2FS_SLOT_LEN) * \
NR_DENTRY_IN_BLOCK + SIZE_OF_DENTRY_BITMAP))
+#define MIN_INLINE_DENTRY_SIZE 40 /* just include '.' and '..' entries */
/* One directory entry slot representing F2FS_SLOT_LEN-sized file name */
struct f2fs_dir_entry {
--
2.18.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] f2fs: fix to check inline_xattr_size boundary correctly
2019-02-14 16:08 [PATCH v2] f2fs: fix to check inline_xattr_size boundary correctly Chao Yu
@ 2019-03-04 6:39 ` Chao Yu
2019-03-06 6:32 ` [f2fs-dev] " Chao Yu
2019-03-12 18:50 ` Jaegeuk Kim
0 siblings, 2 replies; 7+ messages in thread
From: Chao Yu @ 2019-03-04 6:39 UTC (permalink / raw)
To: Chao Yu, jaegeuk; +Cc: linux-f2fs-devel, linux-kernel
Hi Jaegeuk,
On 2019/2/15 0:08, Chao Yu wrote:
> ---
> v2:
> - fix lower bound check, inline xattr size should be larger than
> xattr_header's size at least.
...
> + if (F2FS_OPTION(sbi).inline_xattr_size <
> + sizeof(struct f2fs_xattr_header) / sizeof(__le32)
No sure we should set this low bound as above... now I guess original
non-zero check is enough.
How do you think of setting inline_xattr_size range as
(0, MAX_INLINE_XATTR_SIZE]?
Thanks,
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: fix to check inline_xattr_size boundary correctly
2019-03-04 6:39 ` Chao Yu
@ 2019-03-06 6:32 ` Chao Yu
2019-03-12 18:50 ` Jaegeuk Kim
1 sibling, 0 replies; 7+ messages in thread
From: Chao Yu @ 2019-03-06 6:32 UTC (permalink / raw)
To: Chao Yu, jaegeuk; +Cc: linux-kernel, linux-f2fs-devel
Ping,
On 2019/3/4 14:39, Chao Yu wrote:
> Hi Jaegeuk,
>
> On 2019/2/15 0:08, Chao Yu wrote:
>> ---
>> v2:
>> - fix lower bound check, inline xattr size should be larger than
>> xattr_header's size at least.
>
> ...
>
>> + if (F2FS_OPTION(sbi).inline_xattr_size <
>> + sizeof(struct f2fs_xattr_header) / sizeof(__le32)
>
> No sure we should set this low bound as above... now I guess original
> non-zero check is enough.
>
> How do you think of setting inline_xattr_size range as
> (0, MAX_INLINE_XATTR_SIZE]?
>
> Thanks,
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] f2fs: fix to check inline_xattr_size boundary correctly
2019-03-04 6:39 ` Chao Yu
2019-03-06 6:32 ` [f2fs-dev] " Chao Yu
@ 2019-03-12 18:50 ` Jaegeuk Kim
2019-03-13 1:23 ` Chao Yu
1 sibling, 1 reply; 7+ messages in thread
From: Jaegeuk Kim @ 2019-03-12 18:50 UTC (permalink / raw)
To: Chao Yu; +Cc: Chao Yu, linux-f2fs-devel, linux-kernel
On 03/04, Chao Yu wrote:
> Hi Jaegeuk,
>
> On 2019/2/15 0:08, Chao Yu wrote:
> > ---
> > v2:
> > - fix lower bound check, inline xattr size should be larger than
> > xattr_header's size at least.
>
> ...
>
> > + if (F2FS_OPTION(sbi).inline_xattr_size <
> > + sizeof(struct f2fs_xattr_header) / sizeof(__le32)
>
> No sure we should set this low bound as above... now I guess original
> non-zero check is enough.
>
> How do you think of setting inline_xattr_size range as
> (0, MAX_INLINE_XATTR_SIZE]?
How about this?
---
fs/f2fs/super.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 42eb5c86330a..96302a428fdc 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -821,6 +821,8 @@ static int parse_options(struct super_block *sb, char *options)
}
if (test_opt(sbi, INLINE_XATTR_SIZE)) {
+ int min_size, max_size;
+
if (!f2fs_sb_has_extra_attr(sbi) ||
!f2fs_sb_has_flexible_inline_xattr(sbi)) {
f2fs_msg(sb, KERN_ERR,
@@ -834,15 +836,18 @@ static int parse_options(struct super_block *sb, char *options)
"set with inline_xattr option");
return -EINVAL;
}
- if (F2FS_OPTION(sbi).inline_xattr_size <
- sizeof(struct f2fs_xattr_header) / sizeof(__le32) ||
- F2FS_OPTION(sbi).inline_xattr_size >
- DEF_ADDRS_PER_INODE -
+
+ min_size = sizeof(struct f2fs_xattr_header) / sizeof(__le32);
+ max_size = DEF_ADDRS_PER_INODE -
F2FS_TOTAL_EXTRA_ATTR_SIZE / sizeof(__le32) -
DEF_INLINE_RESERVED_SIZE -
- MIN_INLINE_DENTRY_SIZE / sizeof(__le32)) {
+ MIN_INLINE_DENTRY_SIZE / sizeof(__le32);
+
+ if (F2FS_OPTION(sbi).inline_xattr_size < min ||
+ F2FS_OPTION(sbi).inline_xattr_size > max) {
f2fs_msg(sb, KERN_ERR,
- "inline xattr size is out of range");
+ "inline xattr size is out of range: %d ~ %d",
+ min, max);
return -EINVAL;
}
}
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] f2fs: fix to check inline_xattr_size boundary correctly
2019-03-12 18:50 ` Jaegeuk Kim
@ 2019-03-13 1:23 ` Chao Yu
2019-03-13 2:03 ` Jaegeuk Kim
0 siblings, 1 reply; 7+ messages in thread
From: Chao Yu @ 2019-03-13 1:23 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: Chao Yu, linux-f2fs-devel, linux-kernel
On 2019/3/13 2:50, Jaegeuk Kim wrote:
> On 03/04, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> On 2019/2/15 0:08, Chao Yu wrote:
>>> ---
>>> v2:
>>> - fix lower bound check, inline xattr size should be larger than
>>> xattr_header's size at least.
>>
>> ...
>>
>>> + if (F2FS_OPTION(sbi).inline_xattr_size <
>>> + sizeof(struct f2fs_xattr_header) / sizeof(__le32)
>>
>> No sure we should set this low bound as above... now I guess original
>> non-zero check is enough.
>>
>> How do you think of setting inline_xattr_size range as
>> (0, MAX_INLINE_XATTR_SIZE]?
>
> How about this?
If you think it's necessary to check low bound with size of xattr header,
I'm also okay with that.
And below diff looks good to me, could you please merge this into original one?
Thanks,
>
> ---
> fs/f2fs/super.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 42eb5c86330a..96302a428fdc 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -821,6 +821,8 @@ static int parse_options(struct super_block *sb, char *options)
> }
>
> if (test_opt(sbi, INLINE_XATTR_SIZE)) {
> + int min_size, max_size;
> +
> if (!f2fs_sb_has_extra_attr(sbi) ||
> !f2fs_sb_has_flexible_inline_xattr(sbi)) {
> f2fs_msg(sb, KERN_ERR,
> @@ -834,15 +836,18 @@ static int parse_options(struct super_block *sb, char *options)
> "set with inline_xattr option");
> return -EINVAL;
> }
> - if (F2FS_OPTION(sbi).inline_xattr_size <
> - sizeof(struct f2fs_xattr_header) / sizeof(__le32) ||
> - F2FS_OPTION(sbi).inline_xattr_size >
> - DEF_ADDRS_PER_INODE -
> +
> + min_size = sizeof(struct f2fs_xattr_header) / sizeof(__le32);
> + max_size = DEF_ADDRS_PER_INODE -
> F2FS_TOTAL_EXTRA_ATTR_SIZE / sizeof(__le32) -
> DEF_INLINE_RESERVED_SIZE -
> - MIN_INLINE_DENTRY_SIZE / sizeof(__le32)) {
> + MIN_INLINE_DENTRY_SIZE / sizeof(__le32);
> +
> + if (F2FS_OPTION(sbi).inline_xattr_size < min ||
> + F2FS_OPTION(sbi).inline_xattr_size > max) {
> f2fs_msg(sb, KERN_ERR,
> - "inline xattr size is out of range");
> + "inline xattr size is out of range: %d ~ %d",
> + min, max);
> return -EINVAL;
> }
> }
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] f2fs: fix to check inline_xattr_size boundary correctly
2019-03-13 1:23 ` Chao Yu
@ 2019-03-13 2:03 ` Jaegeuk Kim
2019-03-13 2:14 ` Chao Yu
0 siblings, 1 reply; 7+ messages in thread
From: Jaegeuk Kim @ 2019-03-13 2:03 UTC (permalink / raw)
To: Chao Yu; +Cc: Chao Yu, linux-f2fs-devel, linux-kernel
On 03/13, Chao Yu wrote:
> On 2019/3/13 2:50, Jaegeuk Kim wrote:
> > On 03/04, Chao Yu wrote:
> >> Hi Jaegeuk,
> >>
> >> On 2019/2/15 0:08, Chao Yu wrote:
> >>> ---
> >>> v2:
> >>> - fix lower bound check, inline xattr size should be larger than
> >>> xattr_header's size at least.
> >>
> >> ...
> >>
> >>> + if (F2FS_OPTION(sbi).inline_xattr_size <
> >>> + sizeof(struct f2fs_xattr_header) / sizeof(__le32)
> >>
> >> No sure we should set this low bound as above... now I guess original
> >> non-zero check is enough.
> >>
> >> How do you think of setting inline_xattr_size range as
> >> (0, MAX_INLINE_XATTR_SIZE]?
> >
> > How about this?
>
> If you think it's necessary to check low bound with size of xattr header,
> I'm also okay with that.
>
> And below diff looks good to me, could you please merge this into original one?
>
> Thanks,
I had to add this on top of branch.
From 70db5b04cbe19e5ae7e85ada2d3e82bcfdf90352 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Tue, 12 Mar 2019 11:49:53 -0700
Subject: [PATCH] f2fs: give some messages for inline_xattr_size
This patch adds some kernel messages when user sets wrong inline_xattr_size.
Fixes: 500e0b28ecd3 ("f2fs: fix to check inline_xattr_size boundary correctly")
Signed-off-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/super.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 42eb5c86330a..324938ec95f3 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -821,6 +821,8 @@ static int parse_options(struct super_block *sb, char *options)
}
if (test_opt(sbi, INLINE_XATTR_SIZE)) {
+ int min_size, max_size;
+
if (!f2fs_sb_has_extra_attr(sbi) ||
!f2fs_sb_has_flexible_inline_xattr(sbi)) {
f2fs_msg(sb, KERN_ERR,
@@ -834,15 +836,18 @@ static int parse_options(struct super_block *sb, char *options)
"set with inline_xattr option");
return -EINVAL;
}
- if (F2FS_OPTION(sbi).inline_xattr_size <
- sizeof(struct f2fs_xattr_header) / sizeof(__le32) ||
- F2FS_OPTION(sbi).inline_xattr_size >
- DEF_ADDRS_PER_INODE -
+
+ min_size = sizeof(struct f2fs_xattr_header) / sizeof(__le32);
+ max_size = DEF_ADDRS_PER_INODE -
F2FS_TOTAL_EXTRA_ATTR_SIZE / sizeof(__le32) -
DEF_INLINE_RESERVED_SIZE -
- MIN_INLINE_DENTRY_SIZE / sizeof(__le32)) {
+ MIN_INLINE_DENTRY_SIZE / sizeof(__le32);
+
+ if (F2FS_OPTION(sbi).inline_xattr_size < min_size ||
+ F2FS_OPTION(sbi).inline_xattr_size > max_size) {
f2fs_msg(sb, KERN_ERR,
- "inline xattr size is out of range");
+ "inline xattr size is out of range: %d ~ %d",
+ min_size, max_size);
return -EINVAL;
}
}
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] f2fs: fix to check inline_xattr_size boundary correctly
2019-03-13 2:03 ` Jaegeuk Kim
@ 2019-03-13 2:14 ` Chao Yu
0 siblings, 0 replies; 7+ messages in thread
From: Chao Yu @ 2019-03-13 2:14 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: Chao Yu, linux-f2fs-devel, linux-kernel
On 2019/3/13 10:03, Jaegeuk Kim wrote:
> On 03/13, Chao Yu wrote:
>> On 2019/3/13 2:50, Jaegeuk Kim wrote:
>>> On 03/04, Chao Yu wrote:
>>>> Hi Jaegeuk,
>>>>
>>>> On 2019/2/15 0:08, Chao Yu wrote:
>>>>> ---
>>>>> v2:
>>>>> - fix lower bound check, inline xattr size should be larger than
>>>>> xattr_header's size at least.
>>>>
>>>> ...
>>>>
>>>>> + if (F2FS_OPTION(sbi).inline_xattr_size <
>>>>> + sizeof(struct f2fs_xattr_header) / sizeof(__le32)
>>>>
>>>> No sure we should set this low bound as above... now I guess original
>>>> non-zero check is enough.
>>>>
>>>> How do you think of setting inline_xattr_size range as
>>>> (0, MAX_INLINE_XATTR_SIZE]?
>>>
>>> How about this?
>>
>> If you think it's necessary to check low bound with size of xattr header,
>> I'm also okay with that.
>>
>> And below diff looks good to me, could you please merge this into original one?
>>
>> Thanks,
>
> I had to add this on top of branch.
No problem.
>
>>From 70db5b04cbe19e5ae7e85ada2d3e82bcfdf90352 Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim <jaegeuk@kernel.org>
> Date: Tue, 12 Mar 2019 11:49:53 -0700
> Subject: [PATCH] f2fs: give some messages for inline_xattr_size
>
> This patch adds some kernel messages when user sets wrong inline_xattr_size.
>
> Fixes: 500e0b28ecd3 ("f2fs: fix to check inline_xattr_size boundary correctly")
But this commit id may change when Linus pull your merge request...
Thanks,
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
> fs/f2fs/super.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 42eb5c86330a..324938ec95f3 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -821,6 +821,8 @@ static int parse_options(struct super_block *sb, char *options)
> }
>
> if (test_opt(sbi, INLINE_XATTR_SIZE)) {
> + int min_size, max_size;
> +
> if (!f2fs_sb_has_extra_attr(sbi) ||
> !f2fs_sb_has_flexible_inline_xattr(sbi)) {
> f2fs_msg(sb, KERN_ERR,
> @@ -834,15 +836,18 @@ static int parse_options(struct super_block *sb, char *options)
> "set with inline_xattr option");
> return -EINVAL;
> }
> - if (F2FS_OPTION(sbi).inline_xattr_size <
> - sizeof(struct f2fs_xattr_header) / sizeof(__le32) ||
> - F2FS_OPTION(sbi).inline_xattr_size >
> - DEF_ADDRS_PER_INODE -
> +
> + min_size = sizeof(struct f2fs_xattr_header) / sizeof(__le32);
> + max_size = DEF_ADDRS_PER_INODE -
> F2FS_TOTAL_EXTRA_ATTR_SIZE / sizeof(__le32) -
> DEF_INLINE_RESERVED_SIZE -
> - MIN_INLINE_DENTRY_SIZE / sizeof(__le32)) {
> + MIN_INLINE_DENTRY_SIZE / sizeof(__le32);
> +
> + if (F2FS_OPTION(sbi).inline_xattr_size < min_size ||
> + F2FS_OPTION(sbi).inline_xattr_size > max_size) {
> f2fs_msg(sb, KERN_ERR,
> - "inline xattr size is out of range");
> + "inline xattr size is out of range: %d ~ %d",
> + min_size, max_size);
> return -EINVAL;
> }
> }
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-03-13 2:14 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-14 16:08 [PATCH v2] f2fs: fix to check inline_xattr_size boundary correctly Chao Yu
2019-03-04 6:39 ` Chao Yu
2019-03-06 6:32 ` [f2fs-dev] " Chao Yu
2019-03-12 18:50 ` Jaegeuk Kim
2019-03-13 1:23 ` Chao Yu
2019-03-13 2:03 ` Jaegeuk Kim
2019-03-13 2:14 ` Chao Yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).