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

[Eric Biggers <ebiggers@kernel.org>]

On Wed, Mar 12, 2025 at 10:30:06AM +0800, Herbert Xu wrote:
> On Tue, Mar 11, 2025 at 10:44:31AM -0700, Eric Biggers wrote:
> >
> > I guess you think this is fixing a bug in the case where sg->offset > PAGE_SIZE?
> > Is that case even supported?  It is supposed to be the offset into a page.
> 
> Supported? Obviously not since this bug has been there since the
> very start :)
> 
> But is it conceivable to create such a scatterlist, it certainly
> seems to be.  If we were to create a scatterlist from a single
> order-n folio, then it's quite reasonable for the offset to be
> greater than PAGE_SIZE.

If it needs to work, then it needs to be tested.  Currently the self-tests
always use sg_set_buf(), and thus scatterlist::offset always gets set to a value
less than PAGE_SIZE.

And this is yet more evidence that scatterlist based APIs are just a really bad
idea.  Even 20 years later, there is still no precise definition of what a
scatterlist even is...

> > Even if so, a simpler fix (1 line) would be to use:
> > 'sg->length >= nbytes && sg->offset + nbytes <= PAGE_SIZE'
> 
> That would just mean more use of the fallback code path.  Not
> a big deal but I was feeling generous.

My second suggestion, making it conditional on !HIGHMEM and using nbytes <=
sg->length, would also be simple and would reduce the overhead for !HIGHMEM
(rather than increasing it as your patch does, just like patch 1 which had the
same problem).  !HIGHMEM is the common case (by far) these days, of course.

- Eric