* [PATCH] f2fs: add sanity check for quota sysfile ino
@ 2018-01-29 8:29 Chao Yu
2018-01-31 1:35 ` Jaegeuk Kim
0 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2018-01-29 8:29 UTC (permalink / raw)
To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu
Add missing sanity check for quota sysfile ino.
Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
fs/f2fs/super.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 368f63d7bad2..6011071688ca 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -2116,6 +2116,25 @@ static int sanity_check_raw_super(struct f2fs_sb_info *sbi,
return 1;
}
+ /* check quota sysfile ino info */
+ if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
+ if (le32_to_cpu(raw_super->qf_ino[0]) != 4 ||
+ le32_to_cpu(raw_super->qf_ino[1]) != 5) {
+ f2fs_msg(sb, KERN_INFO,
+ "Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
+ le32_to_cpu(raw_super->qf_ino[0]),
+ le32_to_cpu(raw_super->qf_ino[1]));
+ return 1;
+ }
+ if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
+ le32_to_cpu(raw_super->qf_ino[2]) != 6) {
+ f2fs_msg(sb, KERN_INFO,
+ "Invalid Quota Ino: prj_ino(%u)",
+ le32_to_cpu(raw_super->qf_ino[2]));
+ return 1;
+ }
+ }
+
if (le32_to_cpu(raw_super->segment_count) > F2FS_MAX_SEGMENT) {
f2fs_msg(sb, KERN_INFO,
"Invalid segment count (%u)",
--
2.15.0.55.gc2ece9dc4de6
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: add sanity check for quota sysfile ino
2018-01-29 8:29 [PATCH] f2fs: add sanity check for quota sysfile ino Chao Yu
@ 2018-01-31 1:35 ` Jaegeuk Kim
2018-01-31 2:01 ` Chao Yu
0 siblings, 1 reply; 9+ messages in thread
From: Jaegeuk Kim @ 2018-01-31 1:35 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao
On 01/29, Chao Yu wrote:
> Add missing sanity check for quota sysfile ino.
We don't need to limit the specific inode numbers for quota files, since
we may be able to set any inode numbers later. How about checkint the
numbers are allocated as quota?
>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> fs/f2fs/super.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 368f63d7bad2..6011071688ca 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -2116,6 +2116,25 @@ static int sanity_check_raw_super(struct f2fs_sb_info *sbi,
> return 1;
> }
>
> + /* check quota sysfile ino info */
> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
> + if (le32_to_cpu(raw_super->qf_ino[0]) != 4 ||
> + le32_to_cpu(raw_super->qf_ino[1]) != 5) {
> + f2fs_msg(sb, KERN_INFO,
> + "Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
> + le32_to_cpu(raw_super->qf_ino[0]),
> + le32_to_cpu(raw_super->qf_ino[1]));
> + return 1;
> + }
> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
> + le32_to_cpu(raw_super->qf_ino[2]) != 6) {
> + f2fs_msg(sb, KERN_INFO,
> + "Invalid Quota Ino: prj_ino(%u)",
> + le32_to_cpu(raw_super->qf_ino[2]));
> + return 1;
> + }
> + }
> +
> if (le32_to_cpu(raw_super->segment_count) > F2FS_MAX_SEGMENT) {
> f2fs_msg(sb, KERN_INFO,
> "Invalid segment count (%u)",
> --
> 2.15.0.55.gc2ece9dc4de6
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: add sanity check for quota sysfile ino
2018-01-31 1:35 ` Jaegeuk Kim
@ 2018-01-31 2:01 ` Chao Yu
2018-01-31 2:36 ` Jaegeuk Kim
0 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2018-01-31 2:01 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao
On 2018/1/31 9:35, Jaegeuk Kim wrote:
> On 01/29, Chao Yu wrote:
>> Add missing sanity check for quota sysfile ino.
>
> We don't need to limit the specific inode numbers for quota files, since
> we may be able to set any inode numbers later. How about checkint the
> numbers are allocated as quota?
Do you mean:
if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
if (!le32_to_cpu(raw_super->qf_ino[0]) ||
!le32_to_cpu(raw_super->qf_ino[1])) {
f2fs_msg(sb, KERN_INFO,
"Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
le32_to_cpu(raw_super->qf_ino[0]),
le32_to_cpu(raw_super->qf_ino[1]));
return 1;
}
if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
!le32_to_cpu(raw_super->qf_ino[2])) {
f2fs_msg(sb, KERN_INFO,
"Invalid Quota Ino: prj_ino(%u)",
le32_to_cpu(raw_super->qf_ino[2]));
return 1;
}
}
I think it's equal to:
if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
unsigned int qf_cnt = 0;
if (qf_ino[0])
qf_cnt++;
if (qf_ino[1])
qf_cnt++;
if (qf_cnt != 2) {
f2fs_msg();
return 1;
}
if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA)) {
if (qf_ino[2])
qf_cnt++;
if (qf_cnt != 3) {
f2fs_msg();
return 1;
}
}
}
Thanks,
>
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>> fs/f2fs/super.c | 19 +++++++++++++++++++
>> 1 file changed, 19 insertions(+)
>>
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index 368f63d7bad2..6011071688ca 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -2116,6 +2116,25 @@ static int sanity_check_raw_super(struct f2fs_sb_info *sbi,
>> return 1;
>> }
>>
>> + /* check quota sysfile ino info */
>> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
>> + if (le32_to_cpu(raw_super->qf_ino[0]) != 4 ||
>> + le32_to_cpu(raw_super->qf_ino[1]) != 5) {
>> + f2fs_msg(sb, KERN_INFO,
>> + "Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
>> + le32_to_cpu(raw_super->qf_ino[0]),
>> + le32_to_cpu(raw_super->qf_ino[1]));
>> + return 1;
>> + }
>> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
>> + le32_to_cpu(raw_super->qf_ino[2]) != 6) {
>> + f2fs_msg(sb, KERN_INFO,
>> + "Invalid Quota Ino: prj_ino(%u)",
>> + le32_to_cpu(raw_super->qf_ino[2]));
>> + return 1;
>> + }
>> + }
>> +
>> if (le32_to_cpu(raw_super->segment_count) > F2FS_MAX_SEGMENT) {
>> f2fs_msg(sb, KERN_INFO,
>> "Invalid segment count (%u)",
>> --
>> 2.15.0.55.gc2ece9dc4de6
>
> .
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: add sanity check for quota sysfile ino
2018-01-31 2:01 ` Chao Yu
@ 2018-01-31 2:36 ` Jaegeuk Kim
2018-01-31 3:38 ` Chao Yu
0 siblings, 1 reply; 9+ messages in thread
From: Jaegeuk Kim @ 2018-01-31 2:36 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao
On 01/31, Chao Yu wrote:
> On 2018/1/31 9:35, Jaegeuk Kim wrote:
> > On 01/29, Chao Yu wrote:
> >> Add missing sanity check for quota sysfile ino.
> >
> > We don't need to limit the specific inode numbers for quota files, since
> > we may be able to set any inode numbers later. How about checkint the
> > numbers are allocated as quota?
If quota_ino is set, f2fs_enable_quotas() will be failed, if its ino is invalid.
It'd be also fine, if we have no inode numbers there. What I mean was we may need
to consider reallocating or deallocating inode numbers later.
>
> Do you mean:
>
> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
> if (!le32_to_cpu(raw_super->qf_ino[0]) ||
> !le32_to_cpu(raw_super->qf_ino[1])) {
> f2fs_msg(sb, KERN_INFO,
> "Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
> le32_to_cpu(raw_super->qf_ino[0]),
> le32_to_cpu(raw_super->qf_ino[1]));
> return 1;
> }
> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
> !le32_to_cpu(raw_super->qf_ino[2])) {
> f2fs_msg(sb, KERN_INFO,
> "Invalid Quota Ino: prj_ino(%u)",
> le32_to_cpu(raw_super->qf_ino[2]));
> return 1;
> }
> }
>
> I think it's equal to:
>
> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
> unsigned int qf_cnt = 0;
> if (qf_ino[0])
> qf_cnt++;
> if (qf_ino[1])
> qf_cnt++;
> if (qf_cnt != 2) {
> f2fs_msg();
> return 1;
> }
> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA)) {
> if (qf_ino[2])
> qf_cnt++;
> if (qf_cnt != 3) {
> f2fs_msg();
> return 1;
> }
> }
> }
>
> Thanks,
>
> >
> >>
> >> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> >> ---
> >> fs/f2fs/super.c | 19 +++++++++++++++++++
> >> 1 file changed, 19 insertions(+)
> >>
> >> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> >> index 368f63d7bad2..6011071688ca 100644
> >> --- a/fs/f2fs/super.c
> >> +++ b/fs/f2fs/super.c
> >> @@ -2116,6 +2116,25 @@ static int sanity_check_raw_super(struct f2fs_sb_info *sbi,
> >> return 1;
> >> }
> >>
> >> + /* check quota sysfile ino info */
> >> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
> >> + if (le32_to_cpu(raw_super->qf_ino[0]) != 4 ||
> >> + le32_to_cpu(raw_super->qf_ino[1]) != 5) {
> >> + f2fs_msg(sb, KERN_INFO,
> >> + "Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
> >> + le32_to_cpu(raw_super->qf_ino[0]),
> >> + le32_to_cpu(raw_super->qf_ino[1]));
> >> + return 1;
> >> + }
> >> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
> >> + le32_to_cpu(raw_super->qf_ino[2]) != 6) {
> >> + f2fs_msg(sb, KERN_INFO,
> >> + "Invalid Quota Ino: prj_ino(%u)",
> >> + le32_to_cpu(raw_super->qf_ino[2]));
> >> + return 1;
> >> + }
> >> + }
> >> +
> >> if (le32_to_cpu(raw_super->segment_count) > F2FS_MAX_SEGMENT) {
> >> f2fs_msg(sb, KERN_INFO,
> >> "Invalid segment count (%u)",
> >> --
> >> 2.15.0.55.gc2ece9dc4de6
> >
> > .
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: add sanity check for quota sysfile ino
2018-01-31 2:36 ` Jaegeuk Kim
@ 2018-01-31 3:38 ` Chao Yu
2018-01-31 3:49 ` Jaegeuk Kim
0 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2018-01-31 3:38 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao
On 2018/1/31 10:36, Jaegeuk Kim wrote:
> On 01/31, Chao Yu wrote:
>> On 2018/1/31 9:35, Jaegeuk Kim wrote:
>>> On 01/29, Chao Yu wrote:
>>>> Add missing sanity check for quota sysfile ino.
>>>
>>> We don't need to limit the specific inode numbers for quota files, since
>>> we may be able to set any inode numbers later. How about checkint the
>>> numbers are allocated as quota?
>
> If quota_ino is set, f2fs_enable_quotas() will be failed, if its ino is invalid.
> It'd be also fine, if we have no inode numbers there. What I mean was we may need
> to consider reallocating or deallocating inode numbers later.
Oh, so that it will be better to do more check & repair in fsck tools, right?
Thanks,
>
>>
>> Do you mean:
>>
>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
>> if (!le32_to_cpu(raw_super->qf_ino[0]) ||
>> !le32_to_cpu(raw_super->qf_ino[1])) {
>> f2fs_msg(sb, KERN_INFO,
>> "Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
>> le32_to_cpu(raw_super->qf_ino[0]),
>> le32_to_cpu(raw_super->qf_ino[1]));
>> return 1;
>> }
>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
>> !le32_to_cpu(raw_super->qf_ino[2])) {
>> f2fs_msg(sb, KERN_INFO,
>> "Invalid Quota Ino: prj_ino(%u)",
>> le32_to_cpu(raw_super->qf_ino[2]));
>> return 1;
>> }
>> }
>>
>> I think it's equal to:
>>
>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
>> unsigned int qf_cnt = 0;
>> if (qf_ino[0])
>> qf_cnt++;
>> if (qf_ino[1])
>> qf_cnt++;
>> if (qf_cnt != 2) {
>> f2fs_msg();
>> return 1;
>> }
>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA)) {
>> if (qf_ino[2])
>> qf_cnt++;
>> if (qf_cnt != 3) {
>> f2fs_msg();
>> return 1;
>> }
>> }
>> }
>>
>> Thanks,
>>
>>>
>>>>
>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>> ---
>>>> fs/f2fs/super.c | 19 +++++++++++++++++++
>>>> 1 file changed, 19 insertions(+)
>>>>
>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>> index 368f63d7bad2..6011071688ca 100644
>>>> --- a/fs/f2fs/super.c
>>>> +++ b/fs/f2fs/super.c
>>>> @@ -2116,6 +2116,25 @@ static int sanity_check_raw_super(struct f2fs_sb_info *sbi,
>>>> return 1;
>>>> }
>>>>
>>>> + /* check quota sysfile ino info */
>>>> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
>>>> + if (le32_to_cpu(raw_super->qf_ino[0]) != 4 ||
>>>> + le32_to_cpu(raw_super->qf_ino[1]) != 5) {
>>>> + f2fs_msg(sb, KERN_INFO,
>>>> + "Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
>>>> + le32_to_cpu(raw_super->qf_ino[0]),
>>>> + le32_to_cpu(raw_super->qf_ino[1]));
>>>> + return 1;
>>>> + }
>>>> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
>>>> + le32_to_cpu(raw_super->qf_ino[2]) != 6) {
>>>> + f2fs_msg(sb, KERN_INFO,
>>>> + "Invalid Quota Ino: prj_ino(%u)",
>>>> + le32_to_cpu(raw_super->qf_ino[2]));
>>>> + return 1;
>>>> + }
>>>> + }
>>>> +
>>>> if (le32_to_cpu(raw_super->segment_count) > F2FS_MAX_SEGMENT) {
>>>> f2fs_msg(sb, KERN_INFO,
>>>> "Invalid segment count (%u)",
>>>> --
>>>> 2.15.0.55.gc2ece9dc4de6
>>>
>>> .
>>>
>
> .
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: add sanity check for quota sysfile ino
2018-01-31 3:38 ` Chao Yu
@ 2018-01-31 3:49 ` Jaegeuk Kim
2018-01-31 6:02 ` Chao Yu
0 siblings, 1 reply; 9+ messages in thread
From: Jaegeuk Kim @ 2018-01-31 3:49 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao
On 01/31, Chao Yu wrote:
> On 2018/1/31 10:36, Jaegeuk Kim wrote:
> > On 01/31, Chao Yu wrote:
> >> On 2018/1/31 9:35, Jaegeuk Kim wrote:
> >>> On 01/29, Chao Yu wrote:
> >>>> Add missing sanity check for quota sysfile ino.
> >>>
> >>> We don't need to limit the specific inode numbers for quota files, since
> >>> we may be able to set any inode numbers later. How about checkint the
> >>> numbers are allocated as quota?
> >
> > If quota_ino is set, f2fs_enable_quotas() will be failed, if its ino is invalid.
> > It'd be also fine, if we have no inode numbers there. What I mean was we may need
> > to consider reallocating or deallocating inode numbers later.
>
> Oh, so that it will be better to do more check & repair in fsck tools, right?
What is the rule that you're considering? We're already checking quota inodes
in fsck.f2fs.
>
> Thanks,
>
> >
> >>
> >> Do you mean:
> >>
> >> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
> >> if (!le32_to_cpu(raw_super->qf_ino[0]) ||
> >> !le32_to_cpu(raw_super->qf_ino[1])) {
> >> f2fs_msg(sb, KERN_INFO,
> >> "Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
> >> le32_to_cpu(raw_super->qf_ino[0]),
> >> le32_to_cpu(raw_super->qf_ino[1]));
> >> return 1;
> >> }
> >> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
> >> !le32_to_cpu(raw_super->qf_ino[2])) {
> >> f2fs_msg(sb, KERN_INFO,
> >> "Invalid Quota Ino: prj_ino(%u)",
> >> le32_to_cpu(raw_super->qf_ino[2]));
> >> return 1;
> >> }
> >> }
> >>
> >> I think it's equal to:
> >>
> >> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
> >> unsigned int qf_cnt = 0;
> >> if (qf_ino[0])
> >> qf_cnt++;
> >> if (qf_ino[1])
> >> qf_cnt++;
> >> if (qf_cnt != 2) {
> >> f2fs_msg();
> >> return 1;
> >> }
> >> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA)) {
> >> if (qf_ino[2])
> >> qf_cnt++;
> >> if (qf_cnt != 3) {
> >> f2fs_msg();
> >> return 1;
> >> }
> >> }
> >> }
> >>
> >> Thanks,
> >>
> >>>
> >>>>
> >>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> >>>> ---
> >>>> fs/f2fs/super.c | 19 +++++++++++++++++++
> >>>> 1 file changed, 19 insertions(+)
> >>>>
> >>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> >>>> index 368f63d7bad2..6011071688ca 100644
> >>>> --- a/fs/f2fs/super.c
> >>>> +++ b/fs/f2fs/super.c
> >>>> @@ -2116,6 +2116,25 @@ static int sanity_check_raw_super(struct f2fs_sb_info *sbi,
> >>>> return 1;
> >>>> }
> >>>>
> >>>> + /* check quota sysfile ino info */
> >>>> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
> >>>> + if (le32_to_cpu(raw_super->qf_ino[0]) != 4 ||
> >>>> + le32_to_cpu(raw_super->qf_ino[1]) != 5) {
> >>>> + f2fs_msg(sb, KERN_INFO,
> >>>> + "Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
> >>>> + le32_to_cpu(raw_super->qf_ino[0]),
> >>>> + le32_to_cpu(raw_super->qf_ino[1]));
> >>>> + return 1;
> >>>> + }
> >>>> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
> >>>> + le32_to_cpu(raw_super->qf_ino[2]) != 6) {
> >>>> + f2fs_msg(sb, KERN_INFO,
> >>>> + "Invalid Quota Ino: prj_ino(%u)",
> >>>> + le32_to_cpu(raw_super->qf_ino[2]));
> >>>> + return 1;
> >>>> + }
> >>>> + }
> >>>> +
> >>>> if (le32_to_cpu(raw_super->segment_count) > F2FS_MAX_SEGMENT) {
> >>>> f2fs_msg(sb, KERN_INFO,
> >>>> "Invalid segment count (%u)",
> >>>> --
> >>>> 2.15.0.55.gc2ece9dc4de6
> >>>
> >>> .
> >>>
> >
> > .
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: add sanity check for quota sysfile ino
2018-01-31 3:49 ` Jaegeuk Kim
@ 2018-01-31 6:02 ` Chao Yu
2018-01-31 21:57 ` Jaegeuk Kim
0 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2018-01-31 6:02 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao
On 2018/1/31 11:49, Jaegeuk Kim wrote:
> On 01/31, Chao Yu wrote:
>> On 2018/1/31 10:36, Jaegeuk Kim wrote:
>>> On 01/31, Chao Yu wrote:
>>>> On 2018/1/31 9:35, Jaegeuk Kim wrote:
>>>>> On 01/29, Chao Yu wrote:
>>>>>> Add missing sanity check for quota sysfile ino.
>>>>>
>>>>> We don't need to limit the specific inode numbers for quota files, since
>>>>> we may be able to set any inode numbers later. How about checkint the
>>>>> numbers are allocated as quota?
>>>
>>> If quota_ino is set, f2fs_enable_quotas() will be failed, if its ino is invalid.
>>> It'd be also fine, if we have no inode numbers there. What I mean was we may need
>>> to consider reallocating or deallocating inode numbers later.
>>
>> Oh, so that it will be better to do more check & repair in fsck tools, right?
>
> What is the rule that you're considering? We're already checking quota inodes
> in fsck.f2fs.
The case that qf_ino[] is corrupted, I didn't see it can be relinked to
corresponding quota inode by fsck, am I missing something?
Thanks,
>
>>
>> Thanks,
>>
>>>
>>>>
>>>> Do you mean:
>>>>
>>>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
>>>> if (!le32_to_cpu(raw_super->qf_ino[0]) ||
>>>> !le32_to_cpu(raw_super->qf_ino[1])) {
>>>> f2fs_msg(sb, KERN_INFO,
>>>> "Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
>>>> le32_to_cpu(raw_super->qf_ino[0]),
>>>> le32_to_cpu(raw_super->qf_ino[1]));
>>>> return 1;
>>>> }
>>>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
>>>> !le32_to_cpu(raw_super->qf_ino[2])) {
>>>> f2fs_msg(sb, KERN_INFO,
>>>> "Invalid Quota Ino: prj_ino(%u)",
>>>> le32_to_cpu(raw_super->qf_ino[2]));
>>>> return 1;
>>>> }
>>>> }
>>>>
>>>> I think it's equal to:
>>>>
>>>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
>>>> unsigned int qf_cnt = 0;
>>>> if (qf_ino[0])
>>>> qf_cnt++;
>>>> if (qf_ino[1])
>>>> qf_cnt++;
>>>> if (qf_cnt != 2) {
>>>> f2fs_msg();
>>>> return 1;
>>>> }
>>>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA)) {
>>>> if (qf_ino[2])
>>>> qf_cnt++;
>>>> if (qf_cnt != 3) {
>>>> f2fs_msg();
>>>> return 1;
>>>> }
>>>> }
>>>> }
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>>>
>>>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>>>> ---
>>>>>> fs/f2fs/super.c | 19 +++++++++++++++++++
>>>>>> 1 file changed, 19 insertions(+)
>>>>>>
>>>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>>>> index 368f63d7bad2..6011071688ca 100644
>>>>>> --- a/fs/f2fs/super.c
>>>>>> +++ b/fs/f2fs/super.c
>>>>>> @@ -2116,6 +2116,25 @@ static int sanity_check_raw_super(struct f2fs_sb_info *sbi,
>>>>>> return 1;
>>>>>> }
>>>>>>
>>>>>> + /* check quota sysfile ino info */
>>>>>> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
>>>>>> + if (le32_to_cpu(raw_super->qf_ino[0]) != 4 ||
>>>>>> + le32_to_cpu(raw_super->qf_ino[1]) != 5) {
>>>>>> + f2fs_msg(sb, KERN_INFO,
>>>>>> + "Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
>>>>>> + le32_to_cpu(raw_super->qf_ino[0]),
>>>>>> + le32_to_cpu(raw_super->qf_ino[1]));
>>>>>> + return 1;
>>>>>> + }
>>>>>> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
>>>>>> + le32_to_cpu(raw_super->qf_ino[2]) != 6) {
>>>>>> + f2fs_msg(sb, KERN_INFO,
>>>>>> + "Invalid Quota Ino: prj_ino(%u)",
>>>>>> + le32_to_cpu(raw_super->qf_ino[2]));
>>>>>> + return 1;
>>>>>> + }
>>>>>> + }
>>>>>> +
>>>>>> if (le32_to_cpu(raw_super->segment_count) > F2FS_MAX_SEGMENT) {
>>>>>> f2fs_msg(sb, KERN_INFO,
>>>>>> "Invalid segment count (%u)",
>>>>>> --
>>>>>> 2.15.0.55.gc2ece9dc4de6
>>>>>
>>>>> .
>>>>>
>>>
>>> .
>>>
>
> .
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: add sanity check for quota sysfile ino
2018-01-31 6:02 ` Chao Yu
@ 2018-01-31 21:57 ` Jaegeuk Kim
2018-02-01 13:41 ` Chao Yu
0 siblings, 1 reply; 9+ messages in thread
From: Jaegeuk Kim @ 2018-01-31 21:57 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao
On 01/31, Chao Yu wrote:
> On 2018/1/31 11:49, Jaegeuk Kim wrote:
> > On 01/31, Chao Yu wrote:
> >> On 2018/1/31 10:36, Jaegeuk Kim wrote:
> >>> On 01/31, Chao Yu wrote:
> >>>> On 2018/1/31 9:35, Jaegeuk Kim wrote:
> >>>>> On 01/29, Chao Yu wrote:
> >>>>>> Add missing sanity check for quota sysfile ino.
> >>>>>
> >>>>> We don't need to limit the specific inode numbers for quota files, since
> >>>>> we may be able to set any inode numbers later. How about checkint the
> >>>>> numbers are allocated as quota?
> >>>
> >>> If quota_ino is set, f2fs_enable_quotas() will be failed, if its ino is invalid.
> >>> It'd be also fine, if we have no inode numbers there. What I mean was we may need
> >>> to consider reallocating or deallocating inode numbers later.
> >>
> >> Oh, so that it will be better to do more check & repair in fsck tools, right?
> >
> > What is the rule that you're considering? We're already checking quota inodes
> > in fsck.f2fs.
>
> The case that qf_ino[] is corrupted, I didn't see it can be relinked to
> corresponding quota inode by fsck, am I missing something?
We can check its basic metadata such as i_mode? Or, in order to do that
entirely, we need to add file truncation and creationg in fsck.f2fs, which
was too complicated to add before.
Any idea?
Thanks,
>
> Thanks,
>
> >
> >>
> >> Thanks,
> >>
> >>>
> >>>>
> >>>> Do you mean:
> >>>>
> >>>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
> >>>> if (!le32_to_cpu(raw_super->qf_ino[0]) ||
> >>>> !le32_to_cpu(raw_super->qf_ino[1])) {
> >>>> f2fs_msg(sb, KERN_INFO,
> >>>> "Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
> >>>> le32_to_cpu(raw_super->qf_ino[0]),
> >>>> le32_to_cpu(raw_super->qf_ino[1]));
> >>>> return 1;
> >>>> }
> >>>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
> >>>> !le32_to_cpu(raw_super->qf_ino[2])) {
> >>>> f2fs_msg(sb, KERN_INFO,
> >>>> "Invalid Quota Ino: prj_ino(%u)",
> >>>> le32_to_cpu(raw_super->qf_ino[2]));
> >>>> return 1;
> >>>> }
> >>>> }
> >>>>
> >>>> I think it's equal to:
> >>>>
> >>>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
> >>>> unsigned int qf_cnt = 0;
> >>>> if (qf_ino[0])
> >>>> qf_cnt++;
> >>>> if (qf_ino[1])
> >>>> qf_cnt++;
> >>>> if (qf_cnt != 2) {
> >>>> f2fs_msg();
> >>>> return 1;
> >>>> }
> >>>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA)) {
> >>>> if (qf_ino[2])
> >>>> qf_cnt++;
> >>>> if (qf_cnt != 3) {
> >>>> f2fs_msg();
> >>>> return 1;
> >>>> }
> >>>> }
> >>>> }
> >>>>
> >>>> Thanks,
> >>>>
> >>>>>
> >>>>>>
> >>>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> >>>>>> ---
> >>>>>> fs/f2fs/super.c | 19 +++++++++++++++++++
> >>>>>> 1 file changed, 19 insertions(+)
> >>>>>>
> >>>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> >>>>>> index 368f63d7bad2..6011071688ca 100644
> >>>>>> --- a/fs/f2fs/super.c
> >>>>>> +++ b/fs/f2fs/super.c
> >>>>>> @@ -2116,6 +2116,25 @@ static int sanity_check_raw_super(struct f2fs_sb_info *sbi,
> >>>>>> return 1;
> >>>>>> }
> >>>>>>
> >>>>>> + /* check quota sysfile ino info */
> >>>>>> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
> >>>>>> + if (le32_to_cpu(raw_super->qf_ino[0]) != 4 ||
> >>>>>> + le32_to_cpu(raw_super->qf_ino[1]) != 5) {
> >>>>>> + f2fs_msg(sb, KERN_INFO,
> >>>>>> + "Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
> >>>>>> + le32_to_cpu(raw_super->qf_ino[0]),
> >>>>>> + le32_to_cpu(raw_super->qf_ino[1]));
> >>>>>> + return 1;
> >>>>>> + }
> >>>>>> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
> >>>>>> + le32_to_cpu(raw_super->qf_ino[2]) != 6) {
> >>>>>> + f2fs_msg(sb, KERN_INFO,
> >>>>>> + "Invalid Quota Ino: prj_ino(%u)",
> >>>>>> + le32_to_cpu(raw_super->qf_ino[2]));
> >>>>>> + return 1;
> >>>>>> + }
> >>>>>> + }
> >>>>>> +
> >>>>>> if (le32_to_cpu(raw_super->segment_count) > F2FS_MAX_SEGMENT) {
> >>>>>> f2fs_msg(sb, KERN_INFO,
> >>>>>> "Invalid segment count (%u)",
> >>>>>> --
> >>>>>> 2.15.0.55.gc2ece9dc4de6
> >>>>>
> >>>>> .
> >>>>>
> >>>
> >>> .
> >>>
> >
> > .
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] f2fs: add sanity check for quota sysfile ino
2018-01-31 21:57 ` Jaegeuk Kim
@ 2018-02-01 13:41 ` Chao Yu
0 siblings, 0 replies; 9+ messages in thread
From: Chao Yu @ 2018-02-01 13:41 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel, linux-kernel
On 2018/2/1 5:57, Jaegeuk Kim wrote:
> On 01/31, Chao Yu wrote:
>> On 2018/1/31 11:49, Jaegeuk Kim wrote:
>>> On 01/31, Chao Yu wrote:
>>>> On 2018/1/31 10:36, Jaegeuk Kim wrote:
>>>>> On 01/31, Chao Yu wrote:
>>>>>> On 2018/1/31 9:35, Jaegeuk Kim wrote:
>>>>>>> On 01/29, Chao Yu wrote:
>>>>>>>> Add missing sanity check for quota sysfile ino.
>>>>>>>
>>>>>>> We don't need to limit the specific inode numbers for quota files, since
>>>>>>> we may be able to set any inode numbers later. How about checkint the
>>>>>>> numbers are allocated as quota?
>>>>>
>>>>> If quota_ino is set, f2fs_enable_quotas() will be failed, if its ino is invalid.
>>>>> It'd be also fine, if we have no inode numbers there. What I mean was we may need
>>>>> to consider reallocating or deallocating inode numbers later.
>>>>
>>>> Oh, so that it will be better to do more check & repair in fsck tools, right?
>>>
>>> What is the rule that you're considering? We're already checking quota inodes
>>> in fsck.f2fs.
>>
>> The case that qf_ino[] is corrupted, I didn't see it can be relinked to
>> corresponding quota inode by fsck, am I missing something?
>
> We can check its basic metadata such as i_mode? Or, in order to do that
Agreed, how about:
if (i_mode == 0x8180 && i_flags == FS_IMMUTABLE_FL &&
i_name[0] == 0x00 && i_namelen == 0)
relink super_block::qf_ino to quote inode;
> entirely, we need to add file truncation and creationg in fsck.f2fs, which
Yes, I think we can rebuild sys quote file if original one is missing or corrupted.
Thanks,
> was too complicated to add before.
> Any idea?
>
> Thanks,
>
>>
>> Thanks,
>>
>>>
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>>>
>>>>>> Do you mean:
>>>>>>
>>>>>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
>>>>>> if (!le32_to_cpu(raw_super->qf_ino[0]) ||
>>>>>> !le32_to_cpu(raw_super->qf_ino[1])) {
>>>>>> f2fs_msg(sb, KERN_INFO,
>>>>>> "Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
>>>>>> le32_to_cpu(raw_super->qf_ino[0]),
>>>>>> le32_to_cpu(raw_super->qf_ino[1]));
>>>>>> return 1;
>>>>>> }
>>>>>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
>>>>>> !le32_to_cpu(raw_super->qf_ino[2])) {
>>>>>> f2fs_msg(sb, KERN_INFO,
>>>>>> "Invalid Quota Ino: prj_ino(%u)",
>>>>>> le32_to_cpu(raw_super->qf_ino[2]));
>>>>>> return 1;
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> I think it's equal to:
>>>>>>
>>>>>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
>>>>>> unsigned int qf_cnt = 0;
>>>>>> if (qf_ino[0])
>>>>>> qf_cnt++;
>>>>>> if (qf_ino[1])
>>>>>> qf_cnt++;
>>>>>> if (qf_cnt != 2) {
>>>>>> f2fs_msg();
>>>>>> return 1;
>>>>>> }
>>>>>> if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA)) {
>>>>>> if (qf_ino[2])
>>>>>> qf_cnt++;
>>>>>> if (qf_cnt != 3) {
>>>>>> f2fs_msg();
>>>>>> return 1;
>>>>>> }
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>>>>>>> ---
>>>>>>>> fs/f2fs/super.c | 19 +++++++++++++++++++
>>>>>>>> 1 file changed, 19 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>>>>>>>> index 368f63d7bad2..6011071688ca 100644
>>>>>>>> --- a/fs/f2fs/super.c
>>>>>>>> +++ b/fs/f2fs/super.c
>>>>>>>> @@ -2116,6 +2116,25 @@ static int sanity_check_raw_super(struct f2fs_sb_info *sbi,
>>>>>>>> return 1;
>>>>>>>> }
>>>>>>>>
>>>>>>>> + /* check quota sysfile ino info */
>>>>>>>> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
>>>>>>>> + if (le32_to_cpu(raw_super->qf_ino[0]) != 4 ||
>>>>>>>> + le32_to_cpu(raw_super->qf_ino[1]) != 5) {
>>>>>>>> + f2fs_msg(sb, KERN_INFO,
>>>>>>>> + "Invalid Quota Ino: user_ino(%u), grp_ino(%u)",
>>>>>>>> + le32_to_cpu(raw_super->qf_ino[0]),
>>>>>>>> + le32_to_cpu(raw_super->qf_ino[1]));
>>>>>>>> + return 1;
>>>>>>>> + }
>>>>>>>> + if (raw_super->feature & cpu_to_le32(F2FS_FEATURE_PRJQUOTA) &&
>>>>>>>> + le32_to_cpu(raw_super->qf_ino[2]) != 6) {
>>>>>>>> + f2fs_msg(sb, KERN_INFO,
>>>>>>>> + "Invalid Quota Ino: prj_ino(%u)",
>>>>>>>> + le32_to_cpu(raw_super->qf_ino[2]));
>>>>>>>> + return 1;
>>>>>>>> + }
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> if (le32_to_cpu(raw_super->segment_count) > F2FS_MAX_SEGMENT) {
>>>>>>>> f2fs_msg(sb, KERN_INFO,
>>>>>>>> "Invalid segment count (%u)",
>>>>>>>> --
>>>>>>>> 2.15.0.55.gc2ece9dc4de6
>>>>>>>
>>>>>>> .
>>>>>>>
>>>>>
>>>>> .
>>>>>
>>>
>>> .
>>>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-02-01 13:41 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-29 8:29 [PATCH] f2fs: add sanity check for quota sysfile ino Chao Yu
2018-01-31 1:35 ` Jaegeuk Kim
2018-01-31 2:01 ` Chao Yu
2018-01-31 2:36 ` Jaegeuk Kim
2018-01-31 3:38 ` Chao Yu
2018-01-31 3:49 ` Jaegeuk Kim
2018-01-31 6:02 ` Chao Yu
2018-01-31 21:57 ` Jaegeuk Kim
2018-02-01 13:41 ` 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).