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  PHC 
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Thu, 31 Mar 2022 11:03:59 -0700
From:   Tadeusz Struk <>
To:     Theodore Ts'o <>
Cc:     Andreas Dilger <>,
        Ritesh Harjani <>,,,,
Subject: Re: [PATCH v2] ext4: check if offset+length is valid in fallocate

On 3/31/22 07:43, Theodore Ts'o wrote:
> On Tue, Mar 15, 2022 at 02:54:39PM -0700, Tadeusz Struk wrote:
>> @@ -3967,6 +3968,16 @@ int ext4_punch_hole(struct inode *inode, loff_t offset, loff_t length)
>>   		   offset;
>>   	}
>> +	/*
>> +	 * For punch hole the length + offset needs to be at least within
>> +	 * one block before last
>> +	 */
>> +	max_length = sbi->s_bitmap_maxbytes - inode->i_sb->s_blocksize;
>> +	if (offset + length >= max_length) {
>> +		ret = -ENOSPC;
>> +		goto out_mutex;
>> +	}
> I wonder if we would be better off just simply capping length to
> max_length?  If length is set to some large value, such as LONG_MAX,
> it's pretty clear what the intention should be, which is to simply do
> the equivalent of truncating the file at offset.  Perhaps we should
> just do that?

Don't think that would be the correct behavior. ftrucnate (or truncate)
modify the file size, but fallocate with FALLOC_FL_PUNCH_HOLE should not.

man 2 fallocate says:
The FALLOC_FL_PUNCH_HOLE flag must be ORed with FALLOC_FL_KEEP_SIZE in mode;
in other words, even when punching off the end of the file, the file size
(as reported by stat(2)) does not change.
that is enforced by vfs:

> That being said, we should be consistent with what other file systems
> do when they are asked to punch a hole starting at offset and
> extending out to LONG_MAX.

For all the supported file systems, apart from ext4, only btrfs, gfs2, and xfs
support fallocate and FALLOC_FL_PUNCH_HOLE mode.
Looking at what they do is they round the length of the space to be freed
i.e. offset + length to valid value and then perform the operation
using the valid values.

For ext4 this would mean that one could only deallocate space up to
the one before last block. I will change this to do the same in the
next version if that's ok with you.

> Also, if we are going to return an error, I don't think ENOSPC is the
> correct error to be returning.

I took it from man 2 fallocate, my first suspicion was that it crashed
because the disk on my VM wasn't big enough. It was a bad choice.


Powered by blists - more mailing lists