- 19 Nov, 2019 3 commits
-
-
Theodore Ts'o authored
-
Theodore Ts'o authored
If the file system is corrupted such that a file's i_links_count is too small, then it's possible that when unlinking that file, i_nlink will already be zero. Previously we were working around this kind of corruption by forcing i_nlink to one; but we were doing this before trying to delete the directory entry --- and if the file system is corrupted enough that ext4_delete_entry() fails, then we exit with i_nlink elevated, and this causes the orphan inode list handling to be FUBAR'ed, such that when we unmount the file system, the orphan inode list can get corrupted. A better way to fix this is to simply skip trying to call drop_nlink() if i_nlink is already zero, thus moving the check to the place where it makes the most sense. https://bugzilla.kernel.org/show_bug.cgi?id=205433 Link: https://lore.kernel.org/r/20191112032903.8828-1-tytso@mit.eduSigned-off-by:
Theodore Ts'o <tytso@mit.edu> Cc: stable@kernel.org Reviewed-by:
Andreas Dilger <adilger@dilger.ca>
-
Theodore Ts'o authored
It's possible to specify a non-zero s_want_extra_isize via debugging option, and this can cause bad things(tm) to happen when using a file system with an inode size of 128 bytes. Add better checking when the file system is mounted, as well as when we are actually doing the trying to do the inode expansion. Link: https://lore.kernel.org/r/20191110121510.GH23325@mit.edu Reported-by: syzbot+f8d6f8386ceacdbfff57@syzkaller.appspotmail.com Reported-by: syzbot+33d7ea72e47de3bdf4e1@syzkaller.appspotmail.com Reported-by: syzbot+44b6763edfc17144296f@syzkaller.appspotmail.com Signed-off-by:
-
- 15 Nov, 2019 5 commits
-
-
Jan Kara authored
The helper jbd2_handle_buffer_credits() doesn't correctly handle reserved handles which can lead to crashes. Fix it getting of journal pointer to work for reserved handles as well. Fixes: a9a8344e ("ext4, jbd2: Provide accessor function for handle credits") Reported-by:
Eric Biggers <ebiggers@kernel.org> Signed-off-by:
Jan Kara <jack@suse.cz> Link: https://lore.kernel.org/r/20191115102210.29445-1-jack@suse.czSigned-off-by:
-
yangerkun authored
No need to wait for any commit once the page is fully truncated. Besides, it may confuse e.g. concurrent ext4_writepage() with the page still be dirty (will be cleared by truncate_pagecache() in ext4_setattr()) but buffers has been freed; and then trigger a bug show as below: [ 26.057508] ------------[ cut here ]------------ [ 26.058531] kernel BUG at fs/ext4/inode.c:2134! ... [ 26.088130] Call trace: [ 26.088695] ext4_writepage+0x914/0xb28 [ 26.089541] writeout.isra.4+0x1b4/0x2b8 [ 26.090409] move_to_new_page+0x3b0/0x568 [ 26.091338] __unmap_and_move+0x648/0x988 [ 26.092241] unmap_and_move+0x48c/0xbb8 [ 26.093096] migrate_pages+0x220/0xb28 [ 26.093945] kernel_mbind+0x828/0xa18 [ 26.094791] __arm64_sys_mbind+0xc8/0x138 [ 26.095716] el0_svc_common+0x190/0x490 [ 26.096571] el0_svc_handler+0x60/0xd0 [ 26.097423] el0_svc+0x8/0xc Run the procedure (generate by syzkaller) parallel with ext3. void main() { int fd, fd1, ret; void *addr; size_t length = 4096; int flags; off_t offset = 0; char *str = "12345"; fd = open("a", O_RDWR | O_CREAT); assert(fd >= 0); /* Truncate to 4k */ ret = ftruncate(fd, length); assert(ret == 0); /* Journal data mode */ flags = 0xc00f; ret = ioctl(fd, _IOW('f', 2, long), &flags); assert(ret == 0); /* Truncate to 0 */ fd1 = open("a", O_TRUNC | O_NOATIME); assert(fd1 >= 0); addr = mmap(NULL, length, PROT_WRITE | PROT_READ, MAP_SHARED, fd, offset); assert(addr != (void *)-1); memcpy(addr, str, 5); mbind(addr, length, 0, 0, 0, MPOL_MF_MOVE); } And the bug will be triggered once we seen the below order. reproduce1 reproduce2 ... | ... truncate to 4k | change to journal data mode | | memcpy(set page dirty) truncate to 0: | ext4_setattr: | ... | ext4_wait_for_tail_page_commit | | mbind(trigger bug) truncate_pagecache(clean dirty)| ... ... | mbind will call ext4_writepage() since the page still be dirty, and then report the bug since the buffers has been free. Fix it by return directly once offset equals to 0 which means the page has been fully truncated. Reported-by:Hulk Robot <hulkci@huawei.com> Signed-off-by:
yangerkun <yangerkun@huawei.com> Link: https://lore.kernel.org/r/20190919063508.1045-1-yangerkun@huawei.comReviewed-by:
-
Gao Xiang authored
Similar to [1] [2], bio_alloc with __GFP_DIRECT_RECLAIM flags guarantees bio allocation under some given restrictions, as stated in block/bio.c and fs/direct-io.c So here it's ok to not check for NULL value from bio_alloc(). [1] https://lore.kernel.org/r/20191030035518.65477-1-gaoxiang25@huawei.com [2] https://lore.kernel.org/r/20190830162812.GA10694@infradead.org Cc: Theodore Ts'o <tytso@mit.edu> Cc: Andreas Dilger <adilger.kernel@dilger.ca> Cc: Ritesh Harjani <riteshh@linux.ibm.com> Reviewed-by:
Chao Yu <yuchao0@huawei.com> Signed-off-by:
Gao Xiang <gaoxiang25@huawei.com> Link: https://lore.kernel.org/r/20191031092315.139267-1-gaoxiang25@huawei.comSigned-off-by:
-
Chengguang Xu authored
Now the checks in ext4_get_next_id() and dquot_get_next_id() are almost the same, so just call dquot_get_next_id() instead of ext4_get_next_id(). Signed-off-by:
Chengguang Xu <cgxu519@mykernel.net> Link: https://lore.kernel.org/r/20191006103028.31299-1-cgxu519@mykernel.netSigned-off-by:
-
Jan Kara authored
Commit 8fcc3a58 ("ext4: rework reserved cluster accounting when invalidating pages") moved freeing of delayed allocation reservations from dirty page invalidation time to time when we evict corresponding status extent from extent status tree. For inodes which don't have any blocks allocated this may actually happen only in ext4_clear_blocks() which is after we've dropped references to quota structures from the inode. Thus reservation of quota leaked. Fix the problem by clearing quota information from the inode only after evicting extent status tree in ext4_clear_inode(). Link: https://lore.kernel.org/r/20191108115420.GI20863@quack2.suse.czReported-by:
Konstantin Khlebnikov <khlebnikov@yandex-team.ru> Fixes: 8fcc3a58 ("ext4: rework reserved cluster accounting when invalidating pages") Signed-off-by:
-
- 14 Nov, 2019 3 commits
-
-
Olof Johansson authored
Commit c33fbe8f ("ext4: Enable blocksize < pagesize for dioread_nolock") removed the only user of 'sbi' outside of the ifdef, so it caused a new warning: fs/ext4/super.c:2068:23: warning: unused variable 'sbi' [-Wunused-variable] Fixes: c33fbe8f ("ext4: Enable blocksize < pagesize for dioread_nolock") Signed-off-by:
Olof Johansson <olof@lixom.net> Link: https://lore.kernel.org/r/20191111022523.34256-1-olof@lixom.netSigned-off-by:
Ritesh Harjani <riteshh@linux.ibm.com>
-
Chandan Rajendra authored
Now that we have the code to support encryption for subpage-sized blocks, this commit removes the conditional check in filesystem mount code. The commit also changes the support statement in Documentation/filesystems/fscrypt.rst to reflect the fact that encryption on filesystems with blocksize less than page size now works. [EB: Tested with 'gce-xfstests -c ext4/encrypt_1k -g auto', using the new "encrypt_1k" config I created. All tests pass except for those that already fail or are excluded with the encrypt or 1k configs, and 2 tests that try to create 1023-byte symlinks which fails since encrypted symlinks are limited to blocksize-3 bytes. Also ran the dedicated encryption tests using 'kvm-xfstests -c ext4/1k -g encrypt'; all pass, including the on-disk ciphertext verification tests.] Signed-off-by:
Chandan Rajendra <chandan@linux.ibm.com> Signed-off-by:
Eric Biggers <ebiggers@google.com> Link: https://lore.kernel.org/r/20191023033312.361355-3-ebiggers@kernel.orgSigned-off-by:
-
Eric Biggers authored
After each filesystem block (as represented by a buffer_head) has been read from disk by block_read_full_page(), decrypt it if needed. The decryption is done on the fscrypt_read_workqueue. This is the final change needed to support ext4 encryption with blocksize != PAGE_SIZE, and it's a fairly small change now that CONFIG_FS_ENCRYPTION is a bool and fs/crypto/ exposes functions to decrypt individual blocks and to enqueue work on the fscrypt workqueue. Don't try to add fs-verity support yet, as the fs/verity/ support layer isn't ready for sub-page blocks yet. Just add fscrypt support for now. Almost all the new code is compiled away when CONFIG_FS_ENCRYPTION=n. Cc: Chandan Rajendra <chandan@linux.ibm.com> Signed-off-by:
-
- 11 Nov, 2019 1 commit
-
-
Ritesh Harjani authored
This patch adds the error handling in case of any memory allocation failure for io_end_vec. This was missing in original patch series which enables dioread_nolock for blocksize < pagesize. Fixes: c8cc8816 ("ext4: Add support for blocksize < pagesize in dioread_nolock") Reported-by:
Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by:
-
- 05 Nov, 2019 28 commits
-
-
Theodore Ts'o authored
-
Theodore Ts'o authored
-
Jan Kara authored
Currently we reserve j_max_transaction_buffers / 32 for transaction descriptor blocks. Now that revoke descriptors are accounted for separately this estimate is unnecessarily high and we can actually compute much tighter estimate. In the common case of 32k journal blocks and 4k blocksize this actually reduces the amount of reserved descriptor blocks from 256 to ~25 which allows us to fit more real data into a transaction. Signed-off-by:
-
Jan Kara authored
Provide trace event for handle restarts to ease debugging. Reviewed-by:
-
Jan Kara authored
So far we have reserved only relatively high fixed amount of revoke credits for each transaction. We over-reserved by large amount for most cases but when freeing large directories or files with data journalling, the fixed amount is not enough. In fact the worst case estimate is inconveniently large (maximum extent size) for freeing of one extent. We fix this by doing proper estimate of the amount of blocks that need to be revoked when removing blocks from the inode due to truncate or hole punching and otherwise reserve just a small amount of revoke credits for each transaction to accommodate freeing of xattrs block or so. Signed-off-by:
-
Jan Kara authored
Make checking of available credits in jbd2_journal_dirty_metadata() more strict. There should be always enough credits in the handle to write all potential revoke descriptors. Also we warn in case there are not enough credits since this is a bug in the filesystem. Reviewed-by:
-
Jan Kara authored
The credit counter now contains both buffer and revoke descriptor block credits. Rename to counter to h_total_credits to reflect that. No functional change. Reviewed-by:
-
Jan Kara authored
Extend functions for starting, extending, and restarting transaction handles to take number of revoke records handle must be able to accommodate. These functions then make sure transaction has enough credits to be able to store resulting revoke descriptor blocks. Also revoke code tracks number of revoke records created by a handle to catch situation where some place didn't reserve enough space for revoke records. Similarly to standard transaction credits, space for unused reserved revoke records is released when the handle is stopped. On the ext4 side we currently take a simplistic approach of reserving space for 1024 revoke records for any transaction. This grows amount of credits reserved for each handle only by a few and is enough for any normal workload so that we don't hit warnings in jbd2. We will refine the logic in following commits. Signed-off-by:
-
Jan Kara authored
The function is now just a trivial wrapper returning journal->j_max_transaction_buffers. Drop it. Reviewed-by:
-
Jan Kara authored
Currently, journal descriptor blocks were not accounted in transaction->t_outstanding_credits and we were just leaving some slack space in the journal for them (in jbd2_log_space_left() and jbd2_space_needed()). This is making proper accounting (and reservation we want to add) of descriptor blocks difficult so switch to accounting descriptor blocks in transaction->t_outstanding_credits and just reserve the same amount of credits in t_outstanding credits for journal descriptor blocks when creating transaction. Signed-off-by:
-
Jan Kara authored
jbd2__journal_restart() has quite some code that is common with jbd2_journal_stop(). Factor this functionality into stop_this_handle() helper and use it from both functions. Note that this also drops t_handle_lock protection from jbd2__journal_restart() as jbd2_journal_stop() does the same thing without it. Signed-off-by:
-
Jan Kara authored
When we drop last handle from a transaction and journal->j_barrier_count > 0, jbd2_journal_stop() wakes up journal->j_wait_transaction_locked wait queue. This looks pointless - wait for outstanding handles always happens on journal->j_wait_updates waitqueue. journal->j_wait_transaction_locked is used to wait for transaction state changes and by start_this_handle() for waiting until journal->j_barrier_count drops to 0. The first case is clearly irrelevant here since only jbd2 thread changes transaction state. The second case looks related but jbd2_journal_unlock_updates() is responsible for the wakeup in this case. So just drop the wakeup. Reviewed-by:
-
Jan Kara authored
If a transaction is larger than journal->j_max_transaction_buffers, that is a bug and not a trigger for transaction commit. Also the very next attempt to start new handle will start transaction commit anyway. So just remove the pointless check. Arguably, we could start transaction commit whenever the transaction size is *close* to journal->j_max_transaction_buffers. This has a potential to reduce latency of the next jbd2_journal_start() at the cost of somewhat smaller transactions. However for this to have any effect, it would mean that there isn't someone already waiting in jbd2_journal_start() which means metadata load for the fs is pretty light anyway so probably this optimization is not worth it. Reviewed-by:
-
Jan Kara authored
Move code in jbd2_journal_stop() around a bit. It removes some unnecessary code duplication and will make factoring out parts common with jbd2__journal_restart() easier. Reviewed-by:
-
Jan Kara authored
jbd2 statistics counting number of blocks logged in a transaction was wrong. It didn't count the commit block and more importantly it didn't count revoke descriptor blocks. Make sure these get properly counted. Reviewed-by:
-
Jan Kara authored
Use the jbd2 accessor function for h_buffer_credits. Reviewed-by:
-
Jan Kara authored
Provide accessor function to get number of credits available in a handle and use it from ext4. Later, computation of available credits won't be so straightforward. Reviewed-by:
-
Jan Kara authored
Provide ext4_journal_ensure_credits_fn() function to ensure transaction has given amount of credits and call helper function to prepare for restarting a transaction. This allows to remove some boilerplate code from various places, add proper error handling for the case where transaction extension or restart fails, and reduces following changes needed for proper revoke record reservation tracking. Signed-off-by:
-
Jan Kara authored
Error cleanup path in ext4_alloc_branch() calls ext4_forget() on freshly allocated indirect blocks with 'metadata' set to 1. This results in generating revoke records for these blocks. However this is unnecessary as the freed blocks are only allocated in the current transaction and thus they will never be journalled. Make this cleanup path similar to e.g. cleanup in ext4_splice_branch() and use ext4_free_blocks() to handle block forgetting by passing EXT4_FREE_BLOCKS_FORGET and not EXT4_FREE_BLOCKS_METADATA to ext4_free_blocks(). This also allows allocating transaction not to reserve any credits for revoke records. Reviewed-by:
-
Jan Kara authored
Use ext4 helper ext4_journal_extend() instead of opencoding it in ext4_try_to_expand_extra_isize(). Reviewed-by:
-
Jan Kara authored
Similarly to directories, EA inodes do only journalled modifications to their data. Change ext4_should_journal_data() to return true for them so that we don't have to special-case them during truncate. Signed-off-by:
-
Jan Kara authored
Estimate for the number of credits needed for final freeing of inode in ext4_evict_inode() was to small. We may modify 4 blocks (inode & sb for orphan deletion, bitmap & group descriptor for inode freeing) and not just 3. [ Fixed minor whitespace nit. -- TYT ] Fixes: e50e5129 ("ext4: xattr-in-inode support") CC: stable@vger.kernel.org Signed-off-by:
-
Matthew Bobrowski authored
This patch introduces a new direct I/O write path which makes use of the iomap infrastructure. All direct I/O writes are now passed from the ->write_iter() callback through to the new direct I/O handler ext4_dio_write_iter(). This function is responsible for calling into the iomap infrastructure via iomap_dio_rw(). Code snippets from the existing direct I/O write code within ext4_file_write_iter() such as, checking whether the I/O request is unaligned asynchronous I/O, or whether the write will result in an overwrite have effectively been moved out and into the new direct I/O ->write_iter() handler. The block mapping flags that are eventually passed down to ext4_map_blocks() from the *_get_block_*() suite of routines have been taken out and introduced within ext4_iomap_alloc(). For inode extension cases, ext4_handle_inode_extension() is effectively the function responsible for performing such metadata updates. This is called after iomap_dio_rw() has returned so that we can safely determine whether we need to potentially truncate any allocated blocks that may have been prepared for this direct I/O write. We don't perform the inode extension, or truncate operations from the ->end_io() handler as we don't have the original I/O 'length' available there. The ->end_io() however is responsible fo converting allocated unwritten extents to written extents. In the instance of a short write, we fallback and complete the remainder of the I/O using buffered I/O via ext4_buffered_write_iter(). The existing buffer_head direct I/O implementation has been removed as it's now redundant. [ Fix up ext4_dio_write_iter() per Jan's comments at https://lore.kernel.org/r/20191105135932.GN22379@quack2.suse.cz -- TYT ] Signed-off-by:
Matthew Bobrowski <mbobrowski@mbobrowski.org> Reviewed-by:
-
Jan Kara authored
When ext4_mkdir(), ext4_symlink(), ext4_create(), or ext4_mknod() fail to add entry into directory, it ends up dropping freshly created inode under the running transaction and thus inode truncation happens under that transaction. That breaks assumptions that evict() does not get called from a transaction context and at least in ext4_symlink() case it can result in inode eviction deadlocking in inode_wait_for_writeback() when flush worker finds symlink inode, starts to write it back and blocks on starting a transaction. So change the code in ext4_mkdir() and ext4_add_nondir() to drop inode reference only after the transaction is stopped. We also have to add inode to the orphan list in that case as otherwise the inode would get leaked in case we crash before inode deletion is committed. CC: stable@vger.kernel.org Signed-off-by:
-
Jan Kara authored
Every caller of ext4_add_nondir() marks handle as sync if directory has DIRSYNC set. Move this marking to ext4_add_nondir() so reduce some duplication. Signed-off-by:
-
Jan Kara authored
With 32-bit block numbers, we don't allocate the array for journal buffer heads large enough for corresponding descriptor tags to fill the descriptor block. Thus we end up writing out half-full descriptor blocks to the journal unnecessarily growing the transaction. Fix the logic to allocate the array large enough. Signed-off-by:
-
Jan Kara authored
jbd2_journal_next_log_block() does not look at transaction->t_outstanding_credits. Remove the misleading comment. Reviewed-by:
-
Jan Kara authored
When number of free space in the journal is very low, the arithmetic in jbd2_log_space_left() could underflow resulting in very high number of free blocks and thus triggering assertion failure in transaction commit code complaining there's not enough space in the journal: J_ASSERT(journal->j_free > 1); Properly check for the low number of free blocks. CC: stable@vger.kernel.org Reviewed-by:
-
