linux-kernel - Re: [PATCH v3 04/19] crypto: scatterwalk - add new functions for copying data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250302213742.GB2079@quark.localdomain>
Date: Sun, 2 Mar 2025 13:37:42 -0800
From: Eric Biggers <ebiggers@...nel.org>
To: Herbert Xu <herbert@...dor.apana.org.au>
Cc: linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, David Howells <dhowells@...hat.com>,
	Akinobu Mita <akinobu.mita@...il.com>, Tejun Heo <htejun@...il.com>
Subject: Re: [PATCH v3 04/19] crypto: scatterwalk - add new functions for
 copying data

On Sun, Mar 02, 2025 at 02:40:56PM +0800, Herbert Xu wrote:
> Eric Biggers <ebiggers@...nel.org> wrote:
> >
> > +void memcpy_from_sglist(void *buf, struct scatterlist *sg,
> > +                       unsigned int start, unsigned int nbytes)
> > {
> >        struct scatter_walk walk;
> > -       struct scatterlist tmp[2];
> > 
> > -       if (!nbytes)
> > +       if (unlikely(nbytes == 0)) /* in case sg == NULL */
> >                return;
> > 
> > -       sg = scatterwalk_ffwd(tmp, sg, start);
> > +       scatterwalk_start_at_pos(&walk, sg, start);
> > +       memcpy_from_scatterwalk(buf, &walk, nbytes);
> > +}
> > +EXPORT_SYMBOL_GPL(memcpy_from_sglist);
> > +
> > +void memcpy_to_sglist(struct scatterlist *sg, unsigned int start,
> > +                     const void *buf, unsigned int nbytes)
> 
> These functions duplicate sg_copy_buffer.  Of course scatterwalk
> in general duplicates SG miter which came later IIRC.
> 
> What's your plan for eliminating this duplication?
> 
> Thanks,

The new functions are much better than the lib/scatterlist.c ones: they have a
much better implementation that is faster and doesn't use atomic kmaps, and
(like scatterwalk_map_and_copy() which they are replacing first) they don't
require the unhelpful 'nents' parameter.  My tentative plan is to move them into
lib/scatterlist.c, reimplement sg_copy_buffer() et al on top of them, then
eventually update the callers to use the new functions directly.

However, the 'nents' parameter that sg_copy_buffer() et al take will make the
unification a bit difficult.  Currently those functions copy the minimum of
'buflen' bytes and the first 'nents' scatterlist elements.  I'd like to remove
the 'nents' parameter and just have 'buflen' (or rather 'nbytes'), like the
crypto/scatterwalk.c functions.  I suspect that nearly all callers are passing
in enough 'nents' to cover their 'buflen'.  But there may be some exceptions,
which we'll need to check for.

- Eric