linux-kernel - Re: [PATCH] quota: fix wrong condition in is_quota

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190916082306.GB2485@quack2.suse.cz>
Date:   Mon, 16 Sep 2019 10:23:06 +0200
From:   Jan Kara <jack@...e.cz>
To:     Chao Yu <yuchao0@...wei.com>
Cc:     Jan Kara <jack@...e.cz>, Jan Kara <jack@...e.com>, chao@...nel.org,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] quota: fix wrong condition in is_quota_modification()

On Mon 16-09-19 10:53:08, Chao Yu wrote:
> On 2019/9/12 18:06, Jan Kara wrote:
> > On Wed 11-09-19 17:36:50, Chao Yu wrote:
> >> diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h
> >> index dc905a4ff8d7..bd30acad3a7f 100644
> >> --- a/include/linux/quotaops.h
> >> +++ b/include/linux/quotaops.h
> >> @@ -22,7 +22,7 @@ static inline struct quota_info *sb_dqopt(struct super_block *sb)
> >>  /* i_mutex must being held */
> >>  static inline bool is_quota_modification(struct inode *inode, struct iattr *ia)
> >>  {
> >> -	return (ia->ia_valid & ATTR_SIZE && ia->ia_size != inode->i_size) ||
> >> +	return (ia->ia_valid & ATTR_SIZE && ia->ia_size <= inode->i_size) ||
> >>  		(ia->ia_valid & ATTR_UID && !uid_eq(ia->ia_uid, inode->i_uid)) ||
> >>  		(ia->ia_valid & ATTR_GID && !gid_eq(ia->ia_gid, inode->i_gid));
> >>  }
> > 
> > OK, but your change makes i_size extension not to be quota modification
> 
> I just try to adapt below rules covered with generic/092, which restrict
> to not trim preallocate blocks beyond i_size, in that case, filesystem
> won't change i_blocks.
> 
> 1) truncate(i_size) will trim all blocks past i_size.
> 2) truncate(x) where x > i_size will not trim all blocks past i_size.

Ah, OK.

> However, I'm okay with your change, because there could be filesystems won't
> follow above rule.

Yes, I'm concerned that some filesystem may change i_blocks in some corner
case when growing inode size (e.g. when it decides to convert inode from
inline format to a normal block based format or something like that). So I
don't think the optimization is really worth the chance for breakage.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR