From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 34839C48BC2 for ; Mon, 28 Jun 2021 00:39:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 093C561998 for ; Mon, 28 Jun 2021 00:39:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231725AbhF1AmW (ORCPT ); Sun, 27 Jun 2021 20:42:22 -0400 Received: from mail110.syd.optusnet.com.au ([211.29.132.97]:37016 "EHLO mail110.syd.optusnet.com.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231689AbhF1AmW (ORCPT ); Sun, 27 Jun 2021 20:42:22 -0400 Received: from dread.disaster.area (pa49-179-138-183.pa.nsw.optusnet.com.au [49.179.138.183]) by mail110.syd.optusnet.com.au (Postfix) with ESMTPS id 7D784107992; Mon, 28 Jun 2021 10:39:55 +1000 (AEST) Received: from dave by dread.disaster.area with local (Exim 4.92.3) (envelope-from ) id 1lxfJa-000Buk-6p; Mon, 28 Jun 2021 10:39:54 +1000 Date: Mon, 28 Jun 2021 10:39:54 +1000 From: Dave Chinner To: Wang Shilong Cc: Ext4 Developers List , Wang Shilong Subject: Re: [PATCH] ext4: forbid U32_MAX project ID Message-ID: <20210628003954.GM2419729@dread.disaster.area> References: <20210625124033.5639-1-wangshilong1991@gmail.com> <20210627224217.GL2419729@dread.disaster.area> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.3 cv=F8MpiZpN c=1 sm=1 tr=0 a=MnllW2CieawZLw/OcHE/Ng==:117 a=MnllW2CieawZLw/OcHE/Ng==:17 a=kj9zAlcOel0A:10 a=r6YtysWOX24A:10 a=7-415B0cAAAA:8 a=lB0dNpNiAAAA:8 a=9nCa61v3IhUg5NtY66YA:9 a=CjuIK1q_8ugA:10 a=biEYGPWJfzWAr4FL6Ov7:22 a=c-ZiYqmG3AbHTdtsH08C:22 Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On Mon, Jun 28, 2021 at 07:13:04AM +0800, Wang Shilong wrote: > On Mon, Jun 28, 2021 at 6:42 AM Dave Chinner wrote: > > > > On Fri, Jun 25, 2021 at 08:40:33AM -0400, Wang Shilong wrote: > > > From: Wang Shilong > > > > > > 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 > > > --- > > > 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