Re: [syzbot] [f2fs?] KASAN: slab-use-after-free Read in kill_f2fs_super

2024-01-12 Thread syzbot
syzbot has bisected this issue to:

commit 275dca4630c165edea9abe27113766bc1173f878
Author: Eric Biggers 
Date:   Wed Dec 27 17:14:28 2023 +

f2fs: move release of block devices to after kill_block_super()

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=16071613e8
start commit:   70d201a40823 Merge tag 'f2fs-for-6.8-rc1' of git://git.ker..
git tree:   upstream
final oops: https://syzkaller.appspot.com/x/report.txt?x=15071613e8
console output: https://syzkaller.appspot.com/x/log.txt?x=11071613e8
kernel config:  https://syzkaller.appspot.com/x/.config?x=4607bc15d1c4bb90
dashboard link: https://syzkaller.appspot.com/bug?extid=8f477ac014ff5b32d81f
syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=112b660be8
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14c1df5de8

Reported-by: syzbot+8f477ac014ff5b32d...@syzkaller.appspotmail.com
Fixes: 275dca4630c1 ("f2fs: move release of block devices to after 
kill_block_super()")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection



[f2fs-dev] [PATCH v5 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP

2024-01-12 Thread Chao Yu
If data block in compressed cluster is not persisted with metadata
during checkpoint, after SPOR, the data may be corrupted, let's
guarantee to write compressed page by checkpoint.

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Reviewed-by: Daeho Jeong 
Signed-off-by: Chao Yu 
---
 fs/f2fs/compress.c |  4 +++-
 fs/f2fs/data.c | 17 +
 fs/f2fs/f2fs.h |  4 +++-
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index c5a4364c4482..9940b7886e5d 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1418,6 +1418,8 @@ void f2fs_compress_write_end_io(struct bio *bio, struct 
page *page)
struct f2fs_sb_info *sbi = bio->bi_private;
struct compress_io_ctx *cic =
(struct compress_io_ctx *)page_private(page);
+   enum count_type type = WB_DATA_TYPE(page,
+   f2fs_is_compressed_page(page));
int i;
 
if (unlikely(bio->bi_status))
@@ -1425,7 +1427,7 @@ void f2fs_compress_write_end_io(struct bio *bio, struct 
page *page)
 
f2fs_compress_free_page(page);
 
-   dec_page_count(sbi, F2FS_WB_DATA);
+   dec_page_count(sbi, type);
 
if (atomic_dec_return(>pending_pages))
return;
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index dce8defdf4c7..81f9e2cc49e2 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -48,7 +48,7 @@ void f2fs_destroy_bioset(void)
bioset_exit(_bioset);
 }
 
-static bool __is_cp_guaranteed(struct page *page)
+bool f2fs_is_cp_guaranteed(struct page *page)
 {
struct address_space *mapping = page->mapping;
struct inode *inode;
@@ -65,8 +65,6 @@ static bool __is_cp_guaranteed(struct page *page)
S_ISDIR(inode->i_mode))
return true;
 
-   if (f2fs_is_compressed_page(page))
-   return false;
if ((S_ISREG(inode->i_mode) && IS_NOQUOTA(inode)) ||
page_private_gcing(page))
return true;
@@ -338,7 +336,7 @@ static void f2fs_write_end_io(struct bio *bio)
 
bio_for_each_segment_all(bvec, bio, iter_all) {
struct page *page = bvec->bv_page;
-   enum count_type type = WB_DATA_TYPE(page);
+   enum count_type type = WB_DATA_TYPE(page, false);
 
if (page_private_dummy(page)) {
clear_page_private_dummy(page);
@@ -762,7 +760,7 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
wbc_account_cgroup_owner(fio->io_wbc, fio->page, PAGE_SIZE);
 
inc_page_count(fio->sbi, is_read_io(fio->op) ?
-   __read_io_type(page) : WB_DATA_TYPE(fio->page));
+   __read_io_type(page) : WB_DATA_TYPE(fio->page, false));
 
if (is_read_io(bio_op(bio)))
f2fs_submit_read_bio(fio->sbi, bio, fio->type);
@@ -973,7 +971,7 @@ int f2fs_merge_page_bio(struct f2fs_io_info *fio)
if (fio->io_wbc)
wbc_account_cgroup_owner(fio->io_wbc, fio->page, PAGE_SIZE);
 
-   inc_page_count(fio->sbi, WB_DATA_TYPE(page));
+   inc_page_count(fio->sbi, WB_DATA_TYPE(page, false));
 
*fio->last_block = fio->new_blkaddr;
*fio->bio = bio;
@@ -1007,6 +1005,7 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
enum page_type btype = PAGE_TYPE_OF_BIO(fio->type);
struct f2fs_bio_info *io = sbi->write_io[btype] + fio->temp;
struct page *bio_page;
+   enum count_type type;
 
f2fs_bug_on(sbi, is_read_io(fio->op));
 
@@ -1046,7 +1045,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
/* set submitted = true as a return value */
fio->submitted = 1;
 
-   inc_page_count(sbi, WB_DATA_TYPE(bio_page));
+   type = WB_DATA_TYPE(bio_page, fio->compressed_page);
+   inc_page_count(sbi, type);
 
if (io->bio &&
(!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
@@ -1059,7 +1059,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
if (F2FS_IO_ALIGNED(sbi) &&
(fio->type == DATA || fio->type == NODE) &&
fio->new_blkaddr & F2FS_IO_SIZE_MASK(sbi)) {
-   dec_page_count(sbi, WB_DATA_TYPE(bio_page));
+   dec_page_count(sbi, WB_DATA_TYPE(bio_page,
+   fio->compressed_page));
fio->retry = 1;
goto skip;
}
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 65294e3b0bef..50f3d546ded8 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1080,7 +1080,8 @@ struct f2fs_sm_info {
  * f2fs monitors the number of several block types such as on-writeback,
  * dirty dentry blocks, dirty node blocks, and dirty meta blocks.
  */
-#define WB_DATA_TYPE(p)(__is_cp_guaranteed(p) ? F2FS_WB_CP_DATA : 
F2FS_WB_DATA)
+#define 

[f2fs-dev] [PATCH v5 2/6] f2fs: compress: fix to cover normal cluster write with cp_rwsem

2024-01-12 Thread Chao Yu
When we overwrite compressed cluster w/ normal cluster, we should
not unlock cp_rwsem during f2fs_write_raw_pages(), otherwise data
will be corrupted if partial blocks were persisted before CP & SPOR,
due to cluster metadata wasn't updated atomically.

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Reviewed-by: Daeho Jeong 
Signed-off-by: Chao Yu 
---
v5:
- include cleanup from Jaegeuk.
 fs/f2fs/compress.c | 27 ++-
 fs/f2fs/data.c |  3 ++-
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 9940b7886e5d..06e9bbfba1f1 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1443,12 +1443,14 @@ void f2fs_compress_write_end_io(struct bio *bio, struct 
page *page)
 }
 
 static int f2fs_write_raw_pages(struct compress_ctx *cc,
-   int *submitted,
+   int *submitted_p,
struct writeback_control *wbc,
enum iostat_type io_type)
 {
struct address_space *mapping = cc->inode->i_mapping;
-   int _submitted, compr_blocks, ret, i;
+   struct f2fs_sb_info *sbi = F2FS_M_SB(mapping);
+   int submitted, compr_blocks, i;
+   int ret = 0;
 
compr_blocks = f2fs_compressed_blocks(cc);
 
@@ -1463,6 +1465,10 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
if (compr_blocks < 0)
return compr_blocks;
 
+   /* overwrite compressed cluster w/ normal cluster */
+   if (compr_blocks > 0)
+   f2fs_lock_op(sbi);
+
for (i = 0; i < cc->cluster_size; i++) {
if (!cc->rpages[i])
continue;
@@ -1487,7 +1493,7 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
if (!clear_page_dirty_for_io(cc->rpages[i]))
goto continue_unlock;
 
-   ret = f2fs_write_single_data_page(cc->rpages[i], &_submitted,
+   ret = f2fs_write_single_data_page(cc->rpages[i], ,
NULL, NULL, wbc, io_type,
compr_blocks, false);
if (ret) {
@@ -1495,26 +1501,29 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
unlock_page(cc->rpages[i]);
ret = 0;
} else if (ret == -EAGAIN) {
+   ret = 0;
/*
 * for quota file, just redirty left pages to
 * avoid deadlock caused by cluster update race
 * from foreground operation.
 */
if (IS_NOQUOTA(cc->inode))
-   return 0;
-   ret = 0;
+   goto out;
f2fs_io_schedule_timeout(DEFAULT_IO_TIMEOUT);
goto retry_write;
}
-   return ret;
+   goto out;
}
 
-   *submitted += _submitted;
+   *submitted_p += submitted;
}
 
-   f2fs_balance_fs(F2FS_M_SB(mapping), true);
+out:
+   if (compr_blocks > 0)
+   f2fs_unlock_op(sbi);
 
-   return 0;
+   f2fs_balance_fs(sbi, true);
+   return ret;
 }
 
 int f2fs_write_multi_pages(struct compress_ctx *cc,
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 81f9e2cc49e2..b171a9980f6a 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2839,7 +2839,7 @@ int f2fs_write_single_data_page(struct page *page, int 
*submitted,
.encrypted_page = NULL,
.submitted = 0,
.compr_blocks = compr_blocks,
-   .need_lock = LOCK_RETRY,
+   .need_lock = compr_blocks ? LOCK_DONE : LOCK_RETRY,
.post_read = f2fs_post_read_required(inode) ? 1 : 0,
.io_type = io_type,
.io_wbc = wbc,
@@ -2920,6 +2920,7 @@ int f2fs_write_single_data_page(struct page *page, int 
*submitted,
if (err == -EAGAIN) {
err = f2fs_do_write_data_page();
if (err == -EAGAIN) {
+   f2fs_bug_on(sbi, compr_blocks);
fio.need_lock = LOCK_REQ;
err = f2fs_do_write_data_page();
}
-- 
2.40.1



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH v5 3/6] f2fs: compress: fix to check unreleased compressed cluster

2024-01-12 Thread Chao Yu
From: Sheng Yong 

Compressed cluster may not be released due to we can fail in
release_compress_blocks(), fix to handle reserved compressed
cluster correctly in reserve_compress_blocks().

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Sheng Yong 
Signed-off-by: Chao Yu 
---
v5:
- fix wrong condition pointed out by Daeho.
 fs/f2fs/file.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 026d05a7edd8..4aef2310395f 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3624,7 +3624,13 @@ static int reserve_compress_blocks(struct dnode_of_data 
*dn, pgoff_t count)
goto next;
}
 
-   if (__is_valid_data_blkaddr(blkaddr)) {
+   /*
+* compressed cluster was not released due to it
+* fails in release_compress_blocks(), so NEW_ADDR
+* is a possible case.
+*/
+   if (blkaddr == NEW_ADDR ||
+   __is_valid_data_blkaddr(blkaddr)) {
compr_blocks++;
continue;
}
@@ -3633,6 +3639,11 @@ static int reserve_compress_blocks(struct dnode_of_data 
*dn, pgoff_t count)
}
 
reserved = cluster_size - compr_blocks;
+
+   /* for the case all blocks in cluster were reserved */
+   if (reserved == 1)
+   goto next;
+
ret = inc_valid_block_count(sbi, dn->inode, );
if (ret)
return ret;
-- 
2.40.1



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH v5 5/6] f2fs: fix to remove unnecessary f2fs_bug_on() to avoid panic

2024-01-12 Thread Chao Yu
verify_blkaddr() will trigger panic once we inject fault into
f2fs_is_valid_blkaddr(), fix to remove this unnecessary f2fs_bug_on().

Fixes: 18792e64c86d ("f2fs: support fault injection for 
f2fs_is_valid_blkaddr()")
Reviewed-by: Daeho Jeong 
Signed-off-by: Chao Yu 
---
 fs/f2fs/f2fs.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 69e71460a950..ab710bb6d8b3 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3470,11 +3470,9 @@ bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
 static inline void verify_blkaddr(struct f2fs_sb_info *sbi,
block_t blkaddr, int type)
 {
-   if (!f2fs_is_valid_blkaddr(sbi, blkaddr, type)) {
+   if (!f2fs_is_valid_blkaddr(sbi, blkaddr, type))
f2fs_err(sbi, "invalid blkaddr: %u, type: %d, run fsck to fix.",
 blkaddr, type);
-   f2fs_bug_on(sbi, 1);
-   }
 }
 
 static inline bool __is_valid_data_blkaddr(block_t blkaddr)
-- 
2.40.1



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH v5 6/6] f2fs: introduce FAULT_BLKADDR_CONSISTENCE

2024-01-12 Thread Chao Yu
We will encounter below inconsistent status when FAULT_BLKADDR type
fault injection is on.

Info: checkpoint state = d6 :  nat_bits crc fsck compacted_summary 
orphan_inodes sudden-power-off
[ASSERT] (fsck_chk_inode_blk:1254)  --> ino: 0x1c100 has i_blocks: 00c0, 
but has 191 blocks
[FIX] (fsck_chk_inode_blk:1260)  --> [0x1c100] i_blocks=0x00c0 -> 0xbf
[FIX] (fsck_chk_inode_blk:1269)  --> [0x1c100] i_compr_blocks=0x0026 -> 0x27
[ASSERT] (fsck_chk_inode_blk:1254)  --> ino: 0x1cadb has i_blocks: 002f, 
but has 46 blocks
[FIX] (fsck_chk_inode_blk:1260)  --> [0x1cadb] i_blocks=0x002f -> 0x2e
[FIX] (fsck_chk_inode_blk:1269)  --> [0x1cadb] i_compr_blocks=0x0011 -> 0x12
[ASSERT] (fsck_chk_inode_blk:1254)  --> ino: 0x1c62c has i_blocks: 0002, 
but has 1 blocks
[FIX] (fsck_chk_inode_blk:1260)  --> [0x1c62c] i_blocks=0x0002 -> 0x1

After we inject fault into f2fs_is_valid_blkaddr() during truncation,
a) it missed to increase @nr_free or @valid_blocks
b) it can cause in blkaddr leak in truncated dnode
Which may cause inconsistent status.

This patch separates FAULT_BLKADDR_CONSISTENCE from FAULT_BLKADDR,
and rename FAULT_BLKADDR to FAULT_BLKADDR_VALIDITY
so that we can:
a) use FAULT_BLKADDR_CONSISTENCE in f2fs_truncate_data_blocks_range()
to simulate inconsistent issue independently, then it can verify fsck
repair flow.
b) FAULT_BLKADDR_VALIDITY fault will not cause any inconsistent status,
we can just use it to check error path handling in kernel side.

Reviewed-by: Daeho Jeong 
Signed-off-by: Chao Yu 
---
 Documentation/ABI/testing/sysfs-fs-f2fs | 47 +
 Documentation/filesystems/f2fs.rst  | 47 +
 fs/f2fs/checkpoint.c| 19 +++---
 fs/f2fs/f2fs.h  |  5 ++-
 fs/f2fs/file.c  |  8 +++--
 fs/f2fs/super.c | 37 +--
 6 files changed, 92 insertions(+), 71 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs 
b/Documentation/ABI/testing/sysfs-fs-f2fs
index 99fa87a43926..48c135e24eb5 100644
--- a/Documentation/ABI/testing/sysfs-fs-f2fs
+++ b/Documentation/ABI/testing/sysfs-fs-f2fs
@@ -701,29 +701,30 @@ Description:  Support configuring fault injection 
type, should be
enabled with fault_injection option, fault type value
is shown below, it supports single or combined type.
 
-   ===  ===
-   Type_NameType_Value
-   ===  ===
-   FAULT_KMALLOC0x1
-   FAULT_KVMALLOC   0x2
-   FAULT_PAGE_ALLOC 0x4
-   FAULT_PAGE_GET   0x8
-   FAULT_ALLOC_BIO  0x00010 (obsolete)
-   FAULT_ALLOC_NID  0x00020
-   FAULT_ORPHAN 0x00040
-   FAULT_BLOCK  0x00080
-   FAULT_DIR_DEPTH  0x00100
-   FAULT_EVICT_INODE0x00200
-   FAULT_TRUNCATE   0x00400
-   FAULT_READ_IO0x00800
-   FAULT_CHECKPOINT 0x01000
-   FAULT_DISCARD0x02000
-   FAULT_WRITE_IO   0x04000
-   FAULT_SLAB_ALLOC 0x08000
-   FAULT_DQUOT_INIT 0x1
-   FAULT_LOCK_OP0x2
-   FAULT_BLKADDR0x4
-   ===  ===
+   ===  ===
+   Type_NameType_Value
+   ===  ===
+   FAULT_KMALLOC0x1
+   FAULT_KVMALLOC   0x2
+   FAULT_PAGE_ALLOC 0x4
+   FAULT_PAGE_GET   0x8
+   FAULT_ALLOC_BIO  0x00010 (obsolete)
+   FAULT_ALLOC_NID  0x00020
+   FAULT_ORPHAN 0x00040
+   FAULT_BLOCK  0x00080
+   FAULT_DIR_DEPTH  0x00100
+   FAULT_EVICT_INODE0x00200
+   FAULT_TRUNCATE   0x00400
+   FAULT_READ_IO0x00800
+   FAULT_CHECKPOINT 0x01000
+   FAULT_DISCARD0x02000
+   FAULT_WRITE_IO   0x04000
+   FAULT_SLAB_ALLOC 0x08000
+   FAULT_DQUOT_INIT 0x1
+   FAULT_LOCK_OP0x2
+   FAULT_BLKADDR_VALIDITY   

[f2fs-dev] [PATCH v5 4/6] f2fs: compress: fix to avoid inconsistence bewteen i_blocks and dnode

2024-01-12 Thread Chao Yu
In reserve_compress_blocks(), we update blkaddrs of dnode in prior to
inc_valid_block_count(), it may cause inconsistent status bewteen
i_blocks and blkaddrs once inc_valid_block_count() fails.

To fix this issue, it needs to reverse their invoking order.

Fixes: c75488fb4d82 ("f2fs: introduce F2FS_IOC_RESERVE_COMPRESS_BLOCKS")
Reviewed-by: Daeho Jeong 
Signed-off-by: Chao Yu 
---
 fs/f2fs/data.c|  5 +++--
 fs/f2fs/f2fs.h|  7 ++-
 fs/f2fs/file.c| 26 ++
 fs/f2fs/segment.c |  2 +-
 4 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index b171a9980f6a..8d2ace723310 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1219,7 +1219,8 @@ int f2fs_reserve_new_blocks(struct dnode_of_data *dn, 
blkcnt_t count)
 
if (unlikely(is_inode_flag_set(dn->inode, FI_NO_ALLOC)))
return -EPERM;
-   if (unlikely((err = inc_valid_block_count(sbi, dn->inode, 
+   err = inc_valid_block_count(sbi, dn->inode, , true);
+   if (unlikely(err))
return err;
 
trace_f2fs_reserve_new_blocks(dn->inode, dn->nid,
@@ -1476,7 +1477,7 @@ static int __allocate_data_block(struct dnode_of_data 
*dn, int seg_type)
 
dn->data_blkaddr = f2fs_data_blkaddr(dn);
if (dn->data_blkaddr == NULL_ADDR) {
-   err = inc_valid_block_count(sbi, dn->inode, );
+   err = inc_valid_block_count(sbi, dn->inode, , true);
if (unlikely(err))
return err;
}
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 50f3d546ded8..69e71460a950 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2252,7 +2252,7 @@ static inline bool __allow_reserved_blocks(struct 
f2fs_sb_info *sbi,
 
 static inline void f2fs_i_blocks_write(struct inode *, block_t, bool, bool);
 static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
-struct inode *inode, blkcnt_t *count)
+struct inode *inode, blkcnt_t *count, bool 
partial)
 {
blkcnt_t diff = 0, release = 0;
block_t avail_user_block_count;
@@ -2292,6 +2292,11 @@ static inline int inc_valid_block_count(struct 
f2fs_sb_info *sbi,
avail_user_block_count = 0;
}
if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
+   if (!partial) {
+   spin_unlock(>stat_lock);
+   goto enospc;
+   }
+
diff = sbi->total_valid_block_count - avail_user_block_count;
if (diff > *count)
diff = *count;
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 4aef2310395f..a02c530c7e4b 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3614,14 +3614,16 @@ static int reserve_compress_blocks(struct dnode_of_data 
*dn, pgoff_t count)
blkcnt_t reserved;
int ret;
 
-   for (i = 0; i < cluster_size; i++, dn->ofs_in_node++) {
-   blkaddr = f2fs_data_blkaddr(dn);
+   for (i = 0; i < cluster_size; i++) {
+   blkaddr = data_blkaddr(dn->inode, dn->node_page,
+   dn->ofs_in_node + i);
 
if (i == 0) {
-   if (blkaddr == COMPRESS_ADDR)
-   continue;
-   dn->ofs_in_node += cluster_size;
-   goto next;
+   if (blkaddr != COMPRESS_ADDR) {
+   dn->ofs_in_node += cluster_size;
+   goto next;
+   }
+   continue;
}
 
/*
@@ -3634,8 +3636,6 @@ static int reserve_compress_blocks(struct dnode_of_data 
*dn, pgoff_t count)
compr_blocks++;
continue;
}
-
-   f2fs_set_data_blkaddr(dn, NEW_ADDR);
}
 
reserved = cluster_size - compr_blocks;
@@ -3644,12 +3644,14 @@ static int reserve_compress_blocks(struct dnode_of_data 
*dn, pgoff_t count)
if (reserved == 1)
goto next;
 
-   ret = inc_valid_block_count(sbi, dn->inode, );
-   if (ret)
+   ret = inc_valid_block_count(sbi, dn->inode, , false);
+   if (unlikely(ret))
return ret;
 
-   if (reserved != cluster_size - compr_blocks)
-   return -ENOSPC;
+   for (i = 0; i < cluster_size; i++, dn->ofs_in_node++) {
+   if (f2fs_data_blkaddr(dn) == NULL_ADDR)
+   f2fs_set_data_blkaddr(dn, NEW_ADDR);
+   }
 

Re: [f2fs-dev] [PATCH v4 2/6] f2fs: compress: fix to cover normal cluster write with cp_rwsem

2024-01-12 Thread Chao Yu

Thanks, let me resend v5 w/ blow cleanups.

On 2024/1/13 9:39, Jaegeuk Kim wrote:

Cleaned up a bit:

--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1443,13 +1443,14 @@ void f2fs_compress_write_end_io(struct bio *bio, struct 
page *page)
  }

  static int f2fs_write_raw_pages(struct compress_ctx *cc,
-   int *submitted,
+   int *submitted_p,
 struct writeback_control *wbc,
 enum iostat_type io_type)
  {
 struct address_space *mapping = cc->inode->i_mapping;
 struct f2fs_sb_info *sbi = F2FS_M_SB(mapping);
-   int _submitted, compr_blocks, ret = 0, i;
+   int submitted, compr_blocks, i;
+   int ret = 0;

 compr_blocks = f2fs_compressed_blocks(cc);

@@ -1492,7 +1493,7 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
 if (!clear_page_dirty_for_io(cc->rpages[i]))
 goto continue_unlock;

-   ret = f2fs_write_single_data_page(cc->rpages[i], &_submitted,
+   ret = f2fs_write_single_data_page(cc->rpages[i], ,
 NULL, NULL, wbc, io_type,
 compr_blocks, false);
 if (ret) {
@@ -1514,7 +1515,7 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
 goto out;
 }

-   *submitted += _submitted;
+   *submitted_p += submitted;
 }

  out:

On 01/11, Chao Yu wrote:

When we overwrite compressed cluster w/ normal cluster, we should
not unlock cp_rwsem during f2fs_write_raw_pages(), otherwise data
will be corrupted if partial blocks were persisted before CP & SPOR,
due to cluster metadata wasn't updated atomically.

Fixes: 4c8ff7095bef ("f2fs: support data compression")
Signed-off-by: Chao Yu 
---
  fs/f2fs/compress.c | 20 ++--
  fs/f2fs/data.c |  3 ++-
  2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 9940b7886e5d..bf4cfab67aec 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1448,7 +1448,8 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
enum iostat_type io_type)
  {
struct address_space *mapping = cc->inode->i_mapping;
-   int _submitted, compr_blocks, ret, i;
+   struct f2fs_sb_info *sbi = F2FS_M_SB(mapping);
+   int _submitted, compr_blocks, ret = 0, i;
  
  	compr_blocks = f2fs_compressed_blocks(cc);
  
@@ -1463,6 +1464,10 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,

if (compr_blocks < 0)
return compr_blocks;
  
+	/* overwrite compressed cluster w/ normal cluster */

+   if (compr_blocks > 0)
+   f2fs_lock_op(sbi);
+
for (i = 0; i < cc->cluster_size; i++) {
if (!cc->rpages[i])
continue;
@@ -1495,26 +1500,29 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
unlock_page(cc->rpages[i]);
ret = 0;
} else if (ret == -EAGAIN) {
+   ret = 0;
/*
 * for quota file, just redirty left pages to
 * avoid deadlock caused by cluster update race
 * from foreground operation.
 */
if (IS_NOQUOTA(cc->inode))
-   return 0;
-   ret = 0;
+   goto out;
f2fs_io_schedule_timeout(DEFAULT_IO_TIMEOUT);
goto retry_write;
}
-   return ret;
+   goto out;
}
  
  		*submitted += _submitted;

}
  
-	f2fs_balance_fs(F2FS_M_SB(mapping), true);

+out:
+   if (compr_blocks > 0)
+   f2fs_unlock_op(sbi);
  
-	return 0;

+   f2fs_balance_fs(sbi, true);
+   return ret;
  }
  
  int f2fs_write_multi_pages(struct compress_ctx *cc,

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 81f9e2cc49e2..b171a9980f6a 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2839,7 +2839,7 @@ int f2fs_write_single_data_page(struct page *page, int 
*submitted,
.encrypted_page = NULL,
.submitted = 0,
.compr_blocks = compr_blocks,
-   .need_lock = LOCK_RETRY,
+   .need_lock = compr_blocks ? LOCK_DONE : LOCK_RETRY,
.post_read = f2fs_post_read_required(inode) ? 1 : 0,
.io_type = io_type,
.io_wbc = wbc,
@@ -2920,6 +2920,7 @@ int f2fs_write_single_data_page(struct page 

Re: [f2fs-dev] [PATCH] f2fs: remove unnecessary f2fs_put_page in f2fs_rename

2024-01-12 Thread Chao Yu

On 2024/1/13 1:16, Jaegeuk Kim wrote:

[1] changed the below condition, which made f2fs_put_page() voided.
This patch reapplies the AL's resolution in -next from [2].

-   if (S_ISDIR(old_inode->i_mode)) {
+   if (old_is_dir && old_dir != new_dir) {
 old_dir_entry = f2fs_parent_dir(old_inode, _dir_page);
 if (!old_dir_entry) {
 if (IS_ERR(old_dir_page))

[1] 7deee77b993a ("f2fs: Avoid reading renamed directory if parent does not 
change")
[2] https://lore.kernel.org/all/20231220013402.GW1674809@ZenIV/

Suggested-by: Al Viro 
Signed-off-by: Jaegeuk Kim 


Reviewed-by: Chao Yu 

Thanks,


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [PATCH] f2fs: fix double free of f2fs_sb_info

2024-01-12 Thread Jaegeuk Kim
On 01/12, Eric Biggers wrote:
> On Fri, Jan 12, 2024 at 05:28:31PM -0800, Jaegeuk Kim wrote:
> > On 01/12, Eric Biggers wrote:
> > > On Fri, Jan 12, 2024 at 04:57:47PM -0800, Eric Biggers wrote:
> > > > From: Eric Biggers 
> > > > 
> > > > kill_f2fs_super() is called even if f2fs_fill_super() fails.
> > > > f2fs_fill_super() frees the struct f2fs_sb_info, so it must set
> > > > sb->s_fs_info to NULL to prevent it from being freed again.
> > > > 
> > > > Fixes: 275dca4630c1 ("f2fs: move release of block devices to after 
> > > > kill_block_super()")
> > > > Reported-by: syzbot+8f477ac014ff5b32d...@syzkaller.appspotmail.com
> > > > Closes: 
> > > > https://lore.kernel.org/r/6cb174060ec34...@google.com
> > > > Signed-off-by: Eric Biggers 
> > > 
> > > Jaegeuk, I'd be glad to take this through the fscrypt tree since that's 
> > > where my
> > 
> > Ok, are you heading to push this in -rc1?
> > 
> > > broken commit came from.  But let me know if you want to just take this 
> > > through
> > > the f2fs tree.
> > > 
> 
> Yes, we should get this into -rc1.

Ok, please do so.

> 
> - Eric



Re: [f2fs-dev] [PATCH v4 2/6] f2fs: compress: fix to cover normal cluster write with cp_rwsem

2024-01-12 Thread Jaegeuk Kim
Cleaned up a bit:

--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1443,13 +1443,14 @@ void f2fs_compress_write_end_io(struct bio *bio, struct 
page *page)
 }

 static int f2fs_write_raw_pages(struct compress_ctx *cc,
-   int *submitted,
+   int *submitted_p,
struct writeback_control *wbc,
enum iostat_type io_type)
 {
struct address_space *mapping = cc->inode->i_mapping;
struct f2fs_sb_info *sbi = F2FS_M_SB(mapping);
-   int _submitted, compr_blocks, ret = 0, i;
+   int submitted, compr_blocks, i;
+   int ret = 0;

compr_blocks = f2fs_compressed_blocks(cc);

@@ -1492,7 +1493,7 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
if (!clear_page_dirty_for_io(cc->rpages[i]))
goto continue_unlock;

-   ret = f2fs_write_single_data_page(cc->rpages[i], &_submitted,
+   ret = f2fs_write_single_data_page(cc->rpages[i], ,
NULL, NULL, wbc, io_type,
compr_blocks, false);
if (ret) {
@@ -1514,7 +1515,7 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
goto out;
}

-   *submitted += _submitted;
+   *submitted_p += submitted;
}

 out:

On 01/11, Chao Yu wrote:
> When we overwrite compressed cluster w/ normal cluster, we should
> not unlock cp_rwsem during f2fs_write_raw_pages(), otherwise data
> will be corrupted if partial blocks were persisted before CP & SPOR,
> due to cluster metadata wasn't updated atomically.
> 
> Fixes: 4c8ff7095bef ("f2fs: support data compression")
> Signed-off-by: Chao Yu 
> ---
>  fs/f2fs/compress.c | 20 ++--
>  fs/f2fs/data.c |  3 ++-
>  2 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 9940b7886e5d..bf4cfab67aec 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1448,7 +1448,8 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
>   enum iostat_type io_type)
>  {
>   struct address_space *mapping = cc->inode->i_mapping;
> - int _submitted, compr_blocks, ret, i;
> + struct f2fs_sb_info *sbi = F2FS_M_SB(mapping);
> + int _submitted, compr_blocks, ret = 0, i;
>  
>   compr_blocks = f2fs_compressed_blocks(cc);
>  
> @@ -1463,6 +1464,10 @@ static int f2fs_write_raw_pages(struct compress_ctx 
> *cc,
>   if (compr_blocks < 0)
>   return compr_blocks;
>  
> + /* overwrite compressed cluster w/ normal cluster */
> + if (compr_blocks > 0)
> + f2fs_lock_op(sbi);
> +
>   for (i = 0; i < cc->cluster_size; i++) {
>   if (!cc->rpages[i])
>   continue;
> @@ -1495,26 +1500,29 @@ static int f2fs_write_raw_pages(struct compress_ctx 
> *cc,
>   unlock_page(cc->rpages[i]);
>   ret = 0;
>   } else if (ret == -EAGAIN) {
> + ret = 0;
>   /*
>* for quota file, just redirty left pages to
>* avoid deadlock caused by cluster update race
>* from foreground operation.
>*/
>   if (IS_NOQUOTA(cc->inode))
> - return 0;
> - ret = 0;
> + goto out;
>   f2fs_io_schedule_timeout(DEFAULT_IO_TIMEOUT);
>   goto retry_write;
>   }
> - return ret;
> + goto out;
>   }
>  
>   *submitted += _submitted;
>   }
>  
> - f2fs_balance_fs(F2FS_M_SB(mapping), true);
> +out:
> + if (compr_blocks > 0)
> + f2fs_unlock_op(sbi);
>  
> - return 0;
> + f2fs_balance_fs(sbi, true);
> + return ret;
>  }
>  
>  int f2fs_write_multi_pages(struct compress_ctx *cc,
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 81f9e2cc49e2..b171a9980f6a 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2839,7 +2839,7 @@ int f2fs_write_single_data_page(struct page *page, int 
> *submitted,
>   .encrypted_page = NULL,
>   .submitted = 0,
>   .compr_blocks = compr_blocks,
> - .need_lock = LOCK_RETRY,
> + .need_lock = compr_blocks ? LOCK_DONE : LOCK_RETRY,
>   .post_read = f2fs_post_read_required(inode) ? 1 : 0,
>   .io_type = io_type,
>   .io_wbc = wbc,
> @@ -2920,6 +2920,7 @@ int f2fs_write_single_data_page(struct page *page, 

Re: [PATCH] f2fs: fix double free of f2fs_sb_info

2024-01-12 Thread Eric Biggers
On Fri, Jan 12, 2024 at 05:28:31PM -0800, Jaegeuk Kim wrote:
> On 01/12, Eric Biggers wrote:
> > On Fri, Jan 12, 2024 at 04:57:47PM -0800, Eric Biggers wrote:
> > > From: Eric Biggers 
> > > 
> > > kill_f2fs_super() is called even if f2fs_fill_super() fails.
> > > f2fs_fill_super() frees the struct f2fs_sb_info, so it must set
> > > sb->s_fs_info to NULL to prevent it from being freed again.
> > > 
> > > Fixes: 275dca4630c1 ("f2fs: move release of block devices to after 
> > > kill_block_super()")
> > > Reported-by: syzbot+8f477ac014ff5b32d...@syzkaller.appspotmail.com
> > > Closes: https://lore.kernel.org/r/6cb174060ec34...@google.com
> > > Signed-off-by: Eric Biggers 
> > 
> > Jaegeuk, I'd be glad to take this through the fscrypt tree since that's 
> > where my
> 
> Ok, are you heading to push this in -rc1?
> 
> > broken commit came from.  But let me know if you want to just take this 
> > through
> > the f2fs tree.
> > 

Yes, we should get this into -rc1.

- Eric



Re: [PATCH] f2fs: fix double free of f2fs_sb_info

2024-01-12 Thread Jaegeuk Kim
On 01/12, Eric Biggers wrote:
> On Fri, Jan 12, 2024 at 04:57:47PM -0800, Eric Biggers wrote:
> > From: Eric Biggers 
> > 
> > kill_f2fs_super() is called even if f2fs_fill_super() fails.
> > f2fs_fill_super() frees the struct f2fs_sb_info, so it must set
> > sb->s_fs_info to NULL to prevent it from being freed again.
> > 
> > Fixes: 275dca4630c1 ("f2fs: move release of block devices to after 
> > kill_block_super()")
> > Reported-by: syzbot+8f477ac014ff5b32d...@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/r/6cb174060ec34...@google.com
> > Signed-off-by: Eric Biggers 
> 
> Jaegeuk, I'd be glad to take this through the fscrypt tree since that's where 
> my

Ok, are you heading to push this in -rc1?

> broken commit came from.  But let me know if you want to just take this 
> through
> the f2fs tree.
> 
> - Eric



Re: [f2fs-dev] [PATCH] f2fs: fix double free of f2fs_sb_info

2024-01-12 Thread Chao Yu

On 2024/1/13 8:57, Eric Biggers wrote:

From: Eric Biggers 

kill_f2fs_super() is called even if f2fs_fill_super() fails.
f2fs_fill_super() frees the struct f2fs_sb_info, so it must set
sb->s_fs_info to NULL to prevent it from being freed again.


Oh, I missed that case as well during reviewing, my bad.



Fixes: 275dca4630c1 ("f2fs: move release of block devices to after 
kill_block_super()")
Reported-by: syzbot+8f477ac014ff5b32d...@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/r/6cb174060ec34...@google.com
Signed-off-by: Eric Biggers 


Reviewed-by: Chao Yu 

Thanks,


---
  fs/f2fs/super.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index d00d21a8b53ad..d45ab0992ae59 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4873,20 +4873,21 @@ static int f2fs_fill_super(struct super_block *sb, void 
*data, int silent)
kfree(F2FS_OPTION(sbi).s_qf_names[i]);
  #endif
fscrypt_free_dummy_policy(_OPTION(sbi).dummy_enc_policy);
kvfree(options);
  free_sb_buf:
kfree(raw_super);
  free_sbi:
if (sbi->s_chksum_driver)
crypto_free_shash(sbi->s_chksum_driver);
kfree(sbi);
+   sb->s_fs_info = NULL;
  
  	/* give only one another chance */

if (retry_cnt > 0 && skip_recovery) {
retry_cnt--;
shrink_dcache_sb(sb);
goto try_onemore;
}
return err;
  }
  


base-commit: 38814330fedd778edffcabe0c8cb462ee365782e




Re: [PATCH] f2fs: fix double free of f2fs_sb_info

2024-01-12 Thread Eric Biggers
On Fri, Jan 12, 2024 at 04:57:47PM -0800, Eric Biggers wrote:
> From: Eric Biggers 
> 
> kill_f2fs_super() is called even if f2fs_fill_super() fails.
> f2fs_fill_super() frees the struct f2fs_sb_info, so it must set
> sb->s_fs_info to NULL to prevent it from being freed again.
> 
> Fixes: 275dca4630c1 ("f2fs: move release of block devices to after 
> kill_block_super()")
> Reported-by: syzbot+8f477ac014ff5b32d...@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/r/6cb174060ec34...@google.com
> Signed-off-by: Eric Biggers 

Jaegeuk, I'd be glad to take this through the fscrypt tree since that's where my
broken commit came from.  But let me know if you want to just take this through
the f2fs tree.

- Eric



[PATCH] f2fs: fix double free of f2fs_sb_info

2024-01-12 Thread Eric Biggers
From: Eric Biggers 

kill_f2fs_super() is called even if f2fs_fill_super() fails.
f2fs_fill_super() frees the struct f2fs_sb_info, so it must set
sb->s_fs_info to NULL to prevent it from being freed again.

Fixes: 275dca4630c1 ("f2fs: move release of block devices to after 
kill_block_super()")
Reported-by: syzbot+8f477ac014ff5b32d...@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/r/6cb174060ec34...@google.com
Signed-off-by: Eric Biggers 
---
 fs/f2fs/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index d00d21a8b53ad..d45ab0992ae59 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4873,20 +4873,21 @@ static int f2fs_fill_super(struct super_block *sb, void 
*data, int silent)
kfree(F2FS_OPTION(sbi).s_qf_names[i]);
 #endif
fscrypt_free_dummy_policy(_OPTION(sbi).dummy_enc_policy);
kvfree(options);
 free_sb_buf:
kfree(raw_super);
 free_sbi:
if (sbi->s_chksum_driver)
crypto_free_shash(sbi->s_chksum_driver);
kfree(sbi);
+   sb->s_fs_info = NULL;
 
/* give only one another chance */
if (retry_cnt > 0 && skip_recovery) {
retry_cnt--;
shrink_dcache_sb(sb);
goto try_onemore;
}
return err;
 }
 

base-commit: 38814330fedd778edffcabe0c8cb462ee365782e
-- 
2.43.0




Re: [f2fs-dev] [syzbot] [f2fs?] KASAN: slab-use-after-free Read in kill_f2fs_super

2024-01-12 Thread Eric Biggers
On Fri, Jan 12, 2024 at 04:32:21PM -0800, syzbot wrote:
> loop0: detected capacity change from 0 to 63271
> F2FS-fs (loop0): Mismatch start address, segment0(512) cp_blkaddr(605)
> F2FS-fs (loop0): Can't find valid F2FS filesystem in 1th superblock
> F2FS-fs (loop0): invalid crc value
> F2FS-fs (loop0): SIT is corrupted node# 0 vs 1
> F2FS-fs (loop0): Failed to initialize F2FS segment manager (-117)
> ==
> BUG: KASAN: slab-use-after-free in destroy_device_list fs/f2fs/super.c:1606 
> [inline]
> BUG: KASAN: slab-use-after-free in kill_f2fs_super+0x618/0x690 
> fs/f2fs/super.c:4932
> Read of size 4 at addr 888023bdd77c by task syz-executor275/5046

Sorry, this is my fault.  I'll fix this.

- Eric


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [syzbot] [f2fs?] KASAN: slab-use-after-free Read in kill_f2fs_super

2024-01-12 Thread syzbot
syzbot has found a reproducer for the following issue on:

HEAD commit:70d201a40823 Merge tag 'f2fs-for-6.8-rc1' of git://git.ker..
git tree:   upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=176d9debe8
kernel config:  https://syzkaller.appspot.com/x/.config?x=4607bc15d1c4bb90
dashboard link: https://syzkaller.appspot.com/bug?extid=8f477ac014ff5b32d81f
compiler:   Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 
2.40
syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=112b660be8
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14c1df5de8

Downloadable assets:
disk image: 
https://storage.googleapis.com/syzbot-assets/4faf0f99e43c/disk-70d201a4.raw.xz
vmlinux: 
https://storage.googleapis.com/syzbot-assets/23f59e40d2ef/vmlinux-70d201a4.xz
kernel image: 
https://storage.googleapis.com/syzbot-assets/a0bc6007f0a4/bzImage-70d201a4.xz
mounted in repro: 
https://storage.googleapis.com/syzbot-assets/cf767215e29c/mount_0.gz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+8f477ac014ff5b32d...@syzkaller.appspotmail.com

loop0: detected capacity change from 0 to 63271
F2FS-fs (loop0): Mismatch start address, segment0(512) cp_blkaddr(605)
F2FS-fs (loop0): Can't find valid F2FS filesystem in 1th superblock
F2FS-fs (loop0): invalid crc value
F2FS-fs (loop0): SIT is corrupted node# 0 vs 1
F2FS-fs (loop0): Failed to initialize F2FS segment manager (-117)
==
BUG: KASAN: slab-use-after-free in destroy_device_list fs/f2fs/super.c:1606 
[inline]
BUG: KASAN: slab-use-after-free in kill_f2fs_super+0x618/0x690 
fs/f2fs/super.c:4932
Read of size 4 at addr 888023bdd77c by task syz-executor275/5046

CPU: 0 PID: 5046 Comm: syz-executor275 Not tainted 
6.7.0-syzkaller-06264-g70d201a40823 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 
11/17/2023
Call Trace:
 
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0x1e7/0x2d0 lib/dump_stack.c:106
 print_address_description mm/kasan/report.c:377 [inline]
 print_report+0x163/0x540 mm/kasan/report.c:488
 kasan_report+0x142/0x170 mm/kasan/report.c:601
 destroy_device_list fs/f2fs/super.c:1606 [inline]
 kill_f2fs_super+0x618/0x690 fs/f2fs/super.c:4932
 deactivate_locked_super+0xc1/0x130 fs/super.c:477
 mount_bdev+0x222/0x2d0 fs/super.c:1665
 legacy_get_tree+0xef/0x190 fs/fs_context.c:662
 vfs_get_tree+0x8c/0x2a0 fs/super.c:1784
 do_new_mount+0x2be/0xb40 fs/namespace.c:3352
 do_mount fs/namespace.c:3692 [inline]
 __do_sys_mount fs/namespace.c:3898 [inline]
 __se_sys_mount+0x2d9/0x3c0 fs/namespace.c:3875
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf5/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x63/0x6b
RIP: 0033:0x7fb05d646c7a
Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb a6 e8 5e 04 00 00 66 2e 0f 1f 84 00 
00 00 00 00 0f 1f 40 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 
c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:7ffedf4214a8 EFLAGS: 0286 ORIG_RAX: 00a5
RAX: ffda RBX: 7ffedf4214c0 RCX: 7fb05d646c7a
RDX: 20c0 RSI: 20007f80 RDI: 7ffedf4214c0
RBP: 0010 R08: 7ffedf421500 R09: 7e73
R10: 0010 R11: 0286 R12: 0004
R13: 7ffedf421500 R14: 0003 R15: 01ee4e54
 

Allocated by task 5046:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x3f/0x70 mm/kasan/common.c:68
 poison_kmalloc_redzone mm/kasan/common.c:372 [inline]
 __kasan_kmalloc+0x98/0xb0 mm/kasan/common.c:389
 kasan_kmalloc include/linux/kasan.h:211 [inline]
 kmalloc_trace+0x1d6/0x360 mm/slub.c:4012
 kmalloc include/linux/slab.h:590 [inline]
 kzalloc include/linux/slab.h:711 [inline]
 f2fs_fill_super+0xce/0x8170 fs/f2fs/super.c:4397
 mount_bdev+0x206/0x2d0 fs/super.c:1663
 legacy_get_tree+0xef/0x190 fs/fs_context.c:662
 vfs_get_tree+0x8c/0x2a0 fs/super.c:1784
 do_new_mount+0x2be/0xb40 fs/namespace.c:3352
 do_mount fs/namespace.c:3692 [inline]
 __do_sys_mount fs/namespace.c:3898 [inline]
 __se_sys_mount+0x2d9/0x3c0 fs/namespace.c:3875
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf5/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x63/0x6b

Freed by task 5046:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x3f/0x70 mm/kasan/common.c:68
 kasan_save_free_info+0x4e/0x60 mm/kasan/generic.c:634
 poison_slab_object+0xa6/0xe0 mm/kasan/common.c:241
 __kasan_slab_free+0x34/0x60 mm/kasan/common.c:257
 kasan_slab_free include/linux/kasan.h:184 [inline]
 slab_free_hook mm/slub.c:2121 [inline]
 slab_free mm/slub.c:4299 [inline]
 kfree+0x14a/0x380 mm/slub.c:4409
 f2fs_fill_super+0x6b04/0x8170 fs/f2fs/super.c:4882
 mount_bdev+0x206/0x2d0 fs/super.c:1663
 legacy_get_tree+0xef/0x190 fs/fs_context.c:662
 vfs_get_tree+0x8c/0x2a0 fs/super.c:1784
 

Re: [f2fs-dev] [PATCH v4 6/6] f2fs: introduce FAULT_BLKADDR_CONSISTENCE

2024-01-12 Thread Daeho Jeong
Reviewed-by: Daeho Jeong 

On Wed, Jan 10, 2024 at 10:43 PM Chao Yu  wrote:
>
> We will encounter below inconsistent status when FAULT_BLKADDR type
> fault injection is on.
>
> Info: checkpoint state = d6 :  nat_bits crc fsck compacted_summary 
> orphan_inodes sudden-power-off
> [ASSERT] (fsck_chk_inode_blk:1254)  --> ino: 0x1c100 has i_blocks: 00c0, 
> but has 191 blocks
> [FIX] (fsck_chk_inode_blk:1260)  --> [0x1c100] i_blocks=0x00c0 -> 0xbf
> [FIX] (fsck_chk_inode_blk:1269)  --> [0x1c100] i_compr_blocks=0x0026 -> 
> 0x27
> [ASSERT] (fsck_chk_inode_blk:1254)  --> ino: 0x1cadb has i_blocks: 002f, 
> but has 46 blocks
> [FIX] (fsck_chk_inode_blk:1260)  --> [0x1cadb] i_blocks=0x002f -> 0x2e
> [FIX] (fsck_chk_inode_blk:1269)  --> [0x1cadb] i_compr_blocks=0x0011 -> 
> 0x12
> [ASSERT] (fsck_chk_inode_blk:1254)  --> ino: 0x1c62c has i_blocks: 0002, 
> but has 1 blocks
> [FIX] (fsck_chk_inode_blk:1260)  --> [0x1c62c] i_blocks=0x0002 -> 0x1
>
> After we inject fault into f2fs_is_valid_blkaddr() during truncation,
> a) it missed to increase @nr_free or @valid_blocks
> b) it can cause in blkaddr leak in truncated dnode
> Which may cause inconsistent status.
>
> This patch separates FAULT_BLKADDR_CONSISTENCE from FAULT_BLKADDR,
> and rename FAULT_BLKADDR to FAULT_BLKADDR_VALIDITY
> so that we can:
> a) use FAULT_BLKADDR_CONSISTENCE in f2fs_truncate_data_blocks_range()
> to simulate inconsistent issue independently, then it can verify fsck
> repair flow.
> b) FAULT_BLKADDR_VALIDITY fault will not cause any inconsistent status,
> we can just use it to check error path handling in kernel side.
>
> Signed-off-by: Chao Yu 
> ---
> v4:
> - rename macro to FAULT_BLKADDR_CONSISTENCE and FAULT_BLKADDR_VALIDITY
> suggested by Jaegeuk.
>  Documentation/ABI/testing/sysfs-fs-f2fs | 47 +
>  Documentation/filesystems/f2fs.rst  | 47 +
>  fs/f2fs/checkpoint.c| 19 +++---
>  fs/f2fs/f2fs.h  |  5 ++-
>  fs/f2fs/file.c  |  8 +++--
>  fs/f2fs/super.c | 37 +--
>  6 files changed, 92 insertions(+), 71 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-fs-f2fs 
> b/Documentation/ABI/testing/sysfs-fs-f2fs
> index 99fa87a43926..48c135e24eb5 100644
> --- a/Documentation/ABI/testing/sysfs-fs-f2fs
> +++ b/Documentation/ABI/testing/sysfs-fs-f2fs
> @@ -701,29 +701,30 @@ Description:  Support configuring fault injection 
> type, should be
> enabled with fault_injection option, fault type value
> is shown below, it supports single or combined type.
>
> -   ===  ===
> -   Type_NameType_Value
> -   ===  ===
> -   FAULT_KMALLOC0x1
> -   FAULT_KVMALLOC   0x2
> -   FAULT_PAGE_ALLOC 0x4
> -   FAULT_PAGE_GET   0x8
> -   FAULT_ALLOC_BIO  0x00010 (obsolete)
> -   FAULT_ALLOC_NID  0x00020
> -   FAULT_ORPHAN 0x00040
> -   FAULT_BLOCK  0x00080
> -   FAULT_DIR_DEPTH  0x00100
> -   FAULT_EVICT_INODE0x00200
> -   FAULT_TRUNCATE   0x00400
> -   FAULT_READ_IO0x00800
> -   FAULT_CHECKPOINT 0x01000
> -   FAULT_DISCARD0x02000
> -   FAULT_WRITE_IO   0x04000
> -   FAULT_SLAB_ALLOC 0x08000
> -   FAULT_DQUOT_INIT 0x1
> -   FAULT_LOCK_OP0x2
> -   FAULT_BLKADDR0x4
> -   ===  ===
> +   ===  ===
> +   Type_NameType_Value
> +   ===  ===
> +   FAULT_KMALLOC0x1
> +   FAULT_KVMALLOC   0x2
> +   FAULT_PAGE_ALLOC 0x4
> +   FAULT_PAGE_GET   0x8
> +   FAULT_ALLOC_BIO  0x00010 (obsolete)
> +   FAULT_ALLOC_NID  0x00020
> +   FAULT_ORPHAN 0x00040
> +   FAULT_BLOCK  0x00080
> +   FAULT_DIR_DEPTH  0x00100
> +   FAULT_EVICT_INODE0x00200
> +   FAULT_TRUNCATE   0x00400
> +   FAULT_READ_IO0x00800
> +   FAULT_CHECKPOINT 0x01000
> + 

Re: [f2fs-dev] [PATCH v4 5/6] f2fs: fix to remove unnecessary f2fs_bug_on() to avoid panic

2024-01-12 Thread Daeho Jeong
Reviewed-by: Daeho Jeong 

On Wed, Jan 10, 2024 at 10:43 PM Chao Yu  wrote:
>
> verify_blkaddr() will trigger panic once we inject fault into
> f2fs_is_valid_blkaddr(), fix to remove this unnecessary f2fs_bug_on().
>
> Fixes: 18792e64c86d ("f2fs: support fault injection for 
> f2fs_is_valid_blkaddr()")
> Signed-off-by: Chao Yu 
> ---
>  fs/f2fs/f2fs.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 69e71460a950..ab710bb6d8b3 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3470,11 +3470,9 @@ bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
>  static inline void verify_blkaddr(struct f2fs_sb_info *sbi,
> block_t blkaddr, int type)
>  {
> -   if (!f2fs_is_valid_blkaddr(sbi, blkaddr, type)) {
> +   if (!f2fs_is_valid_blkaddr(sbi, blkaddr, type))
> f2fs_err(sbi, "invalid blkaddr: %u, type: %d, run fsck to 
> fix.",
>  blkaddr, type);
> -   f2fs_bug_on(sbi, 1);
> -   }
>  }
>
>  static inline bool __is_valid_data_blkaddr(block_t blkaddr)
> --
> 2.40.1
>
>
>
> ___
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH v4 4/6] f2fs: compress: fix to avoid inconsistence bewteen i_blocks and dnode

2024-01-12 Thread Daeho Jeong
Reviewed-by: Daeho Jeong 

On Wed, Jan 10, 2024 at 10:43 PM Chao Yu  wrote:
>
> In reserve_compress_blocks(), we update blkaddrs of dnode in prior to
> inc_valid_block_count(), it may cause inconsistent status bewteen
> i_blocks and blkaddrs once inc_valid_block_count() fails.
>
> To fix this issue, it needs to reverse their invoking order.
>
> Fixes: c75488fb4d82 ("f2fs: introduce F2FS_IOC_RESERVE_COMPRESS_BLOCKS")
> Signed-off-by: Chao Yu 
> ---
>  fs/f2fs/data.c|  5 +++--
>  fs/f2fs/f2fs.h|  7 ++-
>  fs/f2fs/file.c| 26 ++
>  fs/f2fs/segment.c |  2 +-
>  4 files changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index b171a9980f6a..8d2ace723310 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1219,7 +1219,8 @@ int f2fs_reserve_new_blocks(struct dnode_of_data *dn, 
> blkcnt_t count)
>
> if (unlikely(is_inode_flag_set(dn->inode, FI_NO_ALLOC)))
> return -EPERM;
> -   if (unlikely((err = inc_valid_block_count(sbi, dn->inode, 
> +   err = inc_valid_block_count(sbi, dn->inode, , true);
> +   if (unlikely(err))
> return err;
>
> trace_f2fs_reserve_new_blocks(dn->inode, dn->nid,
> @@ -1476,7 +1477,7 @@ static int __allocate_data_block(struct dnode_of_data 
> *dn, int seg_type)
>
> dn->data_blkaddr = f2fs_data_blkaddr(dn);
> if (dn->data_blkaddr == NULL_ADDR) {
> -   err = inc_valid_block_count(sbi, dn->inode, );
> +   err = inc_valid_block_count(sbi, dn->inode, , true);
> if (unlikely(err))
> return err;
> }
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 50f3d546ded8..69e71460a950 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -2252,7 +2252,7 @@ static inline bool __allow_reserved_blocks(struct 
> f2fs_sb_info *sbi,
>
>  static inline void f2fs_i_blocks_write(struct inode *, block_t, bool, bool);
>  static inline int inc_valid_block_count(struct f2fs_sb_info *sbi,
> -struct inode *inode, blkcnt_t *count)
> +struct inode *inode, blkcnt_t *count, bool 
> partial)
>  {
> blkcnt_t diff = 0, release = 0;
> block_t avail_user_block_count;
> @@ -2292,6 +2292,11 @@ static inline int inc_valid_block_count(struct 
> f2fs_sb_info *sbi,
> avail_user_block_count = 0;
> }
> if (unlikely(sbi->total_valid_block_count > avail_user_block_count)) {
> +   if (!partial) {
> +   spin_unlock(>stat_lock);
> +   goto enospc;
> +   }
> +
> diff = sbi->total_valid_block_count - avail_user_block_count;
> if (diff > *count)
> diff = *count;
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 80d9c4c096f0..53c495651789 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -3614,14 +3614,16 @@ static int reserve_compress_blocks(struct 
> dnode_of_data *dn, pgoff_t count)
> blkcnt_t reserved;
> int ret;
>
> -   for (i = 0; i < cluster_size; i++, dn->ofs_in_node++) {
> -   blkaddr = f2fs_data_blkaddr(dn);
> +   for (i = 0; i < cluster_size; i++) {
> +   blkaddr = data_blkaddr(dn->inode, dn->node_page,
> +   dn->ofs_in_node + i);
>
> if (i == 0) {
> -   if (blkaddr == COMPRESS_ADDR)
> -   continue;
> -   dn->ofs_in_node += cluster_size;
> -   goto next;
> +   if (blkaddr != COMPRESS_ADDR) {
> +   dn->ofs_in_node += cluster_size;
> +   goto next;
> +   }
> +   continue;
> }
>
> /*
> @@ -3634,20 +3636,20 @@ static int reserve_compress_blocks(struct 
> dnode_of_data *dn, pgoff_t count)
> compr_blocks++;
> continue;
> }
> -
> -   f2fs_set_data_blkaddr(dn, NEW_ADDR);
> }
>
> reserved = cluster_size - compr_blocks;
> if (!reserved)
> goto next;
>
> -   ret = inc_valid_block_count(sbi, dn->inode, );
> -   if (ret)
> +   ret = inc_valid_block_count(sbi, dn->inode, , false);
> +   if (unlikely(ret))
> return ret;
>
> -   if (reserved != cluster_size - compr_blocks)
> -   return -ENOSPC;
> +   for (i = 0; i < cluster_size; i++, dn->ofs_in_node++) {
> +   if 

Re: [f2fs-dev] [PATCH v3 3/6] f2fs: compress: fix to check unreleased compressed cluster

2024-01-12 Thread Daeho Jeong
On Thu, Jan 11, 2024 at 5:06 PM Chao Yu  wrote:
>
> On 2024/1/12 1:15, Daeho Jeong wrote:
> > On Wed, Jan 10, 2024 at 5:33 PM Chao Yu  wrote:
> >>
> >> On 2024/1/11 9:18, Daeho Jeong wrote:
> >>> On Thu, Dec 28, 2023 at 6:33 AM Chao Yu  wrote:
> 
>  From: Sheng Yong 
> 
>  Compressed cluster may not be released due to we can fail in
>  release_compress_blocks(), fix to handle reserved compressed
>  cluster correctly in reserve_compress_blocks().
> 
>  Fixes: 4c8ff7095bef ("f2fs: support data compression")
>  Signed-off-by: Sheng Yong 
>  Signed-off-by: Chao Yu 
>  ---
> fs/f2fs/file.c | 12 
> 1 file changed, 12 insertions(+)
> 
>  diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>  index 026d05a7edd8..782ae3be48f6 100644
>  --- a/fs/f2fs/file.c
>  +++ b/fs/f2fs/file.c
>  @@ -3624,6 +3624,15 @@ static int reserve_compress_blocks(struct 
>  dnode_of_data *dn, pgoff_t count)
>    goto next;
>    }
> 
>  +   /*
>  +* compressed cluster was not released due to
>  +* it fails in release_compress_blocks().
>  +*/
>  +   if (blkaddr == NEW_ADDR) {
>  +   compr_blocks++;
>  +   continue;
>  +   }
>  +
>    if (__is_valid_data_blkaddr(blkaddr)) {
>    compr_blocks++;
>    continue;
> >>>
> >>> How about merging two conditions like "blkaddr == NEW_ADDR ||
> >>> __is_valid_data_blkaddr(blkaddr)"?
> >>
> >> Oh, sure.
> >>
> >>>
>  @@ -3633,6 +3642,9 @@ static int reserve_compress_blocks(struct 
>  dnode_of_data *dn, pgoff_t count)
>    }
> 
>    reserved = cluster_size - compr_blocks;
>  +   if (!reserved)
>  +   goto next;
>  +
> >>>
> >>> How can the reserved variable be zero?
> >>
> >> I guess it can happen if a cluster was not released during
> >> release_compress_blocks(), then all blocks in the cluster should
> >> has been reserved, so, in this round of reserving, it needs to skip
> >> reserve blocks, right?
> >
> > Let's assume cluster_size is 4. How can compr_blocks be 4?
> >
> >  if (i == 0) {
> >  if (blkaddr == COMPRESS_ADDR)
> >  continue;
> >  dn->ofs_in_node += cluster_size;
> >  goto next;
> >  }
> >
> > We skip the block having COMPRESS_ADDR when counting compr_blocks.
> > So, the maximum value of compr_blocks should be 3, right?
>
> Ah, got it, and I think you're right.
>
> Should fix the condition as below?
>
> /* for the case all blocks in cluster were reserved */
> if (reserved == 1)
> goto next;

It looks good to me.

>
> Thanks,
>
> >
> >>
> >> Thanks,
> >>
> >>>
>    ret = inc_valid_block_count(sbi, dn->inode, );
>    if (ret)
>    return ret;
>  --
>  2.40.1
> 
> 
> 
>  ___
>  Linux-f2fs-devel mailing list
>  Linux-f2fs-devel@lists.sourceforge.net
>  https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [PATCH v3 1/6] f2fs: compress: fix to guarantee persisting compressed blocks by CP

2024-01-12 Thread Daeho Jeong
Reviewed-by: Daeho Jeong 

On Thu, Jan 11, 2024 at 9:17 AM Daeho Jeong  wrote:
>
> On Wed, Jan 10, 2024 at 5:26 PM Chao Yu  wrote:
> >
> > On 2024/1/11 8:55, Daeho Jeong wrote:
> > > On Thu, Dec 28, 2023 at 6:33 AM Chao Yu  wrote:
> > >>
> > >> If data block in compressed cluster is not persisted with metadata
> > >> during checkpoint, after SPOR, the data may be corrupted, let's
> > >> guarantee to write compressed page by checkpoint.
> > >>
> > >> Fixes: 4c8ff7095bef ("f2fs: support data compression")
> > >> Signed-off-by: Chao Yu 
> > >> ---
> > >> v3:
> > >> - treat compressed page as CP guaranteed data explictly.
> > >>   fs/f2fs/compress.c |  4 +++-
> > >>   fs/f2fs/data.c | 17 +
> > >>   fs/f2fs/f2fs.h |  4 +++-
> > >>   3 files changed, 15 insertions(+), 10 deletions(-)
> > >>
> > >> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > >> index c5a4364c4482..9940b7886e5d 100644
> > >> --- a/fs/f2fs/compress.c
> > >> +++ b/fs/f2fs/compress.c
> > >> @@ -1418,6 +1418,8 @@ void f2fs_compress_write_end_io(struct bio *bio, 
> > >> struct page *page)
> > >>  struct f2fs_sb_info *sbi = bio->bi_private;
> > >>  struct compress_io_ctx *cic =
> > >>  (struct compress_io_ctx *)page_private(page);
> > >> +   enum count_type type = WB_DATA_TYPE(page,
> > >> +   f2fs_is_compressed_page(page));
> > >>  int i;
> > >>
> > >>  if (unlikely(bio->bi_status))
> > >> @@ -1425,7 +1427,7 @@ void f2fs_compress_write_end_io(struct bio *bio, 
> > >> struct page *page)
> > >>
> > >>  f2fs_compress_free_page(page);
> > >>
> > >> -   dec_page_count(sbi, F2FS_WB_DATA);
> > >> +   dec_page_count(sbi, type);
> > >>
> > >>  if (atomic_dec_return(>pending_pages))
> > >>  return;
> > >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > >> index dce8defdf4c7..81f9e2cc49e2 100644
> > >> --- a/fs/f2fs/data.c
> > >> +++ b/fs/f2fs/data.c
> > >> @@ -48,7 +48,7 @@ void f2fs_destroy_bioset(void)
> > >>  bioset_exit(_bioset);
> > >>   }
> > >>
> > >> -static bool __is_cp_guaranteed(struct page *page)
> > >> +bool f2fs_is_cp_guaranteed(struct page *page)
> > >>   {
> > >>  struct address_space *mapping = page->mapping;
> > >>  struct inode *inode;
> > >> @@ -65,8 +65,6 @@ static bool __is_cp_guaranteed(struct page *page)
> > >>  S_ISDIR(inode->i_mode))
> > >>  return true;
> > >>
> > >> -   if (f2fs_is_compressed_page(page))
> > >> -   return false;
> > >
> > > Out of curiosity, why don't we simply change the above to "return true"?
> >
> > Daeho,
> >
> > I used the implementation, please check v1 and related comments
> > from Jaegeuk and me, let me know if that was not clear enough. :)
> >
> > https://lore.kernel.org/linux-f2fs-devel/aae654e7-8a7e-478d-9f5a-65807a0e0...@kernel.org/
>
> Oh, I missed it. Sorry~
>
> >
> > >
> > >>  if ((S_ISREG(inode->i_mode) && IS_NOQUOTA(inode)) ||
> > >>  page_private_gcing(page))
> > >>  return true;
> > >> @@ -338,7 +336,7 @@ static void f2fs_write_end_io(struct bio *bio)
> > >>
> > >>  bio_for_each_segment_all(bvec, bio, iter_all) {
> > >>  struct page *page = bvec->bv_page;
> > >> -   enum count_type type = WB_DATA_TYPE(page);
> > >> +   enum count_type type = WB_DATA_TYPE(page, false);
> > >>
> > >>  if (page_private_dummy(page)) {
> > >>  clear_page_private_dummy(page);
> > >> @@ -762,7 +760,7 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
> > >>  wbc_account_cgroup_owner(fio->io_wbc, fio->page, 
> > >> PAGE_SIZE);
> > >>
> > >>  inc_page_count(fio->sbi, is_read_io(fio->op) ?
> > >> -   __read_io_type(page) : WB_DATA_TYPE(fio->page));
> > >> +   __read_io_type(page) : WB_DATA_TYPE(fio->page, 
> > >> false));
> > >>
> > >>  if (is_read_io(bio_op(bio)))
> > >>  f2fs_submit_read_bio(fio->sbi, bio, fio->type);
> > >> @@ -973,7 +971,7 @@ int f2fs_merge_page_bio(struct f2fs_io_info *fio)
> > >>  if (fio->io_wbc)
> > >>  wbc_account_cgroup_owner(fio->io_wbc, fio->page, 
> > >> PAGE_SIZE);
> > >>
> > >> -   inc_page_count(fio->sbi, WB_DATA_TYPE(page));
> > >> +   inc_page_count(fio->sbi, WB_DATA_TYPE(page, false));
> > >>
> > >>  *fio->last_block = fio->new_blkaddr;
> > >>  *fio->bio = bio;
> > >> @@ -1007,6 +1005,7 @@ void f2fs_submit_page_write(struct f2fs_io_info 
> > >> *fio)
> > >>  enum page_type btype = PAGE_TYPE_OF_BIO(fio->type);
> > >>  struct f2fs_bio_info *io = sbi->write_io[btype] + fio->temp;
> > >>  struct page *bio_page;
> > >> +   enum count_type type;
> > >>
> > >>  f2fs_bug_on(sbi, is_read_io(fio->op));
> > >>
> > >> @@ -1046,7 +1045,8 @@ void 

Re: [f2fs-dev] [PATCH v4 2/6] f2fs: compress: fix to cover normal cluster write with cp_rwsem

2024-01-12 Thread Daeho Jeong
Reviewed-by: Daeho Jeong 

On Wed, Jan 10, 2024 at 10:43 PM Chao Yu  wrote:
>
> When we overwrite compressed cluster w/ normal cluster, we should
> not unlock cp_rwsem during f2fs_write_raw_pages(), otherwise data
> will be corrupted if partial blocks were persisted before CP & SPOR,
> due to cluster metadata wasn't updated atomically.
>
> Fixes: 4c8ff7095bef ("f2fs: support data compression")
> Signed-off-by: Chao Yu 
> ---
>  fs/f2fs/compress.c | 20 ++--
>  fs/f2fs/data.c |  3 ++-
>  2 files changed, 16 insertions(+), 7 deletions(-)
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 9940b7886e5d..bf4cfab67aec 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1448,7 +1448,8 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
> enum iostat_type io_type)
>  {
> struct address_space *mapping = cc->inode->i_mapping;
> -   int _submitted, compr_blocks, ret, i;
> +   struct f2fs_sb_info *sbi = F2FS_M_SB(mapping);
> +   int _submitted, compr_blocks, ret = 0, i;
>
> compr_blocks = f2fs_compressed_blocks(cc);
>
> @@ -1463,6 +1464,10 @@ static int f2fs_write_raw_pages(struct compress_ctx 
> *cc,
> if (compr_blocks < 0)
> return compr_blocks;
>
> +   /* overwrite compressed cluster w/ normal cluster */
> +   if (compr_blocks > 0)
> +   f2fs_lock_op(sbi);
> +
> for (i = 0; i < cc->cluster_size; i++) {
> if (!cc->rpages[i])
> continue;
> @@ -1495,26 +1500,29 @@ static int f2fs_write_raw_pages(struct compress_ctx 
> *cc,
> unlock_page(cc->rpages[i]);
> ret = 0;
> } else if (ret == -EAGAIN) {
> +   ret = 0;
> /*
>  * for quota file, just redirty left pages to
>  * avoid deadlock caused by cluster update 
> race
>  * from foreground operation.
>  */
> if (IS_NOQUOTA(cc->inode))
> -   return 0;
> -   ret = 0;
> +   goto out;
> f2fs_io_schedule_timeout(DEFAULT_IO_TIMEOUT);
> goto retry_write;
> }
> -   return ret;
> +   goto out;
> }
>
> *submitted += _submitted;
> }
>
> -   f2fs_balance_fs(F2FS_M_SB(mapping), true);
> +out:
> +   if (compr_blocks > 0)
> +   f2fs_unlock_op(sbi);
>
> -   return 0;
> +   f2fs_balance_fs(sbi, true);
> +   return ret;
>  }
>
>  int f2fs_write_multi_pages(struct compress_ctx *cc,
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 81f9e2cc49e2..b171a9980f6a 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2839,7 +2839,7 @@ int f2fs_write_single_data_page(struct page *page, int 
> *submitted,
> .encrypted_page = NULL,
> .submitted = 0,
> .compr_blocks = compr_blocks,
> -   .need_lock = LOCK_RETRY,
> +   .need_lock = compr_blocks ? LOCK_DONE : LOCK_RETRY,
> .post_read = f2fs_post_read_required(inode) ? 1 : 0,
> .io_type = io_type,
> .io_wbc = wbc,
> @@ -2920,6 +2920,7 @@ int f2fs_write_single_data_page(struct page *page, int 
> *submitted,
> if (err == -EAGAIN) {
> err = f2fs_do_write_data_page();
> if (err == -EAGAIN) {
> +   f2fs_bug_on(sbi, compr_blocks);
> fio.need_lock = LOCK_REQ;
> err = f2fs_do_write_data_page();
> }
> --
> 2.40.1
>
>
>
> ___
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [GIT PULL] f2fs update for 6.8-rc1

2024-01-12 Thread Linus Torvalds
On Thu, 11 Jan 2024 at 23:12, Al Viro  wrote:
>
> Where would you end up with old_dir_page != NULL and old_dir_entry == NULL?

D'oh.

You are of course right, and I missed that connection. Happily my
merge still works, just isn't as minimal as yours.

I see that Jaegeuk already posted the patch for the cleanup.

  Linus


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [syzbot] [f2fs?] KASAN: slab-use-after-free Read in kill_f2fs_super

2024-01-12 Thread syzbot
syzbot has found a reproducer for the following issue on:

HEAD commit:70d201a40823 Merge tag 'f2fs-for-6.8-rc1' of git://git.ker..
git tree:   upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=13973ca5e8
kernel config:  https://syzkaller.appspot.com/x/.config?x=4607bc15d1c4bb90
dashboard link: https://syzkaller.appspot.com/bug?extid=8f477ac014ff5b32d81f
compiler:   Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 
2.40
syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=16b910ede8

Downloadable assets:
disk image: 
https://storage.googleapis.com/syzbot-assets/4faf0f99e43c/disk-70d201a4.raw.xz
vmlinux: 
https://storage.googleapis.com/syzbot-assets/23f59e40d2ef/vmlinux-70d201a4.xz
kernel image: 
https://storage.googleapis.com/syzbot-assets/a0bc6007f0a4/bzImage-70d201a4.xz
mounted in repro #1: 
https://storage.googleapis.com/syzbot-assets/47520ebdcfe6/mount_0.gz
mounted in repro #2: 
https://storage.googleapis.com/syzbot-assets/bc946d522410/mount_1.gz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+8f477ac014ff5b32d...@syzkaller.appspotmail.com

==
BUG: KASAN: slab-use-after-free in destroy_device_list fs/f2fs/super.c:1606 
[inline]
BUG: KASAN: slab-use-after-free in kill_f2fs_super+0x618/0x690 
fs/f2fs/super.c:4932
Read of size 4 at addr 88801c6bd77c by task syz-executor.0/5218

CPU: 1 PID: 5218 Comm: syz-executor.0 Not tainted 
6.7.0-syzkaller-06264-g70d201a40823 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 
11/17/2023
Call Trace:
 
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0x1e7/0x2d0 lib/dump_stack.c:106
 print_address_description mm/kasan/report.c:377 [inline]
 print_report+0x163/0x540 mm/kasan/report.c:488
 kasan_report+0x142/0x170 mm/kasan/report.c:601
 destroy_device_list fs/f2fs/super.c:1606 [inline]
 kill_f2fs_super+0x618/0x690 fs/f2fs/super.c:4932
 deactivate_locked_super+0xc1/0x130 fs/super.c:477
 mount_bdev+0x222/0x2d0 fs/super.c:1665
 legacy_get_tree+0xef/0x190 fs/fs_context.c:662
 vfs_get_tree+0x8c/0x2a0 fs/super.c:1784
 do_new_mount+0x2be/0xb40 fs/namespace.c:3352
 do_mount fs/namespace.c:3692 [inline]
 __do_sys_mount fs/namespace.c:3898 [inline]
 __se_sys_mount+0x2d9/0x3c0 fs/namespace.c:3875
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf5/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x63/0x6b
RIP: 0033:0x7ff92ca7e4aa
Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb a6 e8 de 09 00 00 66 2e 0f 1f 84 00 
00 00 00 00 0f 1f 40 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 
c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:7fffd484f9b8 EFLAGS: 0246 ORIG_RAX: 00a5
RAX: ffda RBX: 7fffd484fa40 RCX: 7ff92ca7e4aa
RDX: 2040 RSI: 2080 RDI: 7fffd484fa00
RBP: 2040 R08: 7fffd484fa40 R09: 
R10:  R11: 0246 R12: 2080
R13: 7fffd484fa00 R14: 54f9 R15: 24c0
 

Allocated by task 5218:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x3f/0x70 mm/kasan/common.c:68
 poison_kmalloc_redzone mm/kasan/common.c:372 [inline]
 __kasan_kmalloc+0x98/0xb0 mm/kasan/common.c:389
 kasan_kmalloc include/linux/kasan.h:211 [inline]
 kmalloc_trace+0x1d6/0x360 mm/slub.c:4012
 kmalloc include/linux/slab.h:590 [inline]
 kzalloc include/linux/slab.h:711 [inline]
 f2fs_fill_super+0xce/0x8170 fs/f2fs/super.c:4397
 mount_bdev+0x206/0x2d0 fs/super.c:1663
 legacy_get_tree+0xef/0x190 fs/fs_context.c:662
 vfs_get_tree+0x8c/0x2a0 fs/super.c:1784
 do_new_mount+0x2be/0xb40 fs/namespace.c:3352
 do_mount fs/namespace.c:3692 [inline]
 __do_sys_mount fs/namespace.c:3898 [inline]
 __se_sys_mount+0x2d9/0x3c0 fs/namespace.c:3875
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf5/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x63/0x6b

Freed by task 5218:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x3f/0x70 mm/kasan/common.c:68
 kasan_save_free_info+0x4e/0x60 mm/kasan/generic.c:634
 poison_slab_object+0xa6/0xe0 mm/kasan/common.c:241
 __kasan_slab_free+0x34/0x60 mm/kasan/common.c:257
 kasan_slab_free include/linux/kasan.h:184 [inline]
 slab_free_hook mm/slub.c:2121 [inline]
 slab_free mm/slub.c:4299 [inline]
 kfree+0x14a/0x380 mm/slub.c:4409
 f2fs_fill_super+0x6b04/0x8170 fs/f2fs/super.c:4882
 mount_bdev+0x206/0x2d0 fs/super.c:1663
 legacy_get_tree+0xef/0x190 fs/fs_context.c:662
 vfs_get_tree+0x8c/0x2a0 fs/super.c:1784
 do_new_mount+0x2be/0xb40 fs/namespace.c:3352
 do_mount fs/namespace.c:3692 [inline]
 __do_sys_mount fs/namespace.c:3898 [inline]
 __se_sys_mount+0x2d9/0x3c0 fs/namespace.c:3875
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf5/0x230 arch/x86/entry/common.c:83
 

[f2fs-dev] [syzbot] [f2fs?] KASAN: slab-use-after-free Read in kill_f2fs_super

2024-01-12 Thread syzbot
Hello,

syzbot found the following issue on:

HEAD commit:70d201a40823 Merge tag 'f2fs-for-6.8-rc1' of git://git.ker..
git tree:   upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=17e19adbe8
kernel config:  https://syzkaller.appspot.com/x/.config?x=4607bc15d1c4bb90
dashboard link: https://syzkaller.appspot.com/bug?extid=8f477ac014ff5b32d81f
compiler:   Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 
2.40

Unfortunately, I don't have any reproducer for this issue yet.

Downloadable assets:
disk image: 
https://storage.googleapis.com/syzbot-assets/4faf0f99e43c/disk-70d201a4.raw.xz
vmlinux: 
https://storage.googleapis.com/syzbot-assets/23f59e40d2ef/vmlinux-70d201a4.xz
kernel image: 
https://storage.googleapis.com/syzbot-assets/a0bc6007f0a4/bzImage-70d201a4.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+8f477ac014ff5b32d...@syzkaller.appspotmail.com

==
BUG: KASAN: slab-use-after-free in destroy_device_list fs/f2fs/super.c:1606 
[inline]
BUG: KASAN: slab-use-after-free in kill_f2fs_super+0x618/0x690 
fs/f2fs/super.c:4932
Read of size 4 at addr 88801e12d77c by task syz-executor.1/9994

CPU: 1 PID: 9994 Comm: syz-executor.1 Not tainted 
6.7.0-syzkaller-06264-g70d201a40823 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 
11/17/2023
Call Trace:
 
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0x1e7/0x2d0 lib/dump_stack.c:106
 print_address_description mm/kasan/report.c:377 [inline]
 print_report+0x163/0x540 mm/kasan/report.c:488
 kasan_report+0x142/0x170 mm/kasan/report.c:601
 destroy_device_list fs/f2fs/super.c:1606 [inline]
 kill_f2fs_super+0x618/0x690 fs/f2fs/super.c:4932
 deactivate_locked_super+0xc1/0x130 fs/super.c:477
 mount_bdev+0x222/0x2d0 fs/super.c:1665
 legacy_get_tree+0xef/0x190 fs/fs_context.c:662
 vfs_get_tree+0x8c/0x2a0 fs/super.c:1784
 do_new_mount+0x2be/0xb40 fs/namespace.c:3352
 do_mount fs/namespace.c:3692 [inline]
 __do_sys_mount fs/namespace.c:3898 [inline]
 __se_sys_mount+0x2d9/0x3c0 fs/namespace.c:3875
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf5/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x63/0x6b
RIP: 0033:0x7f0966e7e4aa
Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb a6 e8 de 09 00 00 66 2e 0f 1f 84 00 
00 00 00 00 0f 1f 40 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 
c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:7f0967ca6ef8 EFLAGS: 0202 ORIG_RAX: 00a5
RAX: ffda RBX: 7f0967ca6f80 RCX: 7f0966e7e4aa
RDX: 2040 RSI: 2080 RDI: 7f0967ca6f40
RBP: 2040 R08: 7f0967ca6f80 R09: 
R10:  R11: 0202 R12: 2080
R13: 7f0967ca6f40 R14: 54f9 R15: 24c0
 

Allocated by task 9994:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x3f/0x70 mm/kasan/common.c:68
 poison_kmalloc_redzone mm/kasan/common.c:372 [inline]
 __kasan_kmalloc+0x98/0xb0 mm/kasan/common.c:389
 kasan_kmalloc include/linux/kasan.h:211 [inline]
 kmalloc_trace+0x1d6/0x360 mm/slub.c:4012
 kmalloc include/linux/slab.h:590 [inline]
 kzalloc include/linux/slab.h:711 [inline]
 f2fs_fill_super+0xce/0x8170 fs/f2fs/super.c:4397
 mount_bdev+0x206/0x2d0 fs/super.c:1663
 legacy_get_tree+0xef/0x190 fs/fs_context.c:662
 vfs_get_tree+0x8c/0x2a0 fs/super.c:1784
 do_new_mount+0x2be/0xb40 fs/namespace.c:3352
 do_mount fs/namespace.c:3692 [inline]
 __do_sys_mount fs/namespace.c:3898 [inline]
 __se_sys_mount+0x2d9/0x3c0 fs/namespace.c:3875
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf5/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x63/0x6b

Freed by task 9994:
 kasan_save_stack mm/kasan/common.c:47 [inline]
 kasan_save_track+0x3f/0x70 mm/kasan/common.c:68
 kasan_save_free_info+0x4e/0x60 mm/kasan/generic.c:634
 poison_slab_object+0xa6/0xe0 mm/kasan/common.c:241
 __kasan_slab_free+0x34/0x60 mm/kasan/common.c:257
 kasan_slab_free include/linux/kasan.h:184 [inline]
 slab_free_hook mm/slub.c:2121 [inline]
 slab_free mm/slub.c:4299 [inline]
 kfree+0x14a/0x380 mm/slub.c:4409
 f2fs_fill_super+0x6b04/0x8170 fs/f2fs/super.c:4882
 mount_bdev+0x206/0x2d0 fs/super.c:1663
 legacy_get_tree+0xef/0x190 fs/fs_context.c:662
 vfs_get_tree+0x8c/0x2a0 fs/super.c:1784
 do_new_mount+0x2be/0xb40 fs/namespace.c:3352
 do_mount fs/namespace.c:3692 [inline]
 __do_sys_mount fs/namespace.c:3898 [inline]
 __se_sys_mount+0x2d9/0x3c0 fs/namespace.c:3875
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf5/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x63/0x6b

The buggy address belongs to the object at 88801e12c000
 which belongs to the cache kmalloc-8k of size 8192
The buggy address is located 6012 bytes inside of
 freed 8192-byte region 

Re: [f2fs-dev] [GIT PULL] f2fs update for 6.8-rc1

2024-01-12 Thread Jaegeuk Kim
Posted this.
https://lore.kernel.org/lkml/20240112171645.3929428-1-jaeg...@kernel.org/T/#u

On 01/12, Jaegeuk Kim wrote:
> On 01/12, Al Viro wrote:
> > On Thu, Jan 11, 2024 at 09:05:51PM -0800, Linus Torvalds wrote:
> > > On Thu, 11 Jan 2024 at 10:28, Jaegeuk Kim  wrote:
> > > >
> > > >   git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git 
> > > > tags/f2fs-for-6.8-rc1
> > > 
> > > Hmm. I got a somewhat confusing conflict in f2fs_rename().
> > > 
> > > And honestly, I really don't know what the right resolution is. What I
> > > ended up with was this:
> > > 
> > > if (old_is_dir) {
> > > if (old_dir_entry)
> > > f2fs_set_link(old_inode, old_dir_entry,
> > > old_dir_page, new_dir);
> > > else
> > > f2fs_put_page(old_dir_page, 0);
> > 
> > Where would you end up with old_dir_page != NULL and old_dir_entry == NULL?
> > old_dir_page is initialized to NULL and the only place where it's altered
> > is
> > old_dir_entry = f2fs_parent_dir(old_inode, _dir_page);
> > Which is immediately followed by
> > if (!old_dir_entry) {
> > if (IS_ERR(old_dir_page))
> > err = PTR_ERR(old_dir_page);
> > goto out_old;
> > }
> > so we are *not* going to end up at that if (old_is_dir) in that case.
> 
> It seems [1] changed the condition of getting old_dir_page reference as below,
> which made f2fs_put_page(old_dir_page, 0) voided.
> 
> -   if (S_ISDIR(old_inode->i_mode)) {
> +   if (old_is_dir && old_dir != new_dir) {
> old_dir_entry = f2fs_parent_dir(old_inode, _dir_page);
> if (!old_dir_entry) {
> if (IS_ERR(old_dir_page))
> 
> [1] 7deee77b993a ("f2fs: Avoid reading renamed directory if parent does not 
> change")
> 
> > 
> > Original would have been more clear as
> > if (old_is_dir) {
> > if (old_dir != new_dir) {
> > /* we have .. in old_dir_page/old_dir_entry */
> > if (!whiteout)
> > f2fs_set_link(old_inode, old_dir_entry,
> > old_dir_page, new_dir);
> > else
> > f2fs_put_page(old_dir_page, 0);
> > }
> > f2fs_i_links_write(old_dir, false);
> > }
> > - it is equivalent to what that code used to do.  And "don't update ..
> > if we are leaving a whiteout behind" was teh bug fixed by commit
> > in f2fs tree...
> > 
> > The bottom line: your variant is not broken, but only because
> > f2fs_put_page() starts with
> > static inline void f2fs_put_page(struct page *page, int unlock)
> > {
> > if (!page)
> > return;
> > 
> > IOW, you are doing f2fs_put_page(NULL, 0), which is an explicit no-op.


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH] f2fs: remove unnecessary f2fs_put_page in f2fs_rename

2024-01-12 Thread Jaegeuk Kim
[1] changed the below condition, which made f2fs_put_page() voided.
This patch reapplies the AL's resolution in -next from [2].

-   if (S_ISDIR(old_inode->i_mode)) {
+   if (old_is_dir && old_dir != new_dir) {
old_dir_entry = f2fs_parent_dir(old_inode, _dir_page);
if (!old_dir_entry) {
if (IS_ERR(old_dir_page))

[1] 7deee77b993a ("f2fs: Avoid reading renamed directory if parent does not 
change")
[2] https://lore.kernel.org/all/20231220013402.GW1674809@ZenIV/

Suggested-by: Al Viro 
Signed-off-by: Jaegeuk Kim 
---
 fs/f2fs/namei.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index b3bb815fc6aa..ba11298b7837 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -1105,14 +1105,11 @@ static int f2fs_rename(struct mnt_idmap *idmap, struct 
inode *old_dir,
iput(whiteout);
}
 
-   if (old_is_dir) {
-   if (old_dir_entry)
-   f2fs_set_link(old_inode, old_dir_entry,
-   old_dir_page, new_dir);
-   else
-   f2fs_put_page(old_dir_page, 0);
+   if (old_dir_entry)
+   f2fs_set_link(old_inode, old_dir_entry, old_dir_page, new_dir);
+   if (old_is_dir)
f2fs_i_links_write(old_dir, false);
-   }
+
if (F2FS_OPTION(sbi).fsync_mode == FSYNC_MODE_STRICT) {
f2fs_add_ino_entry(sbi, new_dir->i_ino, TRANS_DIR_INO);
if (S_ISDIR(old_inode->i_mode))
-- 
2.43.0.275.g3460e3d667-goog



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [f2fs-dev] [GIT PULL] f2fs update for 6.8-rc1

2024-01-12 Thread Jaegeuk Kim
On 01/12, Al Viro wrote:
> On Thu, Jan 11, 2024 at 09:05:51PM -0800, Linus Torvalds wrote:
> > On Thu, 11 Jan 2024 at 10:28, Jaegeuk Kim  wrote:
> > >
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git 
> > > tags/f2fs-for-6.8-rc1
> > 
> > Hmm. I got a somewhat confusing conflict in f2fs_rename().
> > 
> > And honestly, I really don't know what the right resolution is. What I
> > ended up with was this:
> > 
> > if (old_is_dir) {
> > if (old_dir_entry)
> > f2fs_set_link(old_inode, old_dir_entry,
> > old_dir_page, new_dir);
> > else
> > f2fs_put_page(old_dir_page, 0);
> 
> Where would you end up with old_dir_page != NULL and old_dir_entry == NULL?
> old_dir_page is initialized to NULL and the only place where it's altered
> is
> old_dir_entry = f2fs_parent_dir(old_inode, _dir_page);
> Which is immediately followed by
> if (!old_dir_entry) {
> if (IS_ERR(old_dir_page))
> err = PTR_ERR(old_dir_page);
> goto out_old;
> }
> so we are *not* going to end up at that if (old_is_dir) in that case.

It seems [1] changed the condition of getting old_dir_page reference as below,
which made f2fs_put_page(old_dir_page, 0) voided.

-   if (S_ISDIR(old_inode->i_mode)) {
+   if (old_is_dir && old_dir != new_dir) {
old_dir_entry = f2fs_parent_dir(old_inode, _dir_page);
if (!old_dir_entry) {
if (IS_ERR(old_dir_page))

[1] 7deee77b993a ("f2fs: Avoid reading renamed directory if parent does not 
change")

> 
> Original would have been more clear as
>   if (old_is_dir) {
>   if (old_dir != new_dir) {
>   /* we have .. in old_dir_page/old_dir_entry */
>   if (!whiteout)
>   f2fs_set_link(old_inode, old_dir_entry,
> old_dir_page, new_dir);
>   else
>   f2fs_put_page(old_dir_page, 0);
>   }
> f2fs_i_links_write(old_dir, false);
>   }
> - it is equivalent to what that code used to do.  And "don't update ..
> if we are leaving a whiteout behind" was teh bug fixed by commit
> in f2fs tree...
> 
> The bottom line: your variant is not broken, but only because
> f2fs_put_page() starts with
> static inline void f2fs_put_page(struct page *page, int unlock)
> {
> if (!page)
> return;
> 
> IOW, you are doing f2fs_put_page(NULL, 0), which is an explicit no-op.


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


Re: [PATCH v2 0/4] xfstests: test custom crypto data unit size

2024-01-12 Thread Zorro Lang
On Wed, Jan 10, 2024 at 07:54:44PM -0800, Eric Biggers wrote:
> On Tue, Nov 21, 2023 at 02:39:05PM -0800, Eric Biggers wrote:
> > This series adds a test that verifies the on-disk format of encrypted
> > files that use a crypto data unit size that differs from the filesystem
> > block size.  This tests the functionality that was introduced in Linux
> > 6.7 by kernel commit 5b1188847180 ("fscrypt: support crypto data unit
> > size less than filesystem block size").
> 
> Hi Zorro, can you consider applying this series?  It's been out for review for
> 3 months, so I don't think reviews are going to come in.  The prerequisite
> xfsprogs patch is already present on the for-next branch of xfsprogs.

Sure Eric, I thought about this patchset last week, I saw that xfsprogs
patchset has been merge. I'll give this patchset a basic test, to make
sure it won't break xfstests, then merge it.

Thanks, and sorry this late.
Zorro

> 
> Thanks!
> 
> - Eric
>