Re: [f2fs-dev] [PATCH] f2fs: fix page leak in redirty_blocks

[Chao Yu <chao@kernel.org>]

On 2022/5/30 9:02, Jack Qiu wrote:
> On 2022/5/29 14:58, Chao Yu wrote:
>> On 2022/5/28 17:35, Jack Qiu via Linux-f2fs-devel wrote:
>>> When find_lock_page return error, page in [i, page_len) will leak.
>>
>> I doubt it is impossible to fail in find_lock_page due to one extra
>> reference count was added in previous read_cache_page().
> 
> Thanks for review.
> I'm not sure about it with limited knowledge.

I guess something like those pages were pinned by adding extra
reference, and find_lock_page() should never miss them...

 > If it is true, maybe use f2fs_bug_on(sbi, !page) is better?

I think so.

Thanks,

> 
>>
>> Thanks,
>>
>>>
>>> Signed-off-by: Jack Qiu <jack.qiu@huawei.com>
>>> ---
>>>    fs/f2fs/file.c | 13 +++++++++++++
>>>    1 file changed, 13 insertions(+)
>>>
>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>> index 100637b1adb3..0e8938c5918e 100644
>>> --- a/fs/f2fs/file.c
>>> +++ b/fs/f2fs/file.c
>>> @@ -3954,6 +3954,12 @@ static int redirty_blocks(struct inode *inode, pgoff_t page_idx, int len)
>>>        struct page *page;
>>>        pgoff_t redirty_idx = page_idx;
>>>        int i, page_len = 0, ret = 0;
>>> +    struct page **pages;
>>> +
>>> +    pages = f2fs_kvzalloc(F2FS_I_SB(inode),
>>> +                sizeof(struct page *) * len, GFP_NOFS);
>>> +    if (!pages)
>>> +        return -ENOMEM;
>>>
>>>        page_cache_ra_unbounded(&ractl, len, 0);
>>>
>>> @@ -3964,6 +3970,7 @@ static int redirty_blocks(struct inode *inode, pgoff_t page_idx, int len)
>>>                break;
>>>            }
>>>            page_len++;
>>> +        pages[i] = page;
>>>        }
>>>
>>>        for (i = 0; i < page_len; i++, redirty_idx++) {
>>> @@ -3975,8 +3982,14 @@ static int redirty_blocks(struct inode *inode, pgoff_t page_idx, int len)
>>>            set_page_dirty(page);
>>>            f2fs_put_page(page, 1);
>>>            f2fs_put_page(page, 0);
>>> +        pages[i] = NULL;
>>>        }
>>>
>>> +    /* put pages[i, page_len) when error happens */
>>> +    for (; ret < 0 && i < page_len; i++)
>>> +        f2fs_put_page(pages[i], 0);
>>> +    kvfree(pages);
>>> +
>>>        return ret;
>>>    }
>>>
>>> -- 
>>> 2.31.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