fs/f2fs/compress.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
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 use igrab before f2fs_free_dic and iput after free the dic when dic free
action is done by kworker.
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>
---
v3: use igrab to replace __iget
v2: use __iget/iput function
---
fs/f2fs/compress.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index b3c1df9..729ad16 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,8 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic,
}
page_array_free(dic->inode, dic->rpages, dic->nr_rpages);
+ if (late_free)
+ iput(dic->inode);
kmem_cache_free(dic_entry_slab, dic);
}
@@ -1783,16 +1785,18 @@ 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);
+ /* use igrab to avoid inode is evicted simultaneously */
+ f2fs_bug_on(F2FS_I_SB(dic->inode), !igrab(dic->inode));
queue_work(F2FS_I_SB(dic->inode)->post_read_wq,
&dic->free_work);
}
--
1.9.1Hi Zhiguo,
This patch causes CPU hang when running fsstress on compressed/non-compressed
files. Please check.
On 06/05, 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 use igrab before f2fs_free_dic and iput after free the dic when dic free
> action is done by kworker.
>
> 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>
> ---
> v3: use igrab to replace __iget
> v2: use __iget/iput function
> ---
> fs/f2fs/compress.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index b3c1df9..729ad16 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,8 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic,
> }
>
> page_array_free(dic->inode, dic->rpages, dic->nr_rpages);
> + if (late_free)
> + iput(dic->inode);
> kmem_cache_free(dic_entry_slab, dic);
> }
>
> @@ -1783,16 +1785,18 @@ 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);
> + /* use igrab to avoid inode is evicted simultaneously */
> + f2fs_bug_on(F2FS_I_SB(dic->inode), !igrab(dic->inode));
> queue_work(F2FS_I_SB(dic->inode)->post_read_wq,
> &dic->free_work);
> }
> --
> 1.9.1
On 6/11/25 00:08, Jaegeuk Kim wrote:
> Hi Zhiguo,
>
> This patch causes CPU hang when running fsstress on compressed/non-compressed
> files. Please check.
Oh, seems it may cause below deadlock:
CPU0
process A
- spin_lock(i_lock)
software IRQ
- end_io
- igrab
- spin_lock(i_lock)
Thanks,
>
> On 06/05, 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 use igrab before f2fs_free_dic and iput after free the dic when dic free
>> action is done by kworker.
>>
>> 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>
>> ---
>> v3: use igrab to replace __iget
>> v2: use __iget/iput function
>> ---
>> fs/f2fs/compress.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>> index b3c1df9..729ad16 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,8 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic,
>> }
>>
>> page_array_free(dic->inode, dic->rpages, dic->nr_rpages);
>> + if (late_free)
>> + iput(dic->inode);
>> kmem_cache_free(dic_entry_slab, dic);
>> }
>>
>> @@ -1783,16 +1785,18 @@ 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);
>> + /* use igrab to avoid inode is evicted simultaneously */
>> + f2fs_bug_on(F2FS_I_SB(dic->inode), !igrab(dic->inode));
>> queue_work(F2FS_I_SB(dic->inode)->post_read_wq,
>> &dic->free_work);
>> }
>> --
>> 1.9.1
Chao Yu <chao@kernel.org> 于2025年6月11日周三 14:07写道：
>
> On 6/11/25 00:08, Jaegeuk Kim wrote:
> > Hi Zhiguo,
> >
> > This patch causes CPU hang when running fsstress on compressed/non-compressed
> > files. Please check.
>
> Oh, seems it may cause below deadlock:
>
> CPU0
> process A
> - spin_lock(i_lock)
> software IRQ
> - end_io
> - igrab
> - spin_lock(i_lock)
>
> Thanks,
Hi Chao,
Thanks for pointing this out.
I have tested this patch locally about some basic cases before submission.
So it seems that should use the following method to solve this problem?
" store i_compress_algorithm/sbi in dic to avoid inode access?"
thanks！
>
> >
> > On 06/05, 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 use igrab before f2fs_free_dic and iput after free the dic when dic free
> >> action is done by kworker.
> >>
> >> 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>
> >> ---
> >> v3: use igrab to replace __iget
> >> v2: use __iget/iput function
> >> ---
> >> fs/f2fs/compress.c | 14 +++++++++-----
> >> 1 file changed, 9 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> >> index b3c1df9..729ad16 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,8 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic,
> >> }
> >>
> >> page_array_free(dic->inode, dic->rpages, dic->nr_rpages);
> >> + if (late_free)
> >> + iput(dic->inode);
> >> kmem_cache_free(dic_entry_slab, dic);
> >> }
> >>
> >> @@ -1783,16 +1785,18 @@ 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);
> >> + /* use igrab to avoid inode is evicted simultaneously */
> >> + f2fs_bug_on(F2FS_I_SB(dic->inode), !igrab(dic->inode));
> >> queue_work(F2FS_I_SB(dic->inode)->post_read_wq,
> >> &dic->free_work);
> >> }
> >> --
> >> 1.9.1
>
On 6/11/25 14:41, Zhiguo Niu wrote:
> Chao Yu <chao@kernel.org> 于2025年6月11日周三 14:07写道：
>>
>> On 6/11/25 00:08, Jaegeuk Kim wrote:
>>> Hi Zhiguo,
>>>
>>> This patch causes CPU hang when running fsstress on compressed/non-compressed
>>> files. Please check.
>>
>> Oh, seems it may cause below deadlock:
>>
>> CPU0
>> process A
>> - spin_lock(i_lock)
>> software IRQ
>> - end_io
>> - igrab
>> - spin_lock(i_lock)
>>
>> Thanks,
> Hi Chao,
> Thanks for pointing this out.
> I have tested this patch locally about some basic cases before submission.
> So it seems that should use the following method to solve this problem?
> " store i_compress_algorithm/sbi in dic to avoid inode access?"
Zhiguo,
Yeah, I guess so.
Thanks,
> thanks！
>
>
>>
>>>
>>> On 06/05, 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 use igrab before f2fs_free_dic and iput after free the dic when dic free
>>>> action is done by kworker.
>>>>
>>>> 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>
>>>> ---
>>>> v3: use igrab to replace __iget
>>>> v2: use __iget/iput function
>>>> ---
>>>> fs/f2fs/compress.c | 14 +++++++++-----
>>>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>>>> index b3c1df9..729ad16 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,8 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic,
>>>> }
>>>>
>>>> page_array_free(dic->inode, dic->rpages, dic->nr_rpages);
>>>> + if (late_free)
>>>> + iput(dic->inode);
>>>> kmem_cache_free(dic_entry_slab, dic);
>>>> }
>>>>
>>>> @@ -1783,16 +1785,18 @@ 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);
>>>> + /* use igrab to avoid inode is evicted simultaneously */
>>>> + f2fs_bug_on(F2FS_I_SB(dic->inode), !igrab(dic->inode));
>>>> queue_work(F2FS_I_SB(dic->inode)->post_read_wq,
>>>> &dic->free_work);
>>>> }
>>>> --
>>>> 1.9.1
>>
Chao Yu <chao@kernel.org> 于2025年6月11日周三 14:47写道：
>
> On 6/11/25 14:41, Zhiguo Niu wrote:
> > Chao Yu <chao@kernel.org> 于2025年6月11日周三 14:07写道：
> >>
> >> On 6/11/25 00:08, Jaegeuk Kim wrote:
> >>> Hi Zhiguo,
> >>>
> >>> This patch causes CPU hang when running fsstress on compressed/non-compressed
> >>> files. Please check.
> >>
> >> Oh, seems it may cause below deadlock:
> >>
> >> CPU0
> >> process A
> >> - spin_lock(i_lock)
> >> software IRQ
> >> - end_io
> >> - igrab
> >> - spin_lock(i_lock)
> >>
> >> Thanks,
> > Hi Chao,
> > Thanks for pointing this out.
> > I have tested this patch locally about some basic cases before submission.
> > So it seems that should use the following method to solve this problem?
> > " store i_compress_algorithm/sbi in dic to avoid inode access?"
>
> Zhiguo,
>
> Yeah, I guess so.
Hi Chao,
OK, I will prepare it .
Thanks a lot.
>
> Thanks,
>
> > thanks！
> >
> >
> >>
> >>>
> >>> On 06/05, 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 use igrab before f2fs_free_dic and iput after free the dic when dic free
> >>>> action is done by kworker.
> >>>>
> >>>> 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>
> >>>> ---
> >>>> v3: use igrab to replace __iget
> >>>> v2: use __iget/iput function
> >>>> ---
> >>>> fs/f2fs/compress.c | 14 +++++++++-----
> >>>> 1 file changed, 9 insertions(+), 5 deletions(-)
> >>>>
> >>>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> >>>> index b3c1df9..729ad16 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,8 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic,
> >>>> }
> >>>>
> >>>> page_array_free(dic->inode, dic->rpages, dic->nr_rpages);
> >>>> + if (late_free)
> >>>> + iput(dic->inode);
> >>>> kmem_cache_free(dic_entry_slab, dic);
> >>>> }
> >>>>
> >>>> @@ -1783,16 +1785,18 @@ 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);
> >>>> + /* use igrab to avoid inode is evicted simultaneously */
> >>>> + f2fs_bug_on(F2FS_I_SB(dic->inode), !igrab(dic->inode));
> >>>> queue_work(F2FS_I_SB(dic->inode)->post_read_wq,
> >>>> &dic->free_work);
> >>>> }
> >>>> --
> >>>> 1.9.1
> >>
>
Zhiguo Niu <niuzhiguo84@gmail.com> 于2025年6月11日周三 14:52写道：
>
> Chao Yu <chao@kernel.org> 于2025年6月11日周三 14:47写道：
> >
> > On 6/11/25 14:41, Zhiguo Niu wrote:
> > > Chao Yu <chao@kernel.org> 于2025年6月11日周三 14:07写道：
> > >>
> > >> On 6/11/25 00:08, Jaegeuk Kim wrote:
> > >>> Hi Zhiguo,
> > >>>
> > >>> This patch causes CPU hang when running fsstress on compressed/non-compressed
> > >>> files. Please check.
> > >>
> > >> Oh, seems it may cause below deadlock:
> > >>
> > >> CPU0
> > >> process A
> > >> - spin_lock(i_lock)
> > >> software IRQ
> > >> - end_io
> > >> - igrab
> > >> - spin_lock(i_lock)
> > >>
> > >> Thanks,
> > > Hi Chao,
> > > Thanks for pointing this out.
> > > I have tested this patch locally about some basic cases before submission.
> > > So it seems that should use the following method to solve this problem?
> > > " store i_compress_algorithm/sbi in dic to avoid inode access?"
> >
> > Zhiguo,
> >
> > Yeah, I guess so.
> Hi Chao,
> OK, I will prepare it .
> Thanks a lot.
> >
> > Thanks,
> >
> > > thanks！
> > >
> > >
> > >>
> > >>>
> > >>> On 06/05, 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 use igrab before f2fs_free_dic and iput after free the dic when dic free
> > >>>> action is done by kworker.
> > >>>>
> > >>>> 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>
> > >>>> ---
> > >>>> v3: use igrab to replace __iget
> > >>>> v2: use __iget/iput function
> > >>>> ---
> > >>>> fs/f2fs/compress.c | 14 +++++++++-----
> > >>>> 1 file changed, 9 insertions(+), 5 deletions(-)
> > >>>>
> > >>>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > >>>> index b3c1df9..729ad16 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,8 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic,
> > >>>> }
> > >>>>
> > >>>> page_array_free(dic->inode, dic->rpages, dic->nr_rpages);
> > >>>> + if (late_free)
> > >>>> + iput(dic->inode);
> > >>>> kmem_cache_free(dic_entry_slab, dic);
> > >>>> }
> > >>>>
> > >>>> @@ -1783,16 +1785,18 @@ 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);
> > >>>> + /* use igrab to avoid inode is evicted simultaneously */
> > >>>> + f2fs_bug_on(F2FS_I_SB(dic->inode), !igrab(dic->inode));
> > >>>> queue_work(F2FS_I_SB(dic->inode)->post_read_wq,
> > >>>> &dic->free_work);
> > >>>> }
> > >>>> --
> > >>>> 1.9.1
> > >>
> >
Hi Chao,
The patch is about as follows, because dic->sbi is used directly in
f2fs_free_dic: page_array_free(dic->sbi, dic->tpages, dic->cluster_size);
so there are two points I want to confirm:
1. As a corresponding, the first parameter (inode) of page_array_alloc
is need to modify to sbi or not ?
2. As a corresponding, do we need to add the sbi field in compress_ctx
so that page_array_alloc/free called
in compress flow can use sbi directly?
Thanks!
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index b3c1df9..897d8ae 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -34,9 +34,8 @@ static void *page_array_alloc(struct inode *inode, int nr)
return f2fs_kzalloc(sbi, size, GFP_NOFS);
}
-static void page_array_free(struct inode *inode, void *pages, int nr)
+static void page_array_free(struct f2fs_sb_info *sbi, void *pages, int nr)
{
- struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
unsigned int size = sizeof(struct page *) * nr;
if (!pages)
@@ -155,7 +154,7 @@ int f2fs_init_compress_ctx(struct compress_ctx *cc)
void f2fs_destroy_compress_ctx(struct compress_ctx *cc, bool reuse)
{
- page_array_free(cc->inode, cc->rpages, cc->cluster_size);
+ page_array_free(F2FS_I_SB(cc->inode), cc->rpages, cc->cluster_size);
cc->rpages = NULL;
cc->nr_rpages = 0;
cc->nr_cpages = 0;
@@ -716,7 +715,7 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
if (cc->cpages[i])
f2fs_compress_free_page(cc->cpages[i]);
}
- page_array_free(cc->inode, cc->cpages, cc->nr_cpages);
+ page_array_free(F2FS_I_SB(cc->inode), cc->cpages, cc->nr_cpages);
cc->cpages = NULL;
destroy_compress_ctx:
if (cops->destroy_compress_ctx)
@@ -734,7 +733,7 @@ static void f2fs_release_decomp_mem(struct
decompress_io_ctx *dic,
void f2fs_decompress_cluster(struct decompress_io_ctx *dic, bool in_task)
{
- struct f2fs_sb_info *sbi = F2FS_I_SB(dic->inode);
+ struct f2fs_sb_info *sbi = dic->sbi;
struct f2fs_inode_info *fi = F2FS_I(dic->inode);
const struct f2fs_compress_ops *cops =
f2fs_cops[fi->i_compress_algorithm];
@@ -1442,13 +1441,13 @@ static int f2fs_write_compressed_pages(struct
compress_ctx *cc,
spin_unlock(&fi->i_size_lock);
f2fs_put_rpages(cc);
- page_array_free(cc->inode, cc->cpages, cc->nr_cpages);
+ page_array_free(F2FS_I_SB(cc->inode), cc->cpages, cc->nr_cpages);
cc->cpages = NULL;
f2fs_destroy_compress_ctx(cc, false);
return 0;
out_destroy_crypt:
- page_array_free(cc->inode, cic->rpages, cc->cluster_size);
+ page_array_free(F2FS_I_SB(cc->inode), cic->rpages, cc->cluster_size);
for (--i; i >= 0; i--) {
if (!cc->cpages[i])
@@ -1469,7 +1468,7 @@ static int f2fs_write_compressed_pages(struct
compress_ctx *cc,
f2fs_compress_free_page(cc->cpages[i]);
cc->cpages[i] = NULL;
}
- page_array_free(cc->inode, cc->cpages, cc->nr_cpages);
+ page_array_free(F2FS_I_SB(cc->inode), cc->cpages, cc->nr_cpages);
cc->cpages = NULL;
return -EAGAIN;
}
@@ -1499,7 +1498,7 @@ void f2fs_compress_write_end_io(struct bio *bio,
struct page *page)
end_page_writeback(cic->rpages[i]);
}
- page_array_free(cic->inode, cic->rpages, cic->nr_rpages);
+ page_array_free(F2FS_I_SB(cic->inode), cic->rpages, cic->nr_rpages);
kmem_cache_free(cic_entry_slab, cic);
}
@@ -1637,7 +1636,7 @@ static int f2fs_prepare_decomp_mem(struct
decompress_io_ctx *dic,
f2fs_cops[F2FS_I(dic->inode)->i_compress_algorithm];
int i;
- if (!allow_memalloc_for_decomp(F2FS_I_SB(dic->inode), pre_alloc))
+ if (!allow_memalloc_for_decomp(dic->sbi, pre_alloc))
return 0;
dic->tpages = page_array_alloc(dic->inode, dic->cluster_size);
@@ -1670,10 +1669,9 @@ static int f2fs_prepare_decomp_mem(struct
decompress_io_ctx *dic,
static void f2fs_release_decomp_mem(struct decompress_io_ctx *dic,
bool bypass_destroy_callback, bool pre_alloc)
{
- const struct f2fs_compress_ops *cops =
- f2fs_cops[F2FS_I(dic->inode)->i_compress_algorithm];
+ const struct f2fs_compress_ops *cops =
f2fs_cops[dic->compress_algorithm];
- if (!allow_memalloc_for_decomp(F2FS_I_SB(dic->inode), pre_alloc))
+ if (!allow_memalloc_for_decomp(dic->sbi, pre_alloc))
return;
if (!bypass_destroy_callback && cops->destroy_decompress_ctx)
@@ -1708,6 +1706,8 @@ struct decompress_io_ctx *f2fs_alloc_dic(struct
compress_ctx *cc)
dic->magic = F2FS_COMPRESSED_PAGE_MAGIC;
dic->inode = cc->inode;
+ dic->sbi = sbi;
+ dic->compress_algorithm = F2FS_I(cc->inode)->i_compress_algorithm;
atomic_set(&dic->remaining_pages, cc->nr_cpages);
dic->cluster_idx = cc->cluster_idx;
dic->cluster_size = cc->cluster_size;
@@ -1762,7 +1762,7 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic,
continue;
f2fs_compress_free_page(dic->tpages[i]);
}
- page_array_free(dic->inode, dic->tpages, dic->cluster_size);
+ page_array_free(dic->sbi, dic->tpages, dic->cluster_size);
}
if (dic->cpages) {
@@ -1771,10 +1771,10 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic,
continue;
f2fs_compress_free_page(dic->cpages[i]);
}
- page_array_free(dic->inode, dic->cpages, dic->nr_cpages);
+ page_array_free(dic->sbi, dic->cpages, dic->nr_cpages);
}
- page_array_free(dic->inode, dic->rpages, dic->nr_rpages);
+ page_array_free(dic->sbi, dic->rpages, dic->nr_rpages);
kmem_cache_free(dic_entry_slab, dic);
}
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 9333a22b..da2137e 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1536,6 +1536,7 @@ struct compress_io_ctx {
struct decompress_io_ctx {
u32 magic; /* magic number to indicate
page is compressed */
struct inode *inode; /* inode the context belong to */
+ struct f2fs_sb_info *sbi; /* f2fs_sb_info pointer */
pgoff_t cluster_idx; /* cluster index number */
unsigned int cluster_size; /* page count in cluster */
unsigned int log_cluster_size; /* log of cluster size */
@@ -1576,6 +1577,7 @@ struct decompress_io_ctx {
bool failed; /* IO error occurred before
decompression? */
bool need_verity; /* need fs-verity verification
after decompression? */
+ unsigned char compress_algorithm; /* backup algorithm type */
void *private; /* payload buffer for
specified decompression algorithm */
void *private2; /* extra payload buffer */
struct work_struct verity_work; /* work to verify the
decompressed pages */
On 6/11/25 15:51, Zhiguo Niu wrote:
> Zhiguo Niu <niuzhiguo84@gmail.com> 于2025年6月11日周三 14:52写道：
>>
>> Chao Yu <chao@kernel.org> 于2025年6月11日周三 14:47写道：
>>>
>>> On 6/11/25 14:41, Zhiguo Niu wrote:
>>>> Chao Yu <chao@kernel.org> 于2025年6月11日周三 14:07写道：
>>>>>
>>>>> On 6/11/25 00:08, Jaegeuk Kim wrote:
>>>>>> Hi Zhiguo,
>>>>>>
>>>>>> This patch causes CPU hang when running fsstress on compressed/non-compressed
>>>>>> files. Please check.
>>>>>
>>>>> Oh, seems it may cause below deadlock:
>>>>>
>>>>> CPU0
>>>>> process A
>>>>> - spin_lock(i_lock)
>>>>> software IRQ
>>>>> - end_io
>>>>> - igrab
>>>>> - spin_lock(i_lock)
>>>>>
>>>>> Thanks,
>>>> Hi Chao,
>>>> Thanks for pointing this out.
>>>> I have tested this patch locally about some basic cases before submission.
>>>> So it seems that should use the following method to solve this problem?
>>>> " store i_compress_algorithm/sbi in dic to avoid inode access?"
>>>
>>> Zhiguo,
>>>
>>> Yeah, I guess so.
>> Hi Chao,
>> OK, I will prepare it .
>> Thanks a lot.
>>>
>>> Thanks,
>>>
>>>> thanks！
>>>>
>>>>
>>>>>
>>>>>>
>>>>>> On 06/05, 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 use igrab before f2fs_free_dic and iput after free the dic when dic free
>>>>>>> action is done by kworker.
>>>>>>>
>>>>>>> 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>
>>>>>>> ---
>>>>>>> v3: use igrab to replace __iget
>>>>>>> v2: use __iget/iput function
>>>>>>> ---
>>>>>>> fs/f2fs/compress.c | 14 +++++++++-----
>>>>>>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>>>>>>
>>>>>>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>>>>>>> index b3c1df9..729ad16 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,8 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic,
>>>>>>> }
>>>>>>>
>>>>>>> page_array_free(dic->inode, dic->rpages, dic->nr_rpages);
>>>>>>> + if (late_free)
>>>>>>> + iput(dic->inode);
>>>>>>> kmem_cache_free(dic_entry_slab, dic);
>>>>>>> }
>>>>>>>
>>>>>>> @@ -1783,16 +1785,18 @@ 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);
>>>>>>> + /* use igrab to avoid inode is evicted simultaneously */
>>>>>>> + f2fs_bug_on(F2FS_I_SB(dic->inode), !igrab(dic->inode));
>>>>>>> queue_work(F2FS_I_SB(dic->inode)->post_read_wq,
>>>>>>> &dic->free_work);
>>>>>>> }
>>>>>>> --
>>>>>>> 1.9.1
>>>>>
>>>
>
> Hi Chao,
>
> The patch is about as follows, because dic->sbi is used directly in
> f2fs_free_dic: page_array_free(dic->sbi, dic->tpages, dic->cluster_size);
> so there are two points I want to confirm:
> 1. As a corresponding, the first parameter (inode) of page_array_alloc
> is need to modify to sbi or not ?
> 2. As a corresponding, do we need to add the sbi field in compress_ctx
> so that page_array_alloc/free called
> in compress flow can use sbi directly?
Zhiguo, both 1) and 2) changes look fine to me.
Thanks,
> Thanks!
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index b3c1df9..897d8ae 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -34,9 +34,8 @@ static void *page_array_alloc(struct inode *inode, int nr)
> return f2fs_kzalloc(sbi, size, GFP_NOFS);
> }
>
> -static void page_array_free(struct inode *inode, void *pages, int nr)
> +static void page_array_free(struct f2fs_sb_info *sbi, void *pages, int nr)
> {
> - struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> unsigned int size = sizeof(struct page *) * nr;
>
> if (!pages)
> @@ -155,7 +154,7 @@ int f2fs_init_compress_ctx(struct compress_ctx *cc)
>
> void f2fs_destroy_compress_ctx(struct compress_ctx *cc, bool reuse)
> {
> - page_array_free(cc->inode, cc->rpages, cc->cluster_size);
> + page_array_free(F2FS_I_SB(cc->inode), cc->rpages, cc->cluster_size);
> cc->rpages = NULL;
> cc->nr_rpages = 0;
> cc->nr_cpages = 0;
> @@ -716,7 +715,7 @@ static int f2fs_compress_pages(struct compress_ctx *cc)
> if (cc->cpages[i])
> f2fs_compress_free_page(cc->cpages[i]);
> }
> - page_array_free(cc->inode, cc->cpages, cc->nr_cpages);
> + page_array_free(F2FS_I_SB(cc->inode), cc->cpages, cc->nr_cpages);
> cc->cpages = NULL;
> destroy_compress_ctx:
> if (cops->destroy_compress_ctx)
> @@ -734,7 +733,7 @@ static void f2fs_release_decomp_mem(struct
> decompress_io_ctx *dic,
>
> void f2fs_decompress_cluster(struct decompress_io_ctx *dic, bool in_task)
> {
> - struct f2fs_sb_info *sbi = F2FS_I_SB(dic->inode);
> + struct f2fs_sb_info *sbi = dic->sbi;
> struct f2fs_inode_info *fi = F2FS_I(dic->inode);
> const struct f2fs_compress_ops *cops =
> f2fs_cops[fi->i_compress_algorithm];
> @@ -1442,13 +1441,13 @@ static int f2fs_write_compressed_pages(struct
> compress_ctx *cc,
> spin_unlock(&fi->i_size_lock);
>
> f2fs_put_rpages(cc);
> - page_array_free(cc->inode, cc->cpages, cc->nr_cpages);
> + page_array_free(F2FS_I_SB(cc->inode), cc->cpages, cc->nr_cpages);
> cc->cpages = NULL;
> f2fs_destroy_compress_ctx(cc, false);
> return 0;
>
> out_destroy_crypt:
> - page_array_free(cc->inode, cic->rpages, cc->cluster_size);
> + page_array_free(F2FS_I_SB(cc->inode), cic->rpages, cc->cluster_size);
>
> for (--i; i >= 0; i--) {
> if (!cc->cpages[i])
> @@ -1469,7 +1468,7 @@ static int f2fs_write_compressed_pages(struct
> compress_ctx *cc,
> f2fs_compress_free_page(cc->cpages[i]);
> cc->cpages[i] = NULL;
> }
> - page_array_free(cc->inode, cc->cpages, cc->nr_cpages);
> + page_array_free(F2FS_I_SB(cc->inode), cc->cpages, cc->nr_cpages);
> cc->cpages = NULL;
> return -EAGAIN;
> }
> @@ -1499,7 +1498,7 @@ void f2fs_compress_write_end_io(struct bio *bio,
> struct page *page)
> end_page_writeback(cic->rpages[i]);
> }
>
> - page_array_free(cic->inode, cic->rpages, cic->nr_rpages);
> + page_array_free(F2FS_I_SB(cic->inode), cic->rpages, cic->nr_rpages);
> kmem_cache_free(cic_entry_slab, cic);
> }
>
> @@ -1637,7 +1636,7 @@ static int f2fs_prepare_decomp_mem(struct
> decompress_io_ctx *dic,
> f2fs_cops[F2FS_I(dic->inode)->i_compress_algorithm];
> int i;
>
> - if (!allow_memalloc_for_decomp(F2FS_I_SB(dic->inode), pre_alloc))
> + if (!allow_memalloc_for_decomp(dic->sbi, pre_alloc))
> return 0;
>
> dic->tpages = page_array_alloc(dic->inode, dic->cluster_size);
> @@ -1670,10 +1669,9 @@ static int f2fs_prepare_decomp_mem(struct
> decompress_io_ctx *dic,
> static void f2fs_release_decomp_mem(struct decompress_io_ctx *dic,
> bool bypass_destroy_callback, bool pre_alloc)
> {
> - const struct f2fs_compress_ops *cops =
> - f2fs_cops[F2FS_I(dic->inode)->i_compress_algorithm];
> + const struct f2fs_compress_ops *cops =
> f2fs_cops[dic->compress_algorithm];
>
> - if (!allow_memalloc_for_decomp(F2FS_I_SB(dic->inode), pre_alloc))
> + if (!allow_memalloc_for_decomp(dic->sbi, pre_alloc))
> return;
>
> if (!bypass_destroy_callback && cops->destroy_decompress_ctx)
> @@ -1708,6 +1706,8 @@ struct decompress_io_ctx *f2fs_alloc_dic(struct
> compress_ctx *cc)
>
> dic->magic = F2FS_COMPRESSED_PAGE_MAGIC;
> dic->inode = cc->inode;
> + dic->sbi = sbi;
> + dic->compress_algorithm = F2FS_I(cc->inode)->i_compress_algorithm;
> atomic_set(&dic->remaining_pages, cc->nr_cpages);
> dic->cluster_idx = cc->cluster_idx;
> dic->cluster_size = cc->cluster_size;
> @@ -1762,7 +1762,7 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic,
> continue;
> f2fs_compress_free_page(dic->tpages[i]);
> }
> - page_array_free(dic->inode, dic->tpages, dic->cluster_size);
> + page_array_free(dic->sbi, dic->tpages, dic->cluster_size);
> }
>
> if (dic->cpages) {
> @@ -1771,10 +1771,10 @@ static void f2fs_free_dic(struct decompress_io_ctx *dic,
> continue;
> f2fs_compress_free_page(dic->cpages[i]);
> }
> - page_array_free(dic->inode, dic->cpages, dic->nr_cpages);
> + page_array_free(dic->sbi, dic->cpages, dic->nr_cpages);
> }
>
> - page_array_free(dic->inode, dic->rpages, dic->nr_rpages);
> + page_array_free(dic->sbi, dic->rpages, dic->nr_rpages);
> kmem_cache_free(dic_entry_slab, dic);
> }
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 9333a22b..da2137e 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1536,6 +1536,7 @@ struct compress_io_ctx {
> struct decompress_io_ctx {
> u32 magic; /* magic number to indicate
> page is compressed */
> struct inode *inode; /* inode the context belong to */
> + struct f2fs_sb_info *sbi; /* f2fs_sb_info pointer */
> pgoff_t cluster_idx; /* cluster index number */
> unsigned int cluster_size; /* page count in cluster */
> unsigned int log_cluster_size; /* log of cluster size */
> @@ -1576,6 +1577,7 @@ struct decompress_io_ctx {
>
> bool failed; /* IO error occurred before
> decompression? */
> bool need_verity; /* need fs-verity verification
> after decompression? */
> + unsigned char compress_algorithm; /* backup algorithm type */
> void *private; /* payload buffer for
> specified decompression algorithm */
> void *private2; /* extra payload buffer */
> struct work_struct verity_work; /* work to verify the
> decompressed pages */
On 6/5/25 15:18, 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 use igrab before f2fs_free_dic and iput after free the dic when dic free
> action is done by kworker.
>
> 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>
Reviewed-by: Chao Yu <chao@kernel.org>
Thanks,
© 2016 - 2025 Red Hat, Inc.