Re: [PATCH] crypto: zstd - fix segmented acomp streaming paths

[Giovanni Cabiddu <giovanni.cabiddu@intel.com>]

On Mon, Mar 09, 2026 at 02:20:51AM -0600, Wesley Atwell wrote:
> The zstd acomp implementation does not correctly handle segmented
> source and destination walks.
> 
> The compression path advances the destination walk by the full
> segment length rather than the bytes actually produced, and it only
> calls zstd_end_stream() once even though the streaming API requires it
> to be called until it returns 0.  With segmented destinations this can
> leave buffered output behind and misaccount the walk progress.
> 
> The decompression path has the same destination accounting issue, and
> it stops when the source walk is exhausted even if
> zstd_decompress_stream() has not yet reported that the frame is fully
> decoded and flushed.  That can report success too early for segmented
> requests and incomplete frames.
> 
> Fix both streaming paths by advancing destination segments by actual
> output bytes, refilling destination segments as needed, draining
> zstd_end_stream() until completion, and continuing to flush buffered
> decompression output after the source walk is exhausted.  Return
> -EINVAL if decompression cannot finish once the input has been fully
> consumed.
> 
> Fixes: f5ad93ffb541 ("crypto: zstd - convert to acomp")
> Assisted-by: Codex:GPT-5
> Signed-off-by: Wesley Atwell <atwellwea@gmail.com>
> ---
> Local validation:
> - built bzImage with CONFIG_CRYPTO_SELFTESTS=y and CONFIG_CRYPTO_SELFTESTS_FULL=y
> - exercised segmented zstd acomp requests using temporary local testmgr scaffolding
> - booted under virtme and verified zstd-generic selftest passed in /proc/crypto
> 
>  crypto/zstd.c | 228 ++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 156 insertions(+), 72 deletions(-)
> 
> diff --git a/crypto/zstd.c b/crypto/zstd.c
> index 556f5d2bdd5f..3e19da1fed22 100644
> --- a/crypto/zstd.c
> +++ b/crypto/zstd.c
> @@ -94,18 +94,30 @@ static int zstd_compress_one(struct acomp_req *req, struct zstd_ctx *ctx,
>  	return 0;
>  }
>  
> +static int zstd_acomp_next_dst(struct acomp_walk *walk, zstd_out_buffer *outbuf)
> +{
> +	unsigned int dcur = acomp_walk_next_dst(walk);
> +
> +	if (!dcur)
> +		return -ENOSPC;
> +
> +	outbuf->pos = 0;
> +	outbuf->dst = walk->dst.virt.addr;
> +	outbuf->size = dcur;
> +
> +	return 0;
> +}
> +
>  static int zstd_compress(struct acomp_req *req)
>  {
>  	struct crypto_acomp_stream *s;
> -	unsigned int pos, scur, dcur;
> +	unsigned int scur;
>  	unsigned int total_out = 0;
> -	bool data_available = true;
>  	zstd_out_buffer outbuf;
>  	struct acomp_walk walk;
>  	zstd_in_buffer inbuf;
>  	struct zstd_ctx *ctx;
> -	size_t pending_bytes;
> -	size_t num_bytes;
> +	size_t remaining;
>  	int ret;
>  
>  	s = crypto_acomp_lock_stream_bh(&zstd_streams);
> @@ -115,66 +127,87 @@ static int zstd_compress(struct acomp_req *req)
>  	if (ret)
>  		goto out;
>  
> +	ret = zstd_acomp_next_dst(&walk, &outbuf);
> +	if (ret)
> +		goto out;
> +
>  	ctx->cctx = zstd_init_cstream(&ctx->params, 0, ctx->wksp, ctx->wksp_size);
>  	if (!ctx->cctx) {
>  		ret = -EINVAL;
>  		goto out;
>  	}
>  
> -	do {
> -		dcur = acomp_walk_next_dst(&walk);
> -		if (!dcur) {
> -			ret = -ENOSPC;
> +	for (;;) {
> +		scur = acomp_walk_next_src(&walk);
> +		if (outbuf.size == req->dlen && scur == req->slen) {
> +			ret = zstd_compress_one(req, ctx, walk.src.virt.addr,
> +						walk.dst.virt.addr, &total_out);
> +			if (!ret) {
> +				acomp_walk_done_src(&walk, scur);
> +				acomp_walk_done_dst(&walk, total_out);
These two functions should be called regardless of the return code of
zstd_compress_one() as they are unmapping the buffers.

...

> @@ -209,12 +242,12 @@ static int zstd_decompress(struct acomp_req *req)
>  {
>  	struct crypto_acomp_stream *s;
>  	unsigned int total_out = 0;
> -	unsigned int scur, dcur;
> +	unsigned int scur;
>  	zstd_out_buffer outbuf;
>  	struct acomp_walk walk;
>  	zstd_in_buffer inbuf;
>  	struct zstd_ctx *ctx;
> -	size_t pending_bytes;
> +	size_t remaining = 1;
>  	int ret;
>  
>  	s = crypto_acomp_lock_stream_bh(&zstd_streams);
> @@ -224,54 +257,105 @@ static int zstd_decompress(struct acomp_req *req)
>  	if (ret)
>  		goto out;
>  
> +	ret = zstd_acomp_next_dst(&walk, &outbuf);
> +	if (ret)
> +		goto out;
> +
>  	ctx->dctx = zstd_init_dstream(ZSTD_MAX_SIZE, ctx->wksp, ctx->wksp_size);
>  	if (!ctx->dctx) {
>  		ret = -EINVAL;
>  		goto out;
>  	}
>  
> -	do {
> +	for (;;) {
>  		scur = acomp_walk_next_src(&walk);
> -		if (scur) {
> -			inbuf.pos = 0;
> -			inbuf.size = scur;
> -			inbuf.src = walk.src.virt.addr;
> -		} else {
> -			break;
> +		if (outbuf.size == req->dlen && scur == req->slen) {
> +			ret = zstd_decompress_one(req, ctx, walk.src.virt.addr,
> +						  walk.dst.virt.addr, &total_out);
> +			if (!ret) {
> +				acomp_walk_done_src(&walk, scur);
> +				acomp_walk_done_dst(&walk, total_out);
> +			}
Same here.

Regards,

-- 
Giovanni