Re: [PATCH 6.1 1/2] erofs: handle overlapped pclusters out of crafted images properly

[Gao Xiang <hsiangkao@linux.alibaba.com>]

On 2025/3/3 02:13, Fedor Pchelkin wrote:
> On Mon, 03. Mar 01:41, Gao Xiang wrote:
>>>> diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
>>>> index 94e9e0bf3bbd..ac01c0ede7f7 100644
>>>
>>> I'm looking at the diff of upstream commit and the first thing it does
>>> is to remove zeroing out the folio/page private field here:
>>>
>>>     // upstream commit 9e2f9d34dd12 ("erofs: handle overlapped pclusters out of crafted images properly")
>>>     @@ -1450,7 +1451,6 @@ static void z_erofs_fill_bio_vec(struct bio_vec *bvec,
>>>              * file-backed folios will be used instead.
>>>              */
>>>             if (folio->private == (void *)Z_EROFS_PREALLOCATED_PAGE) {
>>>     -               folio->private = 0;
>>>                     tocache = true;
>>>                     goto out_tocache;
>>>             }
>>>
>>> while in 6.1.129 the corresponding fragment seems untouched with the
>>> backport patch. Is it intended?
>>
>> Yes, because it was added in
>> commit 2080ca1ed3e4 ("erofs: tidy up `struct z_erofs_bvec`")
>> and dropped again.
>>
>> But for Linux 6.6.y and 6.1.y, we don't need to backport
>> 2080ca1ed3e4.
> 
> Thanks for overall clarification, Gao!
> 
> My concern was that in 6.1 and 6.6 there is still a pattern at that
> place, not directly related to 2080ca1ed3e4 ("erofs: tidy up
> `struct z_erofs_bvec`"):
> 
> 1. checking ->private against Z_EROFS_PREALLOCATED_PAGE
> 2. zeroing out ->private if the previous check holds true
> 
> // 6.1/6.6 fragment
> 
> 	if (page->private == Z_EROFS_PREALLOCATED_PAGE) {
> 		WRITE_ONCE(pcl->compressed_bvecs[nr].page, page);
> 		set_page_private(page, 0);
> 		tocache = true;
> 		goto out_tocache;
> 	}
> 
> while the upstream patch changed the situation. If it's okay then no
> remarks from me. Sorry for the noise..

Yeah, yet as I mentioned `set_page_private(page, 0);`
seems redundant from the codebase, I'm fine with either
way.

Thanks,
Gao Xiang