On Fri, 21 Jul 2023 at 13:12, Simon Horman [off-list ref] wrote:

On Tue, Jul 18, 2023 at 02:58:46PM +0200, Ard Biesheuvel wrote:

...

quoted

-static int deflate_comp_init(struct deflate_ctx *ctx)
+static int deflate_process(struct acomp_req *req, struct z_stream_s *stream,
+                        int (*process)(struct z_stream_s *, int))
 {
-     int ret = 0;
-     struct z_stream_s *stream = &ctx->comp_stream;
+     unsigned int slen = req->slen;
+     unsigned int dlen = req->dlen;
+     struct scatter_walk src, dst;
+     unsigned int scur, dcur;
+     int ret;

-     stream->workspace = vzalloc(zlib_deflate_workspacesize(
-                             -DEFLATE_DEF_WINBITS, DEFLATE_DEF_MEMLEVEL));
-     if (!stream->workspace) {
-             ret = -ENOMEM;
-             goto out;
-     }
+     stream->avail_in = stream->avail_out = 0;
+
+     scatterwalk_start(&src, req->src);
+     scatterwalk_start(&dst, req->dst);
+
+     scur = dcur = 0;
+
+     do {
+             if (stream->avail_in == 0) {
+                     if (scur) {
+                             slen -= scur;
+
+                             scatterwalk_unmap(stream->next_in - scur);
+                             scatterwalk_advance(&src, scur);
+                             scatterwalk_done(&src, 0, slen);
+                     }
+
+                     scur = scatterwalk_clamp(&src, slen);
+                     if (scur) {
+                             stream->next_in = scatterwalk_map(&src);
+                             stream->avail_in = scur;
+                     }
+             }
+
+             if (stream->avail_out == 0) {
+                     if (dcur) {
+                             dlen -= dcur;
+
+                             scatterwalk_unmap(stream->next_out - dcur);
+                             scatterwalk_advance(&dst, dcur);
+                             scatterwalk_done(&dst, 1, dlen);
+                     }
+
+                     dcur = scatterwalk_clamp(&dst, dlen);
+                     if (!dcur)
+                             break;

Hi Ard,

I'm unsure if this can happen. But if this break occurs in the first
iteration of this do loop, then ret will be used uninitialised below.

Smatch noticed this.

Thanks.

This should not happen - it would mean req->dlen == 0, which is
rejected before this function is even called.

Whether or not it might ever happen in practice is a different matter,
of course, so I should probably initialize 'ret' to something sane.

quoted

+
+                     stream->next_out = scatterwalk_map(&dst);
+                     stream->avail_out = dcur;
+             }
+
+             ret = process(stream, (slen == scur) ? Z_FINISH : Z_SYNC_FLUSH);
+     } while (ret == Z_OK);
+
+     if (scur)
+             scatterwalk_unmap(stream->next_in - scur);
+     if (dcur)
+             scatterwalk_unmap(stream->next_out - dcur);
+
+     if (ret != Z_STREAM_END)
+             return -EINVAL;
+
+     req->dlen = stream->total_out;
+     return 0;
+}

...