linux-kernel - Re: [PATCH] ext4: deaccount delayed allocations at freeing inode in ext4_evict

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: <20191108115420.GI20863@quack2.suse.cz>
Date:   Fri, 8 Nov 2019 12:54:20 +0100
From:   Jan Kara <jack@...e.cz>
To:     Konstantin Khlebnikov <khlebnikov@...dex-team.ru>
Cc:     Ritesh Harjani <riteshh@...ux.ibm.com>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        linux-ext4@...r.kernel.org, Theodore Ts'o <tytso@....edu>,
        linux-kernel@...r.kernel.org,
        Dmitry Monakhov <dmtrmonakhov@...dex-team.ru>,
        Eric Whitney <enwlinux@...il.com>
Subject: Re: [PATCH] ext4: deaccount delayed allocations at freeing inode in
 ext4_evict_inode()

On Fri 08-11-19 11:30:56, Konstantin Khlebnikov wrote:
> On 08/11/2019 05.08, Ritesh Harjani wrote:
> > 
> > 
> > On 10/29/19 12:47 PM, Konstantin Khlebnikov wrote:
> > > If inode->i_blocks is zero then ext4_evict_inode() skips ext4_truncate().
> > > Delayed allocation extents are freed later in ext4_clear_inode() but this
> > > happens when quota reference is already dropped. This leads to leak of
> > > reserved space in quota block, which disappears after umount-mount.
> > > 
> > > This seems broken for a long time but worked somehow until recent changes
> > > in delayed allocation.
> > 
> > Sorry, I may have missed it, but could you please help understand
> > what recent changes in delayed allocation make this break or worse?
> 
> I don't see problem for 4.19. Haven't bisected yet.
> Most likely this is around 'reserved cluster accounting'.
> 
> I suspect before these changes something always triggered da before
> unlink and space usage committed and then truncated at eviction.

Yes, I think it's commit 8fcc3a580651 "ext4: rework reserved cluster
accounting when invalidating pages". Because that commit moved releasing of
reserved space from page invalidation time to extent status tree eviction
time. Does attached patch fix the problem for you?

> > A silly query, since I couldn't figure it out. Maybe the code has been
> > there ever since like this:-
> 
> > So why can't we just move drop_dquot later after the ext4_es_remove_extent() (in function ext4_clear_inode)? Any known
> > problems around that?
> 
> Clear_inode is called also when inode evicts from cache while it has nlinks
> and stays at disk. I'm not sure how this must interact with reserves.

In that case all data should be written out for such inode and thus there
should be no reserves...

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

View attachment "0001-ext4-Fix-leak-of-quota-reservations.patch" of type "text/x-patch" (2147 bytes)