Switching address_space_operations while a file is used is difficult to
do in a race-free way. To be able to use single address_space_operations
in UDF, make in-ICB files use udf_direct_IO().
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jan Kara <jack@suse.cz>
Switching address_space_operations while a file is used is difficult to
do in a race-free way. To be able to use single address_space_operations
in UDF, make in-ICB files use udf_writepages().
Reported-by: syzbot+c27475eb921c46bbdc62@syzkaller.appspotmail.com
Reported-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jan Kara <jack@suse.cz>
Switching address_space_operations while a file is used is difficult to
do in a race-free way. To be able to use single address_space_operations
in UDF, make udf_read_folio() handle both normal and in-ICB files.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jan Kara <jack@suse.cz>
The condition determining whether the preallocation can be used had
an off-by-one error so we didn't discard preallocation when new
allocation was just following it. This can then confuse code in
inode_getblk().
CC: stable@vger.kernel.org
Fixes: 16d0556568 ("udf: Discard preallocation before extending file with a hole")
Signed-off-by: Jan Kara <jack@suse.cz>
When we append new block just after the end of preallocated extent, the
code in inode_getblk() wrongly determined we're going to use the
preallocated extent which resulted in adding block into a wrong logical
offset in the file. Sequence like this manifests it:
xfs_io -f -c "pwrite 0x2cacf 0xd122" -c "truncate 0x2dd6f" \
-c "pwrite 0x27fd9 0x69a9" -c "pwrite 0x32981 0x7244" <file>
The code that determined the use of preallocated extent is actually
stale because udf_do_extend_file() does not create preallocation anymore
so after calling that function we are sure there's no usable
preallocation. Just remove the faulty condition.
CC: stable@vger.kernel.org
Fixes: 16d0556568 ("udf: Discard preallocation before extending file with a hole")
Signed-off-by: Jan Kara <jack@suse.cz>
Now when we allocate blocks on write page fault there should be no block
allocation happening on page writeback. So just ignore the 'create' flag
passed to udf_get_block(). Note that we can spot dirty buffers without
underlying blocks allocated in writeback when we race with expanding
truncate. However in that case these buffers do not contain valid data
so we can safely ignore them and we would just create ourselves problem
when to trim the tail extent.
Signed-off-by: Jan Kara <jack@suse.cz>
When filesystem's ->get_block function does not map the buffer head when
called from __mpage_writepage(), the function will happily go and pass
bogus bdev and block number to bio allocation routines which leads to
crashes sooner or later. E.g. UDF can do this because it doesn't want to
allocate blocks from ->writepages callbacks. It allocates blocks on
write or page fault but writeback can still spot dirty buffers without
underlying blocks allocated e.g. if blocksize < pagesize, the tail page
is dirtied (which means all its buffers are dirtied), and truncate
extends the file so that some buffer starts to be within i_size.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jan Kara <jack@suse.cz>
Currently if file with holes is mapped, udf allocates blocks for dirtied
pages during page writeback. This however creates problems when to
truncate final extent to proper size and currently we leave the last
extent untruncated which violates UDF standard. So allocate blocks on
write page fault instead. In that case the last extent gets truncated
the file is closed and everything is happy.
Signed-off-by: Jan Kara <jack@suse.cz>
Protect truncate and file type conversion in udf_file_write_iter() with
invalidate lock. That will allow us to serialize these paths with page
faults so that the page fault can determine the file type in a racefree
way.
Signed-off-by: Jan Kara <jack@suse.cz>
The checks we do in udf_setsize() and udf_file_write_iter() are safe to
do only with i_rwsem locked as it stabilizes both file type and file
size. Hence we don't need to lock i_data_sem before we enter
udf_expand_file_adinicb() which simplifies the locking somewhat.
Signed-off-by: Jan Kara <jack@suse.cz>
When we are renaming a directory to a different directory, we need to
update '..' entry in the moved directory. However nothing prevents moved
directory from being modified and even converted from the in-ICB format
to the normal format which results in a crash. Fix the problem by
locking the moved directory.
Reported-by: syzbot+aebf90eea2671c43112a@syzkaller.appspotmail.com
Signed-off-by: Jan Kara <jack@suse.cz>
In some cases we don't want to create block preallocation when
allocating blocks. Add a flag to udf_map_rq controlling the behavior.
Signed-off-by: Jan Kara <jack@suse.cz>
Currently udf_symlink_filler() called udf_block_map() even on files
which have data stored inside the ICB. This is invalid as we cannot map
blocks for such files (although so far the error got silently ignored).
The call happened because we could not call block mapping function once
we've acquired i_data_sem and determined whether the file has data
stored in the ICB. For symlinks the situation is luckily simple as they
get never modified so file type never changes once it is set. Hence we
can check the file type even without i_data_sem. Just drop the
i_data_sem locking and move block mapping to where it is needed.
Signed-off-by: Jan Kara <jack@suse.cz>
Create new block mapping function udf_map_block() that takes new
udf_map_rq structure describing mapping request. We will convert other
places to use this function for block mapping.
Signed-off-by: Jan Kara <jack@suse.cz>
inode_getblk() sets goal block for the next allocation to the currently
allocated block. This is obviously one less than what the goal block
should be which we fixup in udf_get_block(). Just set the right goal
block directly in inode_getblk().
Signed-off-by: Jan Kara <jack@suse.cz>
UDF was supporting a strange mode where the media was containing 7
blocks of unknown data for every 32 blocks of the filesystem. I have yet
to see the media that would need such conversion (maybe it comes from
packet writing times) and the conversions have been inconsistent in the
code. In particular any write will write to a wrong block and corrupt
the media. This is an indication and no user actually needs this so
let's just drop the support instead of trying to fix it.
Signed-off-by: Jan Kara <jack@suse.cz>
When detecting last recorded block and from it derived anchor block
position, we were mixing unsigned long, u32, and sector_t types. Since
udf supports only 32-bit block numbers this is harmless but sometimes
makes things awkward. Convert everything to udf_pblk_t and also handle
the situation when block device size would not fit into udf_pblk_t.
Signed-off-by: Jan Kara <jack@suse.cz>
When directory's last extent has more that one block and its length is
not multiple of a block side, the code wrongly decided to move to the
next extent instead of processing the last partial block. This led to
directory corruption. Fix the rounding issue.
Signed-off-by: Jan Kara <jack@suse.cz>
When we spot directory corruption when trying to load next directory
extent, we didn't propagate the error up properly, leading to possibly
indefinite looping on corrupted directories. Fix the problem by
propagating the error properly.
Signed-off-by: Jan Kara <jack@suse.cz>
Propagate errors from ext2_prepare_chunk to the callers and handle them
there. While touching the prototype also turn update_times into a bool
from the current int used as bool.
[JK: fixed up error recovery path in ext2_rename()]
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jan Kara <jack@suse.cz>
Message-Id: <20230116085205.2342975-1-hch@lst.de>
The variable netype is assigned a value that is never read, the assignment
is redundant the variable can be removed.
Message-Id: <20230105134925.45599-1-colin.i.king@gmail.com>
Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
When UDF filesystem is corrupted, hidden system inodes can be linked
into directory hierarchy which is an avenue for further serious
corruption of the filesystem and kernel confusion as noticed by syzbot
fuzzed images. Refuse to access system inodes linked into directory
hierarchy and vice versa.
CC: stable@vger.kernel.org
Reported-by: syzbot+38695a20b8addcbc1084@syzkaller.appspotmail.com
Signed-off-by: Jan Kara <jack@suse.cz>
System files in UDF filesystem have link count 0. To not confuse VFS we
fudge the link count to be 1 when reading such inodes however we forget
to restore the link count of 0 when writing such inodes. Fix that.
CC: stable@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
When write to inline file fails (or happens only partly), we still
updated length of inline data as if the whole write succeeded. Fix the
update of length of inline data to happen only if the write succeeds.
Reported-by: syzbot+0937935b993956ba28ab@syzkaller.appspotmail.com
CC: stable@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
There is a spelling mistake in a udf_err message. Fix it.
Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Message-Id: <20221230231452.5821-1-colin.i.king@gmail.com>
kmap_atomic() is deprecated in favor of kmap_local_page(). Therefore,
replace kmap_atomic() with kmap_local_page().
kmap_atomic() is implemented like a kmap_local_page() which also disables
page-faults and preemption (the latter only for !PREEMPT_RT kernels).
However, the code within the mapping and un-mapping in ext2_make_empty()
does not depend on the above-mentioned side effects.
Therefore, a mere replacement of the old API with the new one is all it
is required (i.e., there is no need to explicitly add any calls to
pagefault_disable() and/or preempt_disable()).
Suggested-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Message-Id: <20221231174205.8492-1-fmdefrancesco@gmail.com>
When rounding the last extent to blocksize in inode_getblk() we forgot
to update also i_lenExtents to match the new extent length. This
inconsistency can later confuse some assertion checks.
Signed-off-by: Jan Kara <jack@suse.cz>
When expanding file for a write into a hole, we were not updating total
length of inode's extents properly. Move the update of i_lenExtents into
udf_do_extend_file() so that both expanding of file by truncate and
expanding of file by writing beyond EOF properly update the length of
extents. As a bonus, we also correctly update the length of extents when
only part of extents can be written.
Signed-off-by: Jan Kara <jack@suse.cz>
Currently we allocate name buffer in directory iterators (struct
udf_fileident_iter) on stack. These structures are relatively large
(some 360 bytes on 64-bit architectures). For udf_rename() which needs
to keep three of these structures in parallel the stack usage becomes
rather heavy - 1536 bytes in total. Allocate the name buffer in the
iterator from heap to avoid excessive stack usage.
Link: https://lore.kernel.org/all/202212200558.lK9x1KW0-lkp@intel.com
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Jan Kara <jack@suse.cz>
When adding extent to a file fails, so far we've silently squelshed the
error. Make sure to propagate it up properly.
Signed-off-by: Jan Kara <jack@suse.cz>
When adding extent describing symlink data fails, make sure to handle
the error properly, propagate it up and free the already allocated
block.
Signed-off-by: Jan Kara <jack@suse.cz>
When there is an error when adding extent to the directory to expand it,
make sure to propagate the error up properly. This is not expected to
happen currently but let's make the code more futureproof.
Signed-off-by: Jan Kara <jack@suse.cz>
When merging very long extents we try to push as much length as possible
to the first extent. However this is unnecessarily complicated and not
really worth the trouble. Furthermore there was a bug in the logic
resulting in corrupting extents in the file as syzbot reproducer shows.
So just don't bother with the merging of extents that are too long
together.
CC: stable@vger.kernel.org
Reported-by: syzbot+60f291a24acecb3c2bd5@syzkaller.appspotmail.com
Signed-off-by: Jan Kara <jack@suse.cz>
When a file expansion failed because we didn't have enough space for
indirect extents make sure we truncate extents created so far so that we
don't leave extents beyond EOF.
CC: stable@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
Jan Kara