linux-ext4 - Re: [RFC PATCH 2/5] ext4: reduce reserved cluster count by number of allocated clusters

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]

Date:   Wed, 30 May 2018 16:31:10 +0200
From:   Jan Kara <jack@...e.cz>
To:     Eric Whitney <enwlinux@...il.com>
Cc:     linux-ext4@...r.kernel.org, tytso@....edu
Subject: Re: [RFC PATCH 2/5] ext4: reduce reserved cluster count by number of
 allocated clusters

On Sun 13-05-18 13:56:21, Eric Whitney wrote:
> Ext4 does not always reduce the reserved cluster count by the number
> of clusters allocated when mapping a delayed extent.  It sometimes
> adds back one or more clusters after allocation if delalloc blocks
> adjacent to the range allocated by ext4_ext_map_blocks() share the
> clusters newly allocated for that range.  However, this overcounts
> the number of clusters needed to satisfy future mapping requests
> (holding one or more reservations for clusters that have already been
> allocated) and premature ENOSPC and quota failures, etc., result.
> 
> The current ext4 code does not reduce the reserved cluster count
> when allocating clusters for non-delalloc writes that have also been
> previously reserved for delalloc writes.  This also results in a
> reserved cluster overcount.
> 
> To make it possible to handle reserved cluster accounting for
> fallocated regions in the same manner as used for other non-delayed
> writes, do the reserved cluster accounting for them at the time of
> allocation.  In the current code, this is only done later when a
> delayed extent sharing the fallocated region is finally mapped.  This
> behavior can also result in a temporary reserved cluster overcount.
> 
> Signed-off-by: Eric Whitney <enwlinux@...il.com>

Nice simplification. Just one small comment below. With that fixed you can
add:

Reviewed-by: Jan Kara <jack@...e.cz>

...
> +static unsigned int __es_delayed_clu(struct inode *inode, ext4_lblk_t start,
> +				     ext4_lblk_t end)
> +{
> +	struct ext4_es_tree *tree = &EXT4_I(inode)->i_es_tree;
> +	struct extent_status *es;
> +	struct ext4_sb_info *sbi = EXT4_SB(inode->i_sb);
> +	struct rb_node *node;
> +	ext4_lblk_t first_lclu, last_lclu;
> +	unsigned long long last_counted_lclu;
> +	unsigned int n = 0;
> +
> +	/* guaranteed to be unequal to any ext4_lblk_t value */
> +	last_counted_lclu = ~0;

Missing ULL here? ~0 actually also is not valid lblk offset (look at
ext4_max_size()) but then last_counted_lclu could be just ext4_lblk_t...

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