* [PATCH] ext4: forbid U32_MAX project ID @ 2021-06-25 12:40 Wang Shilong 2021-06-27 22:42 ` Dave Chinner 0 siblings, 1 reply; 4+ messages in thread From: Wang Shilong @ 2021-06-25 12:40 UTC (permalink / raw) To: linux-ext4; +Cc: wangshilong1991, Wang Shilong From: Wang Shilong <wshilong@ddn.com> U32_MAX is reserved for special purpose, qid_has_mapping() will return false if projid is 4294967295, dqget() will return NULL for it. So U32_MAX is unsupported Project ID, fix to forbid it. Signed-off-by: Wang Shilong <wshilong@ddn.com> --- fs/ext4/ioctl.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c index 31627f7dc5cd..f3a8d962c291 100644 --- a/fs/ext4/ioctl.c +++ b/fs/ext4/ioctl.c @@ -744,6 +744,9 @@ int ext4_fileattr_set(struct user_namespace *mnt_userns, u32 flags = fa->flags; int err = -EOPNOTSUPP; + if (fa->fsx_projid >= U32_MAX) + return -EINVAL; + ext4_fc_start_update(inode); if (flags & ~EXT4_FL_USER_VISIBLE) goto out; -- 2.27.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ext4: forbid U32_MAX project ID 2021-06-25 12:40 [PATCH] ext4: forbid U32_MAX project ID Wang Shilong @ 2021-06-27 22:42 ` Dave Chinner 2021-06-27 23:13 ` Wang Shilong 0 siblings, 1 reply; 4+ messages in thread From: Dave Chinner @ 2021-06-27 22:42 UTC (permalink / raw) To: Wang Shilong; +Cc: linux-ext4, Wang Shilong On Fri, Jun 25, 2021 at 08:40:33AM -0400, Wang Shilong wrote: > From: Wang Shilong <wshilong@ddn.com> > > U32_MAX is reserved for special purpose, > qid_has_mapping() will return false if projid is > 4294967295, dqget() will return NULL for it. > > So U32_MAX is unsupported Project ID, fix to forbid > it. Actually, it's INVALID_PROJID, not U32_MAX, and we already have a check function for that: static inline bool projid_valid(kprojid_t projid) { return !projid_eq(projid, INVALID_PROJID); } > Signed-off-by: Wang Shilong <wshilong@ddn.com> > --- > fs/ext4/ioctl.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c > index 31627f7dc5cd..f3a8d962c291 100644 > --- a/fs/ext4/ioctl.c > +++ b/fs/ext4/ioctl.c > @@ -744,6 +744,9 @@ int ext4_fileattr_set(struct user_namespace *mnt_userns, > u32 flags = fa->flags; > int err = -EOPNOTSUPP; > > + if (fa->fsx_projid >= U32_MAX) > + return -EINVAL; > + This should actually be calling qid_valid() or projid_valid(), and it should be in generic code because multiple filesystems support project quotas. i.e this should be checked in fileattr_set_prepare(), not in ext4 specific code. Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ext4: forbid U32_MAX project ID 2021-06-27 22:42 ` Dave Chinner @ 2021-06-27 23:13 ` Wang Shilong 2021-06-28 0:39 ` Dave Chinner 0 siblings, 1 reply; 4+ messages in thread From: Wang Shilong @ 2021-06-27 23:13 UTC (permalink / raw) To: Dave Chinner; +Cc: Ext4 Developers List, Wang Shilong On Mon, Jun 28, 2021 at 6:42 AM Dave Chinner <david@fromorbit.com> wrote: > > On Fri, Jun 25, 2021 at 08:40:33AM -0400, Wang Shilong wrote: > > From: Wang Shilong <wshilong@ddn.com> > > > > U32_MAX is reserved for special purpose, > > qid_has_mapping() will return false if projid is > > 4294967295, dqget() will return NULL for it. > > > > So U32_MAX is unsupported Project ID, fix to forbid > > it. > > Actually, it's INVALID_PROJID, not U32_MAX, and we already have a > check function for that: > > static inline bool projid_valid(kprojid_t projid) > { > return !projid_eq(projid, INVALID_PROJID); > } > I was not aware of this, thanks for pointing it out. > > Signed-off-by: Wang Shilong <wshilong@ddn.com> > > --- > > fs/ext4/ioctl.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c > > index 31627f7dc5cd..f3a8d962c291 100644 > > --- a/fs/ext4/ioctl.c > > +++ b/fs/ext4/ioctl.c > > @@ -744,6 +744,9 @@ int ext4_fileattr_set(struct user_namespace *mnt_userns, > > u32 flags = fa->flags; > > int err = -EOPNOTSUPP; > > > > + if (fa->fsx_projid >= U32_MAX) > > + return -EINVAL; > > + > > This should actually be calling qid_valid() or projid_valid(), > and it should be in generic code because multiple filesystems > support project quotas. i.e this should be checked in > fileattr_set_prepare(), not in ext4 specific code. I tried to fix ext4/f2fs, i am not sure about XFS, it looks to me XFS implemented quota mostly by itself. Anyway, let me fix this in generic code. > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ext4: forbid U32_MAX project ID 2021-06-27 23:13 ` Wang Shilong @ 2021-06-28 0:39 ` Dave Chinner 0 siblings, 0 replies; 4+ messages in thread From: Dave Chinner @ 2021-06-28 0:39 UTC (permalink / raw) To: Wang Shilong; +Cc: Ext4 Developers List, Wang Shilong On Mon, Jun 28, 2021 at 07:13:04AM +0800, Wang Shilong wrote: > On Mon, Jun 28, 2021 at 6:42 AM Dave Chinner <david@fromorbit.com> wrote: > > > > On Fri, Jun 25, 2021 at 08:40:33AM -0400, Wang Shilong wrote: > > > From: Wang Shilong <wshilong@ddn.com> > > > > > > U32_MAX is reserved for special purpose, > > > qid_has_mapping() will return false if projid is > > > 4294967295, dqget() will return NULL for it. > > > > > > So U32_MAX is unsupported Project ID, fix to forbid > > > it. > > > > Actually, it's INVALID_PROJID, not U32_MAX, and we already have a > > check function for that: > > > > static inline bool projid_valid(kprojid_t projid) > > { > > return !projid_eq(projid, INVALID_PROJID); > > } > > > > I was not aware of this, thanks for pointing it out. > > > > Signed-off-by: Wang Shilong <wshilong@ddn.com> > > > --- > > > fs/ext4/ioctl.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c > > > index 31627f7dc5cd..f3a8d962c291 100644 > > > --- a/fs/ext4/ioctl.c > > > +++ b/fs/ext4/ioctl.c > > > @@ -744,6 +744,9 @@ int ext4_fileattr_set(struct user_namespace *mnt_userns, > > > u32 flags = fa->flags; > > > int err = -EOPNOTSUPP; > > > > > > + if (fa->fsx_projid >= U32_MAX) > > > + return -EINVAL; > > > + > > > > This should actually be calling qid_valid() or projid_valid(), > > and it should be in generic code because multiple filesystems > > support project quotas. i.e this should be checked in > > fileattr_set_prepare(), not in ext4 specific code. > > I tried to fix ext4/f2fs, i am not sure about XFS, it looks to me XFS > implemented quota mostly by itself. Yes, XFS is where project quotas originally come from - XFS has had project quotas since quotas were first implemented in XFS back in 1995, long before it was ported to Linux. Ext4 and other filesystems are very recent Linux re-implementations, hence the different quota infrastructure. As it is, XFS project quotas can be queried and controlled through the same generic linux quota APIs ithat ext4 uses as well as it's own.... But the above change is not in quota code - you're changing code in the FS_IOC_FSSETXATTR ioctl call (again, originally XFS code, but lifted to the VFS level so ext4 et al can manipulate project quotas). Hence parameter validity checks for parameters need to be done at the VFS layers... > Anyway, let me fix this in generic code. Thanks! Cheers, -Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-28 0:39 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-06-25 12:40 [PATCH] ext4: forbid U32_MAX project ID Wang Shilong 2021-06-27 22:42 ` Dave Chinner 2021-06-27 23:13 ` Wang Shilong 2021-06-28 0:39 ` Dave Chinner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox