linux-kernel - Re: [PATCH 1/2] dm crypt: use GFP_ATOMIC when allocating crypto requests from softirq

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>] [day] [month] [year] [list]

Date:   Wed, 30 Dec 2020 11:21:59 +0000
From:   Ignat Korchagin <ignat@...udflare.com>
To:     Hillf Danton <hdanton@...a.com>
Cc:     Alasdair G Kergon <agk@...hat.com>,
        Mike Snitzer <snitzer@...hat.com>,
        device-mapper development <dm-devel@...hat.com>,
        dm-crypt@...ut.de, linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] dm crypt: use GFP_ATOMIC when allocating crypto
 requests from softirq

On Wed, Dec 30, 2020 at 7:36 AM Hillf Danton <hdanton@...a.com> wrote:
>
> On Tue, 29 Dec 2020 22:57:13 +0000
> >
> > Fixes: 39d42fa96ba1 ("dm crypt: add flags to optionally bypass kcryptd workqueues")
>
> Looks like a seperate fix to this commit is needed if what can be found
> at (Subject: [patch 00/12] UBS: Cleanup in_interupt/in_irq/in_atomic() usage)
> https://lore.kernel.org/lkml/20201014145215.518912759@linutronix.de/

I think the above request should be satisfied by device mapper core
code itself rather than individual DM module implementations, as the
execution context in the module is dependent on the underlying block
driver: some block drivers complete requests in task contexts and some
in interrupt - but the underlying block drivers should be transparent
to the modules.

The device mapper core code can pass context information to the
modules if we are to avoid in_*irq() marcos in the code.

> is correct.
>
> > Reported-by: Maciej S. Szmigiero <mail@...iej.szmigiero.name>
> > Cc: <stable@...r.kernel.org> # v5.9+
> > Signed-off-by: Ignat Korchagin <ignat@...udflare.com>
> > ---
> >  drivers/md/dm-crypt.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> > index 5f9f9b3a226d..777b5c71a2f7 100644
> > --- a/drivers/md/dm-crypt.c
> > +++ b/drivers/md/dm-crypt.c
> > @@ -1460,7 +1460,7 @@ static void crypt_alloc_req_skcipher(struct crypt_config *cc,
> >       unsigned key_index = ctx->cc_sector & (cc->tfms_count - 1);
> >
> >       if (!ctx->r.req)
> > -             ctx->r.req = mempool_alloc(&cc->req_pool, GFP_NOIO);
> > +             ctx->r.req = mempool_alloc(&cc->req_pool, in_interrupt() ? GFP_ATOMIC : GFP_NOIO);
> >
> >       skcipher_request_set_tfm(ctx->r.req, cc->cipher_tfm.tfms[key_index]);
> >
> > @@ -1477,7 +1477,7 @@ static void crypt_alloc_req_aead(struct crypt_config *cc,
> >                                struct convert_context *ctx)
> >  {
> >       if (!ctx->r.req_aead)
> > -             ctx->r.req_aead = mempool_alloc(&cc->req_pool, GFP_NOIO);
> > +             ctx->r.req_aead = mempool_alloc(&cc->req_pool, in_interrupt() ? GFP_ATOMIC : GFP_NOIO);
> >
> >       aead_request_set_tfm(ctx->r.req_aead, cc->cipher_tfm.tfms_aead[0]);
> >
> > --
> > 2.20.1