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

[Fedor Pchelkin <pchelkin@xxxxxxxxx>]

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..