Re: [PATCH v2] ext4: check if offset+length is valid in fallocate

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ira,
On 3/22/22 09:37, Ira Weiny wrote:
On Tue, Mar 15, 2022 at 02:54:39PM -0700, Tadeusz Struk wrote:
Syzbot found an issue [1] in ext4_fallocate().
The C reproducer [2] calls fallocate(), passing size 0xffeffeff000ul,
and offset 0x1000000ul, which, when added together exceed the disk size,
and trigger a BUG in ext4_ind_remove_space() [3].
According to the comment doc in ext4_ind_remove_space() the 'end' block
parameter needs to be one block after the last block to remove.
In the case when the BUG is triggered it points to the last block on
a 4GB virtual disk image. This is calculated in
ext4_ind_remove_space() in [4].
This patch adds a check that ensure the length + offest to be
within the valid range and returns -ENOSPC error code in case
it is invalid.

Why is the check in vfs_fallocate() not working for this?

https://elixir.bootlin.com/linux/v5.17-rc8/source/fs/open.c#L300

Good question. From reading the comment:
https://elixir.bootlin.com/linux/v5.17/source/fs/ext4/file.c#L225

it is possible that, for the bitmap-format, the limit might be smaller
than the s_maxbytes.

But even for a extent-mapped file the offest+len needs to be within the
first to last-1 block range for fallocate(fd, FALLOC_FL_PUNCH_HOLE, ...)
If it points to the last one then it is invalid, no?

The check you pointed to in vfs code checks if offest+len goes beyond
maximal file size.

Also why do other file systems not fail?  Is it because ext4 is special due to
the end block needing to be one block after the last.  That seems to imply the
last block can't be used or there is some off by one issue somewhere?

According to the comment in
https://elixir.bootlin.com/linux/v5.17/source/fs/ext4/indirect.c#L1214
it has to be one block after the last to be removed.

--
Thanks,
Tadeusz



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux