If z_erofs_gbuf_growsize() partially fails on a global buffer due to
memory allocation failure or fault injection (as reported by syzbot [1]),
new pages need to be freed by comparing to the existing pages to avoid
memory leaks.

However, the old gbuf->pages[] array may not be large enough, which can
lead to null-ptr-deref or out-of-bound access.

Fix this by checking against gbuf->nrpages in advance.

Fixes: d6db47e571dc ("erofs: do not use pagepool in z_erofs_gbuf_growsize()")
Cc: <stable@vger.kernel.org> # 6.10+
Cc: Chunhai Guo <guochunhai@vivo.com>
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/zutil.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/erofs/zutil.c b/fs/erofs/zutil.c
index 9b53883e5caf..37afe2024840 100644
--- a/fs/erofs/zutil.c
+++ b/fs/erofs/zutil.c
@@ -111,7 +111,8 @@ int z_erofs_gbuf_growsize(unsigned int nrpages)
 out:
 	if (i < z_erofs_gbuf_count && tmp_pages) {
 		for (j = 0; j < nrpages; ++j)
-			if (tmp_pages[j] && tmp_pages[j] != gbuf->pages[j])
+			if (tmp_pages[j] && (j >= gbuf->nrpages ||
+					     tmp_pages[j] != gbuf->pages[j]))
 				__free_page(tmp_pages[j]);
 		kfree(tmp_pages);
 	}
-- 
2.43.5

在 2024/8/20 16:42, Gao Xiang 写道:
> If z_erofs_gbuf_growsize() partially fails on a global buffer due to
> memory allocation failure or fault injection (as reported by syzbot [1]),
> new pages need to be freed by comparing to the existing pages to avoid
> memory leaks.
>
> However, the old gbuf->pages[] array may not be large enough, which can
> lead to null-ptr-deref or out-of-bound access.
>
> Fix this by checking against gbuf->nrpages in advance.
>
> Fixes: d6db47e571dc ("erofs: do not use pagepool in z_erofs_gbuf_growsize()")
> Cc: <stable@vger.kernel.org> # 6.10+
> Cc: Chunhai Guo <guochunhai@vivo.com>
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
>
Reviewed-by: Chunhai Guo <guochunhai@vivo.com>

Thanks,

Chunhai Guo

Hi Chunhai,

On 2024/8/20 17:25, Chunhai Guo wrote:
> 在 2024/8/20 16:42, Gao Xiang 写道:
>> If z_erofs_gbuf_growsize() partially fails on a global buffer due to
>> memory allocation failure or fault injection (as reported by syzbot [1]),
>> new pages need to be freed by comparing to the existing pages to avoid
>> memory leaks.
>>
>> However, the old gbuf->pages[] array may not be large enough, which can
>> lead to null-ptr-deref or out-of-bound access.
>>
>> Fix this by checking against gbuf->nrpages in advance.
>>
>> Fixes: d6db47e571dc ("erofs: do not use pagepool in z_erofs_gbuf_growsize()")
>> Cc: <stable@vger.kernel.org> # 6.10+
>> Cc: Chunhai Guo <guochunhai@vivo.com>
>> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
>>
> Reviewed-by: Chunhai Guo <guochunhai@vivo.com>

I've sent a patch to add links and reported-by.

I assume I can add your reviewed-by to that version too?

Thanks,
Gao Xiang

> 
> Thanks,
> 
> Chunhai Guo
> 

在 2024/8/20 17:27, Gao Xiang 写道:
> Hi Chunhai,
>
> On 2024/8/20 17:25, Chunhai Guo wrote:
>> 在 2024/8/20 16:42, Gao Xiang 写道:
>>> If z_erofs_gbuf_growsize() partially fails on a global buffer due to
>>> memory allocation failure or fault injection (as reported by syzbot 
>>> [1]),
>>> new pages need to be freed by comparing to the existing pages to avoid
>>> memory leaks.
>>>
>>> However, the old gbuf->pages[] array may not be large enough, which can
>>> lead to null-ptr-deref or out-of-bound access.
>>>
>>> Fix this by checking against gbuf->nrpages in advance.
>>>
>>> Fixes: d6db47e571dc ("erofs: do not use pagepool in 
>>> z_erofs_gbuf_growsize()")
>>> Cc: <stable@vger.kernel.org> # 6.10+
>>> Cc: Chunhai Guo <guochunhai@vivo.com>
>>> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
>>>
>> Reviewed-by: Chunhai Guo <guochunhai@vivo.com>
>
> I've sent a patch to add links and reported-by.
>
> I assume I can add your reviewed-by to that version too?

Yes. That is right.

Thanks,

>
> Thanks,
> Gao Xiang
>
>>
>> Thanks,
>>
>> Chunhai Guo
>>