Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221006234138.1835739-1-keescook@chromium.org>
 <191ec24d-35d4-e4e5-85f7-d7301984e647@igalia.com> <202210171100.5BAC4A5CC8@keescook>
In-Reply-To: <202210171100.5BAC4A5CC8@keescook>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Mon, 17 Oct 2022 20:14:14 +0200
Message-ID: <CAMj1kXHzrRTVcxb5+hgUPV3tjekPcDWzVf6cG_Mc9JJmYBz2Mw@mail.gmail.com>
Subject: Re: [PATCH] pstore: migrate to crypto acomp interface (take 2)
To:     Kees Cook <keescook@chromium.org>
Cc:     "Guilherme G. Piccoli" <gpiccoli@igalia.com>,
        Anton Vorontsov <anton@enomsg.org>,
        Colin Cross <ccross@android.com>,
        Tony Luck <tony.luck@intel.com>, linux-kernel@vger.kernel.org,
        linux-hardening@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, 17 Oct 2022 at 20:01, Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, Oct 17, 2022 at 01:26:12PM -0300, Guilherme G. Piccoli wrote:
> > On 06/10/2022 20:41, Kees Cook wrote:
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > >
> > > The crypto 'compress' interface is deprecated, so before adding new
> > > features, migrate to the acomp interface. Note that we are only using
> > > synchronous implementations of acomp, so we don't have to deal with
> > > asynchronous completion.
>
> Ard, given your observation about all the per-cpu memory allocation,
> should pstore still go ahead with this conversion?
>

Well, the reason for doing this conversion was so that we could move
the 'worst case buffer size' logic into the individual drivers, as
Herbert would't allow that for legacy comp.

But as we found, we don't really care about the theoretical worst case
of an input that is incompressible - we can just pass the uncompressed
size as the upper bound, and if the crypto fails, we just store the
data uncompressed (which never happens in the first place with ASCII
text)

So once we use the same size for input and output, I was curious
whether we could encrypt in place, and get rid of the big_oops_buf.
And the answer is 'yes', precisely because we have this horrid per-CPU
allocation which serves as a bounce buffer. And this is not specific
to acomp, the old comp algorithms get wrapped in scomps which receive
the same treatment.

So at that point, I wondered what the point is of all this complexity.
Do we really need 6 different algorithms to compress a couple of K of
ASCII text on a code path that is ice cold by definition? Wouldn't it
be better to drop the crypto API altogether here, and just use GZIP
via the library interface?