* [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-kernel, linux-f2fs-devel 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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ 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-kernel, linux-f2fs-devel 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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ 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-kernel, linux-f2fs-devel 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 >>> >>> . >>> > > . > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ 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-kernel, linux-f2fs-devel 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 > >>> > >>> . > >>> > > > > . > > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ 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).