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
| ||
|
Date: Fri, 1 Jan 2021 10:26:46 +0000 From: Ignat Korchagin <ignat@...udflare.com> To: Mikulas Patocka <mpatocka@...hat.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>, Eric Biggers <ebiggers@...nel.org>, Damien Le Moal <Damien.LeMoal@....com>, Herbert Xu <herbert@...dor.apana.org.au>, kernel-team <kernel-team@...udflare.com>, Nobuto Murata <nobuto.murata@...onical.com>, Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>, David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org, "Maciej S. Szmigiero" <mail@...iej.szmigiero.name>, stable@...r.kernel.org Subject: Re: [PATCH v2 1/2] dm crypt: use GFP_ATOMIC when allocating crypto requests from softirq On Fri, Jan 1, 2021 at 10:00 AM Mikulas Patocka <mpatocka@...hat.com> wrote: > > > > On Wed, 30 Dec 2020, Ignat Korchagin wrote: > > > diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c > > index 53791138d78b..e4fd690c70e1 100644 > > --- a/drivers/md/dm-crypt.c > > +++ b/drivers/md/dm-crypt.c > > @@ -1539,7 +1549,10 @@ static blk_status_t crypt_convert(struct crypt_config *cc, > > > > while (ctx->iter_in.bi_size && ctx->iter_out.bi_size) { > > > > - crypt_alloc_req(cc, ctx); > > + r = crypt_alloc_req(cc, ctx); > > + if (r) > > + return BLK_STS_RESOURCE; > > + > > atomic_inc(&ctx->cc_pending); > > > > if (crypt_integrity_aead(cc)) > > -- > > 2.20.1 > > I'm not quite convinced that returning BLK_STS_RESOURCE will help. The > block layer will convert this value back to -ENOMEM and return it to the > caller, resulting in an I/O error. > > Note that GFP_ATOMIC allocations may fail anytime and you must handle > allocation failure gracefully - i.e. process the request without any > error. > > An acceptable solution would be to punt the request to a workqueue and do > GFP_NOIO allocation from the workqueue. Or add the request to some list > and process the list when some other request completes. We can do the workqueue, if that's the desired behaviour. The second patch from this patchset already adds the code for the request to be retried from the workqueue if crypt_convert returns BLK_STS_DEV_RESOURCE, so all is needed is to change returning BLK_STS_RESOURCE to BLK_STS_DEV_RESOURCE here. Does that sound reasonable? > You should write a test that simulates allocation failure and verify that > the kernel handles it gracefully without any I/O error. I already have the test for the second patch in the set, but will adapt it to make sure the allocation failure codepath is covered as well. > Mikulas >
Powered by blists - more mailing lists