* [PATCH] f2fs: compress: fix UAF of f2fs_inode_info in f2fs_free_dic
@ 2025-06-04 5:54 Zhiguo Niu
2025-06-04 9:48 ` Chao Yu
0 siblings, 1 reply; 5+ messages in thread
From: Zhiguo Niu @ 2025-06-04 5:54 UTC (permalink / raw)
To: jaegeuk, chao
Cc: linux-f2fs-devel, linux-kernel, niuzhiguo84, zhiguo.niu, ke.wang,
Hao_hao.Wang, baocong.liu
The decompress_io_ctx may be released asynchronously after
I/O completion. If this file is deleted immediately after read,
and the kworker of processing post_read_wq has not been executed yet
due to high workloads, It is possible that the inode(f2fs_inode_info)
is evicted and freed before it is used f2fs_free_dic.
The UAF case as below:
Thread A Thread B
- f2fs_decompress_end_io
- f2fs_put_dic
- queue_work
add free_dic work to post_read_wq
- do_unlink
- iput
- evict
- call_rcu
This file is deleted after read.
Thread C kworker to process post_read_wq
- rcu_do_batch
- f2fs_free_inode
- kmem_cache_free
inode is freed by rcu
- process_scheduled_works
- f2fs_late_free_dic
- f2fs_free_dic
- f2fs_release_decomp_mem
read (dic->inode)->i_compress_algorithm
This patch increase inode->i_count before f2fs_free_dic and decrease it
after free the dic.
Cc: Daeho Jeong <daehojeong@google.com>
Fixes: bff139b49d9f ("f2fs: handle decompress only post processing in softirq")
Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com>
Signed-off-by: Baocong Liu <baocong.liu@unisoc.com>
---
fs/f2fs/compress.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index b3c1df9..6b3b3a7 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1687,7 +1687,7 @@ static void f2fs_release_decomp_mem(struct decompress_io_ctx *dic,
}
static void f2fs_free_dic(struct decompress_io_ctx *dic,
- bool bypass_destroy_callback);
+ bool bypass_destroy_callback, bool late_free);
struct decompress_io_ctx *f2fs_alloc_dic(struct compress_ctx *cc)
{
@@ -1743,12 +1743,12 @@ struct decompress_io_ctx *f2fs_alloc_dic(struct compress_ctx *cc)
return dic;
out_free:
- f2fs_free_dic(dic, true);
+ f2fs_free_dic(dic, true, false);
return ERR_PTR(ret);
}
static void f2fs_free_dic(struct decompress_io_ctx *dic,
- bool bypass_destroy_callback)
+ bool bypass_destroy_callback, bool late_free)
{
int i;
@@ -1775,6 +1775,11 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic,
}
page_array_free(dic->inode, dic->rpages, dic->nr_rpages);
+ if (late_free) {
+ spin_lock(&dic->inode->i_lock);
+ atomic_dec(&dic->inode->i_count);
+ spin_unlock(&dic->inode->i_lock);
+ }
kmem_cache_free(dic_entry_slab, dic);
}
@@ -1783,16 +1788,20 @@ static void f2fs_late_free_dic(struct work_struct *work)
struct decompress_io_ctx *dic =
container_of(work, struct decompress_io_ctx, free_work);
- f2fs_free_dic(dic, false);
+ f2fs_free_dic(dic, false, true);
}
static void f2fs_put_dic(struct decompress_io_ctx *dic, bool in_task)
{
if (refcount_dec_and_test(&dic->refcnt)) {
if (in_task) {
- f2fs_free_dic(dic, false);
+ f2fs_free_dic(dic, false, false);
} else {
INIT_WORK(&dic->free_work, f2fs_late_free_dic);
+ /* to avoid inode is evicted simultaneously */
+ spin_lock(&dic->inode->i_lock);
+ atomic_inc(&dic->inode->i_count);
+ spin_unlock(&dic->inode->i_lock);
queue_work(F2FS_I_SB(dic->inode)->post_read_wq,
&dic->free_work);
}
--
1.9.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] f2fs: compress: fix UAF of f2fs_inode_info in f2fs_free_dic 2025-06-04 5:54 [PATCH] f2fs: compress: fix UAF of f2fs_inode_info in f2fs_free_dic Zhiguo Niu @ 2025-06-04 9:48 ` Chao Yu 2025-06-04 10:49 ` Zhiguo Niu 0 siblings, 1 reply; 5+ messages in thread From: Chao Yu @ 2025-06-04 9:48 UTC (permalink / raw) To: Zhiguo Niu, jaegeuk Cc: chao, linux-f2fs-devel, linux-kernel, niuzhiguo84, ke.wang, Hao_hao.Wang, baocong.liu On 6/4/25 13:54, Zhiguo Niu wrote: > The decompress_io_ctx may be released asynchronously after > I/O completion. If this file is deleted immediately after read, > and the kworker of processing post_read_wq has not been executed yet > due to high workloads, It is possible that the inode(f2fs_inode_info) > is evicted and freed before it is used f2fs_free_dic. > > The UAF case as below: > Thread A Thread B > - f2fs_decompress_end_io > - f2fs_put_dic > - queue_work > add free_dic work to post_read_wq > - do_unlink > - iput > - evict > - call_rcu > This file is deleted after read. > > Thread C kworker to process post_read_wq > - rcu_do_batch > - f2fs_free_inode > - kmem_cache_free > inode is freed by rcu > - process_scheduled_works > - f2fs_late_free_dic > - f2fs_free_dic > - f2fs_release_decomp_mem > read (dic->inode)->i_compress_algorithm > > This patch increase inode->i_count before f2fs_free_dic and decrease it > after free the dic. > > Cc: Daeho Jeong <daehojeong@google.com> > Fixes: bff139b49d9f ("f2fs: handle decompress only post processing in softirq") > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > Signed-off-by: Baocong Liu <baocong.liu@unisoc.com> > --- > fs/f2fs/compress.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c > index b3c1df9..6b3b3a7 100644 > --- a/fs/f2fs/compress.c > +++ b/fs/f2fs/compress.c > @@ -1687,7 +1687,7 @@ static void f2fs_release_decomp_mem(struct decompress_io_ctx *dic, > } > > static void f2fs_free_dic(struct decompress_io_ctx *dic, > - bool bypass_destroy_callback); > + bool bypass_destroy_callback, bool late_free); > > struct decompress_io_ctx *f2fs_alloc_dic(struct compress_ctx *cc) > { > @@ -1743,12 +1743,12 @@ struct decompress_io_ctx *f2fs_alloc_dic(struct compress_ctx *cc) > return dic; > > out_free: > - f2fs_free_dic(dic, true); > + f2fs_free_dic(dic, true, false); > return ERR_PTR(ret); > } > > static void f2fs_free_dic(struct decompress_io_ctx *dic, > - bool bypass_destroy_callback) > + bool bypass_destroy_callback, bool late_free) > { > int i; > > @@ -1775,6 +1775,11 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic, > } > > page_array_free(dic->inode, dic->rpages, dic->nr_rpages); > + if (late_free) { > + spin_lock(&dic->inode->i_lock); > + atomic_dec(&dic->inode->i_count); > + spin_unlock(&dic->inode->i_lock); If it is the last one release i_count, it needs to call iput_final to evict inode like what iput did, so we'd better to call iput() here? > + } > kmem_cache_free(dic_entry_slab, dic); > } > > @@ -1783,16 +1788,20 @@ static void f2fs_late_free_dic(struct work_struct *work) > struct decompress_io_ctx *dic = > container_of(work, struct decompress_io_ctx, free_work); > > - f2fs_free_dic(dic, false); > + f2fs_free_dic(dic, false, true); > } > > static void f2fs_put_dic(struct decompress_io_ctx *dic, bool in_task) > { > if (refcount_dec_and_test(&dic->refcnt)) { > if (in_task) { > - f2fs_free_dic(dic, false); > + f2fs_free_dic(dic, false, false); > } else { > INIT_WORK(&dic->free_work, f2fs_late_free_dic); > + /* to avoid inode is evicted simultaneously */ > + spin_lock(&dic->inode->i_lock); > + atomic_inc(&dic->inode->i_count); > + spin_unlock(&dic->inode->i_lock); iget()? BTW, can we store i_compress_algorithm in dic to avoid inode access? Thanks, > queue_work(F2FS_I_SB(dic->inode)->post_read_wq, > &dic->free_work); > } ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] f2fs: compress: fix UAF of f2fs_inode_info in f2fs_free_dic 2025-06-04 9:48 ` Chao Yu @ 2025-06-04 10:49 ` Zhiguo Niu 2025-06-04 11:09 ` Chao Yu 0 siblings, 1 reply; 5+ messages in thread From: Zhiguo Niu @ 2025-06-04 10:49 UTC (permalink / raw) To: Chao Yu Cc: Zhiguo Niu, jaegeuk, linux-f2fs-devel, linux-kernel, ke.wang, Hao_hao.Wang, baocong.liu Chao Yu <chao@kernel.org> 于2025年6月4日周三 17:48写道: > > On 6/4/25 13:54, Zhiguo Niu wrote: > > The decompress_io_ctx may be released asynchronously after > > I/O completion. If this file is deleted immediately after read, > > and the kworker of processing post_read_wq has not been executed yet > > due to high workloads, It is possible that the inode(f2fs_inode_info) > > is evicted and freed before it is used f2fs_free_dic. > > > > The UAF case as below: > > Thread A Thread B > > - f2fs_decompress_end_io > > - f2fs_put_dic > > - queue_work > > add free_dic work to post_read_wq > > - do_unlink > > - iput > > - evict > > - call_rcu > > This file is deleted after read. > > > > Thread C kworker to process post_read_wq > > - rcu_do_batch > > - f2fs_free_inode > > - kmem_cache_free > > inode is freed by rcu > > - process_scheduled_works > > - f2fs_late_free_dic > > - f2fs_free_dic > > - f2fs_release_decomp_mem > > read (dic->inode)->i_compress_algorithm > > > > This patch increase inode->i_count before f2fs_free_dic and decrease it > > after free the dic. > > > > Cc: Daeho Jeong <daehojeong@google.com> > > Fixes: bff139b49d9f ("f2fs: handle decompress only post processing in softirq") > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > > Signed-off-by: Baocong Liu <baocong.liu@unisoc.com> > > --- > > fs/f2fs/compress.c | 19 ++++++++++++++----- > > 1 file changed, 14 insertions(+), 5 deletions(-) > > > > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c > > index b3c1df9..6b3b3a7 100644 > > --- a/fs/f2fs/compress.c > > +++ b/fs/f2fs/compress.c > > @@ -1687,7 +1687,7 @@ static void f2fs_release_decomp_mem(struct decompress_io_ctx *dic, > > } > > > > static void f2fs_free_dic(struct decompress_io_ctx *dic, > > - bool bypass_destroy_callback); > > + bool bypass_destroy_callback, bool late_free); > > > > struct decompress_io_ctx *f2fs_alloc_dic(struct compress_ctx *cc) > > { > > @@ -1743,12 +1743,12 @@ struct decompress_io_ctx *f2fs_alloc_dic(struct compress_ctx *cc) > > return dic; > > > > out_free: > > - f2fs_free_dic(dic, true); > > + f2fs_free_dic(dic, true, false); > > return ERR_PTR(ret); > > } > > > > static void f2fs_free_dic(struct decompress_io_ctx *dic, > > - bool bypass_destroy_callback) > > + bool bypass_destroy_callback, bool late_free) > > { > > int i; > > > > @@ -1775,6 +1775,11 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic, > > } > > > > page_array_free(dic->inode, dic->rpages, dic->nr_rpages); > > + if (late_free) { > > + spin_lock(&dic->inode->i_lock); > > + atomic_dec(&dic->inode->i_count); > > + spin_unlock(&dic->inode->i_lock); > > If it is the last one release i_count, it needs to call iput_final to evict inode > like what iput did, so we'd better to call iput() here? Hi Chao, Yes, we have also tested this method(iput/__iget), and it worked. Just think It is simpler and easier to read to directly operate i_count, and then free it by relying on the memory module when i_count=0. But It seems iput/__iget is better. > > > + } > > kmem_cache_free(dic_entry_slab, dic); > > } > > > > @@ -1783,16 +1788,20 @@ static void f2fs_late_free_dic(struct work_struct *work) > > struct decompress_io_ctx *dic = > > container_of(work, struct decompress_io_ctx, free_work); > > > > - f2fs_free_dic(dic, false); > > + f2fs_free_dic(dic, false, true); > > } > > > > static void f2fs_put_dic(struct decompress_io_ctx *dic, bool in_task) > > { > > if (refcount_dec_and_test(&dic->refcnt)) { > > if (in_task) { > > - f2fs_free_dic(dic, false); > > + f2fs_free_dic(dic, false, false); > > } else { > > INIT_WORK(&dic->free_work, f2fs_late_free_dic); > > + /* to avoid inode is evicted simultaneously */ > > + spin_lock(&dic->inode->i_lock); > > + atomic_inc(&dic->inode->i_count); > > + spin_unlock(&dic->inode->i_lock); > > iget()? > > BTW, can we store i_compress_algorithm in dic to avoid inode access? Also thought of this method, but it would require more changes. dic->inode used in f2fs_free_dic are all needed to modify except i_compress_algorithm. such as page_array_free(dic->inode), allow_memalloc_for_decomp(F2FS_I_SB(dic->inode)). Do you have any other suggestions? thanks! > > Thanks, > > > queue_work(F2FS_I_SB(dic->inode)->post_read_wq, > > &dic->free_work); > > } > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] f2fs: compress: fix UAF of f2fs_inode_info in f2fs_free_dic 2025-06-04 10:49 ` Zhiguo Niu @ 2025-06-04 11:09 ` Chao Yu 2025-06-04 11:14 ` Zhiguo Niu 0 siblings, 1 reply; 5+ messages in thread From: Chao Yu @ 2025-06-04 11:09 UTC (permalink / raw) To: Zhiguo Niu Cc: chao, Zhiguo Niu, jaegeuk, linux-f2fs-devel, linux-kernel, ke.wang, Hao_hao.Wang, baocong.liu On 6/4/25 18:49, Zhiguo Niu wrote: > Chao Yu <chao@kernel.org> 于2025年6月4日周三 17:48写道: >> >> On 6/4/25 13:54, Zhiguo Niu wrote: >>> The decompress_io_ctx may be released asynchronously after >>> I/O completion. If this file is deleted immediately after read, >>> and the kworker of processing post_read_wq has not been executed yet >>> due to high workloads, It is possible that the inode(f2fs_inode_info) >>> is evicted and freed before it is used f2fs_free_dic. >>> >>> The UAF case as below: >>> Thread A Thread B >>> - f2fs_decompress_end_io >>> - f2fs_put_dic >>> - queue_work >>> add free_dic work to post_read_wq >>> - do_unlink >>> - iput >>> - evict >>> - call_rcu >>> This file is deleted after read. >>> >>> Thread C kworker to process post_read_wq >>> - rcu_do_batch >>> - f2fs_free_inode >>> - kmem_cache_free >>> inode is freed by rcu >>> - process_scheduled_works >>> - f2fs_late_free_dic >>> - f2fs_free_dic >>> - f2fs_release_decomp_mem >>> read (dic->inode)->i_compress_algorithm >>> >>> This patch increase inode->i_count before f2fs_free_dic and decrease it >>> after free the dic. >>> >>> Cc: Daeho Jeong <daehojeong@google.com> >>> Fixes: bff139b49d9f ("f2fs: handle decompress only post processing in softirq") >>> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> >>> Signed-off-by: Baocong Liu <baocong.liu@unisoc.com> >>> --- >>> fs/f2fs/compress.c | 19 ++++++++++++++----- >>> 1 file changed, 14 insertions(+), 5 deletions(-) >>> >>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c >>> index b3c1df9..6b3b3a7 100644 >>> --- a/fs/f2fs/compress.c >>> +++ b/fs/f2fs/compress.c >>> @@ -1687,7 +1687,7 @@ static void f2fs_release_decomp_mem(struct decompress_io_ctx *dic, >>> } >>> >>> static void f2fs_free_dic(struct decompress_io_ctx *dic, >>> - bool bypass_destroy_callback); >>> + bool bypass_destroy_callback, bool late_free); >>> >>> struct decompress_io_ctx *f2fs_alloc_dic(struct compress_ctx *cc) >>> { >>> @@ -1743,12 +1743,12 @@ struct decompress_io_ctx *f2fs_alloc_dic(struct compress_ctx *cc) >>> return dic; >>> >>> out_free: >>> - f2fs_free_dic(dic, true); >>> + f2fs_free_dic(dic, true, false); >>> return ERR_PTR(ret); >>> } >>> >>> static void f2fs_free_dic(struct decompress_io_ctx *dic, >>> - bool bypass_destroy_callback) >>> + bool bypass_destroy_callback, bool late_free) >>> { >>> int i; >>> >>> @@ -1775,6 +1775,11 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic, >>> } >>> >>> page_array_free(dic->inode, dic->rpages, dic->nr_rpages); >>> + if (late_free) { >>> + spin_lock(&dic->inode->i_lock); >>> + atomic_dec(&dic->inode->i_count); >>> + spin_unlock(&dic->inode->i_lock); >> >> If it is the last one release i_count, it needs to call iput_final to evict inode >> like what iput did, so we'd better to call iput() here? > Hi Chao, > Yes, we have also tested this method(iput/__iget), and it worked. > Just think It is simpler and easier to read to directly operate > i_count, and then free it > by relying on the memory module when i_count=0. > But It seems iput/__iget is better. >> >>> + } >>> kmem_cache_free(dic_entry_slab, dic); >>> } >>> >>> @@ -1783,16 +1788,20 @@ static void f2fs_late_free_dic(struct work_struct *work) >>> struct decompress_io_ctx *dic = >>> container_of(work, struct decompress_io_ctx, free_work); >>> >>> - f2fs_free_dic(dic, false); >>> + f2fs_free_dic(dic, false, true); >>> } >>> >>> static void f2fs_put_dic(struct decompress_io_ctx *dic, bool in_task) >>> { >>> if (refcount_dec_and_test(&dic->refcnt)) { >>> if (in_task) { >>> - f2fs_free_dic(dic, false); >>> + f2fs_free_dic(dic, false, false); >>> } else { >>> INIT_WORK(&dic->free_work, f2fs_late_free_dic); >>> + /* to avoid inode is evicted simultaneously */ >>> + spin_lock(&dic->inode->i_lock); >>> + atomic_inc(&dic->inode->i_count); >>> + spin_unlock(&dic->inode->i_lock); >> >> iget()? >> >> BTW, can we store i_compress_algorithm in dic to avoid inode access? > > Also thought of this method, but it would require more changes. > dic->inode used in f2fs_free_dic are all needed to modify except > i_compress_algorithm. > such as page_array_free(dic->inode), Zhiguo, page_array_free() parses dic->inode to get sbi only, so we can pass sbi to page_array_free() directly to avoid using dic->inode. > allow_memalloc_for_decomp(F2FS_I_SB(dic->inode)). > > Do you have any other suggestions? Using iget/iput looks fine to me, please go ahead. Thanks, > thanks! > >> >> Thanks, >> >>> queue_work(F2FS_I_SB(dic->inode)->post_read_wq, >>> &dic->free_work); >>> } >> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] f2fs: compress: fix UAF of f2fs_inode_info in f2fs_free_dic 2025-06-04 11:09 ` Chao Yu @ 2025-06-04 11:14 ` Zhiguo Niu 0 siblings, 0 replies; 5+ messages in thread From: Zhiguo Niu @ 2025-06-04 11:14 UTC (permalink / raw) To: Chao Yu Cc: Zhiguo Niu, jaegeuk, linux-f2fs-devel, linux-kernel, ke.wang, Hao_hao.Wang, baocong.liu Chao Yu <chao@kernel.org> 于2025年6月4日周三 19:09写道: > > On 6/4/25 18:49, Zhiguo Niu wrote: > > Chao Yu <chao@kernel.org> 于2025年6月4日周三 17:48写道: > >> > >> On 6/4/25 13:54, Zhiguo Niu wrote: > >>> The decompress_io_ctx may be released asynchronously after > >>> I/O completion. If this file is deleted immediately after read, > >>> and the kworker of processing post_read_wq has not been executed yet > >>> due to high workloads, It is possible that the inode(f2fs_inode_info) > >>> is evicted and freed before it is used f2fs_free_dic. > >>> > >>> The UAF case as below: > >>> Thread A Thread B > >>> - f2fs_decompress_end_io > >>> - f2fs_put_dic > >>> - queue_work > >>> add free_dic work to post_read_wq > >>> - do_unlink > >>> - iput > >>> - evict > >>> - call_rcu > >>> This file is deleted after read. > >>> > >>> Thread C kworker to process post_read_wq > >>> - rcu_do_batch > >>> - f2fs_free_inode > >>> - kmem_cache_free > >>> inode is freed by rcu > >>> - process_scheduled_works > >>> - f2fs_late_free_dic > >>> - f2fs_free_dic > >>> - f2fs_release_decomp_mem > >>> read (dic->inode)->i_compress_algorithm > >>> > >>> This patch increase inode->i_count before f2fs_free_dic and decrease it > >>> after free the dic. > >>> > >>> Cc: Daeho Jeong <daehojeong@google.com> > >>> Fixes: bff139b49d9f ("f2fs: handle decompress only post processing in softirq") > >>> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > >>> Signed-off-by: Baocong Liu <baocong.liu@unisoc.com> > >>> --- > >>> fs/f2fs/compress.c | 19 ++++++++++++++----- > >>> 1 file changed, 14 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c > >>> index b3c1df9..6b3b3a7 100644 > >>> --- a/fs/f2fs/compress.c > >>> +++ b/fs/f2fs/compress.c > >>> @@ -1687,7 +1687,7 @@ static void f2fs_release_decomp_mem(struct decompress_io_ctx *dic, > >>> } > >>> > >>> static void f2fs_free_dic(struct decompress_io_ctx *dic, > >>> - bool bypass_destroy_callback); > >>> + bool bypass_destroy_callback, bool late_free); > >>> > >>> struct decompress_io_ctx *f2fs_alloc_dic(struct compress_ctx *cc) > >>> { > >>> @@ -1743,12 +1743,12 @@ struct decompress_io_ctx *f2fs_alloc_dic(struct compress_ctx *cc) > >>> return dic; > >>> > >>> out_free: > >>> - f2fs_free_dic(dic, true); > >>> + f2fs_free_dic(dic, true, false); > >>> return ERR_PTR(ret); > >>> } > >>> > >>> static void f2fs_free_dic(struct decompress_io_ctx *dic, > >>> - bool bypass_destroy_callback) > >>> + bool bypass_destroy_callback, bool late_free) > >>> { > >>> int i; > >>> > >>> @@ -1775,6 +1775,11 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic, > >>> } > >>> > >>> page_array_free(dic->inode, dic->rpages, dic->nr_rpages); > >>> + if (late_free) { > >>> + spin_lock(&dic->inode->i_lock); > >>> + atomic_dec(&dic->inode->i_count); > >>> + spin_unlock(&dic->inode->i_lock); > >> > >> If it is the last one release i_count, it needs to call iput_final to evict inode > >> like what iput did, so we'd better to call iput() here? > > Hi Chao, > > Yes, we have also tested this method(iput/__iget), and it worked. > > Just think It is simpler and easier to read to directly operate > > i_count, and then free it > > by relying on the memory module when i_count=0. > > But It seems iput/__iget is better. > >> > >>> + } > >>> kmem_cache_free(dic_entry_slab, dic); > >>> } > >>> > >>> @@ -1783,16 +1788,20 @@ static void f2fs_late_free_dic(struct work_struct *work) > >>> struct decompress_io_ctx *dic = > >>> container_of(work, struct decompress_io_ctx, free_work); > >>> > >>> - f2fs_free_dic(dic, false); > >>> + f2fs_free_dic(dic, false, true); > >>> } > >>> > >>> static void f2fs_put_dic(struct decompress_io_ctx *dic, bool in_task) > >>> { > >>> if (refcount_dec_and_test(&dic->refcnt)) { > >>> if (in_task) { > >>> - f2fs_free_dic(dic, false); > >>> + f2fs_free_dic(dic, false, false); > >>> } else { > >>> INIT_WORK(&dic->free_work, f2fs_late_free_dic); > >>> + /* to avoid inode is evicted simultaneously */ > >>> + spin_lock(&dic->inode->i_lock); > >>> + atomic_inc(&dic->inode->i_count); > >>> + spin_unlock(&dic->inode->i_lock); > >> > >> iget()? > >> > >> BTW, can we store i_compress_algorithm in dic to avoid inode access? > > > > Also thought of this method, but it would require more changes. > > dic->inode used in f2fs_free_dic are all needed to modify except > > i_compress_algorithm. > > such as page_array_free(dic->inode), > > Zhiguo, > > page_array_free() parses dic->inode to get sbi only, so we can pass sbi to > page_array_free() directly to avoid using dic->inode. Hi Chao, but now sbi is not in dic structure, so we also need to add it ? > > > allow_memalloc_for_decomp(F2FS_I_SB(dic->inode)). > > > > Do you have any other suggestions? > > Using iget/iput looks fine to me, please go ahead. OK~ thanks! > > Thanks, > > > thanks! > > > >> > >> Thanks, > >> > >>> queue_work(F2FS_I_SB(dic->inode)->post_read_wq, > >>> &dic->free_work); > >>> } > >> > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-06-04 11:14 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-06-04 5:54 [PATCH] f2fs: compress: fix UAF of f2fs_inode_info in f2fs_free_dic Zhiguo Niu 2025-06-04 9:48 ` Chao Yu 2025-06-04 10:49 ` Zhiguo Niu 2025-06-04 11:09 ` Chao Yu 2025-06-04 11:14 ` Zhiguo Niu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®