Re: [PATCH v5 02/12] crypto: acomp - Define new interfaces for compress/decompress batching.

[Sergey Senozhatsky <senozhatsky@chromium.org>]

On (25/02/23 10:52), Herbert Xu wrote:
> On Sun, Feb 23, 2025 at 11:09:32AM +0900, Sergey Senozhatsky wrote:
> >
> > Right, for lzo/lzo-rle we need a safety page.
> 
> We should fix it because it's a security hole for anyone who calls
> it through the Crypto API.

Yeah, I don't disagree.

> > It also seems that there is no common way of reporting dst_but overflow.
> > Some algos return -ENOSPC immediately, some don't return anything at all,
> > and deflate does it's own thing - there are these places where they see
> > they are out of out space but they Z_OK it
> > 
> > if (s->pending != 0) {
> > 	flush_pending(strm);
> > 	if (strm->avail_out == 0) {
> > 		/* Since avail_out is 0, deflate will be called again with
> > 		 * more output space, but possibly with both pending and
> > 		 * avail_in equal to zero. There won't be anything to do,
> > 		 * but this is not an error situation so make sure we
> > 		 * return OK instead of BUF_ERROR at next call of deflate:
> > 		 */
> > 		s->last_flush = -1;
> > 		return Z_OK;
> > 	}
> > }
> 
> Z_OK is actually an error, see crypto/deflate.c:

I saw Z_STREAM_END, but deflate states "this is not an error" and
there are more places like this.

> 	ret = zlib_deflate(stream, Z_FINISH);
> 	if (ret != Z_STREAM_END) {
> 		ret = -EINVAL;
> 		goto out;
> 	}
> 
> We could change this to ENOSPC for consistency.

So it will ENOSPC all errors, not sure how good that is.  We also
have lz4/lz4hc that return the number of bytes "(((char *)op) - dest)"
if successful and 0 otherwise.  So any error is 0. dst_buf overrun
is also 0, impossible to tell the difference, again not sure if we
can just ENOSPC.