Re: [v2 PATCH 2/2] crypto: hash - Use nth_page instead of doing it by hand

[Eric Biggers <ebiggers@kernel.org>]

On Thu, Mar 13, 2025 at 10:36:44AM +0800, Herbert Xu wrote:
> On Wed, Mar 12, 2025 at 01:09:08PM -0700, Eric Biggers wrote:
> >
> > It seems that the "real bug" mentioned above is the case of
> > scatterlist::offset > PAGE_SIZE.  That's unrelated to the nth_page() fix, which
> > seems to be for scatterlist elements that span physical memory sections.  Also,
> 
> Alright I'll try to split it up.
> 
> > Note that there is also page arithmetic being done in scatterwalk_done_dst() and
> > scomp_acomp_comp_decomp().  Those presumably need the nth_page() fix too.
> 
> Thanks, I had missed the flushing code in scatterwalk.
> 
> As for scomp yes that's already fixed in my acomp series which I
> will repost soon.
> 
> > scomp_acomp_comp_decomp() also assumes that if the first page in a given
> > scatterlist element is lowmem, then any additional pages are lowmem too.  That
> 
> Yes I've fixed that by changing it to test the last page rather than
> the first, assuming that highmem indeed comes after lowmem.
> 
> > sounds like another potentially wrong assumption.  Can scatterlist elements span
> > memory zones?  Or just physical memory sections?
> 
> Theoretically it can cross anything.  Check out the block merging
> code in __blk_rq_map_sg, it tries to merge any physically contiguous
> page.

Actually the block layer avoids this edge case.  See the comment above struct
bio_vec, and the corresponding code in bvec_try_merge_page():

        if (bv->bv_page + bv_end / PAGE_SIZE != page + off / PAGE_SIZE)
                return false;

- Eric