* [PATCH -next] crypto: asymmetric_keys - Fix error return code on failure
From: Wei Yongjun @ 2017-02-09 15:57 UTC (permalink / raw)
To: David Howells, Herbert Xu; +Cc: Wei Yongjun, keyrings, linux-crypto
From: Wei Yongjun <weiyongjun1@huawei.com>
Fix to return error code -ENOMEM from the akcipher_request_alloc()
error handling case instead of 0.
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
crypto/asymmetric_keys/public_key.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index 3a23274..3131bba 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -184,8 +184,10 @@ static int software_key_eds_op(struct kernel_pkey_params *params,
return PTR_ERR(tfm);
req = akcipher_request_alloc(tfm, GFP_KERNEL);
- if (!req)
+ if (!req) {
+ ret = -ENOMEM;
goto error_free_tfm;
+ }
if (pkey->key_is_private)
ret = crypto_akcipher_set_priv_key(tfm,
@@ -268,8 +270,10 @@ int public_key_verify_signature(const struct public_key *pkey,
return PTR_ERR(tfm);
req = akcipher_request_alloc(tfm, GFP_KERNEL);
- if (!req)
+ if (!req) {
+ ret = -ENOMEM;
goto error_free_tfm;
+ }
if (pkey->key_is_private)
ret = crypto_akcipher_set_priv_key(tfm,
^ permalink raw reply related
* Re: [PATCH v7 0/5] Update LZ4 compressor module
From: Sven Schmidt @ 2017-02-09 11:05 UTC (permalink / raw)
To: Eric Biggers
Cc: Minchan Kim, akpm, bongkyu.kim, rsalvaterra, sergey.senozhatsky,
gregkh, linux-kernel, herbert, davem, linux-crypto, anton, ccross,
keescook, tony.luck
In-Reply-To: <20170209052425.GA4678@zzz>
Hey Eric,
On Wed, Feb 08, 2017 at 09:24:25PM -0800, Eric Biggers wrote:
> Also I noticed another bug, this time in LZ4_count():
>
> > #if defined(CONFIG_64BIT)
> > #define LZ4_ARCH64 1
> > #else
> > #define LZ4_ARCH64 0
> > #endif
> ...
> > #ifdef LZ4_ARCH64
> > if ((pIn < (pInLimit-3))
> > && (LZ4_read32(pMatch) == LZ4_read32(pIn))) {
> > pIn += 4; pMatch += 4;
> > }
> > #endif
>
> Because of how LZ4_ARCH64 is defined, it needs to be '#if LZ4_ARCH64'.
>
> But I also think the way upstream LZ4 does 64-bit detection could have just been
> left as-is; it has a function which gets inlined:
>
> static unsigned LZ4_64bits(void) { return sizeof(void*)==8; }
>
> Eric
does this apply for LZ4_isLittleEndian() as well? As a reminder:
static unsigned LZ4_isLittleEndian(void)
{
/* don't use static : performance detrimental */
const union { U32 u; BYTE c[4]; } one = { 1 };
return one.c[0];
}
It is surely easier to read and understand using these functions in favor of the macros.
Thanks,
Sven
^ permalink raw reply
* Re: [PATCH v7 0/5] Update LZ4 compressor module
From: Sven Schmidt @ 2017-02-09 11:02 UTC (permalink / raw)
To: Eric Biggers
Cc: Minchan Kim, akpm, bongkyu.kim, rsalvaterra, sergey.senozhatsky,
gregkh, linux-kernel, herbert, davem, linux-crypto, anton, ccross,
keescook, tony.luck
In-Reply-To: <20170209002436.GA103792@gmail.com>
Hey Eric,
On Wed, Feb 08, 2017 at 04:24:36PM -0800, Eric Biggers wrote:
> On Thu, Feb 09, 2017 at 08:31:21AM +0900, Minchan Kim wrote:
> >
> > Today, I did zram-lz4 performance test with fio in current mmotm and
> > found it makes regression about 20%.
> >
>
> This may or may not be the cause of the specific regression you're observing,
> but I just noticed that the proposed patch drops a lot of FORCEINLINE
> annotations from upstream LZ4. The FORCEINLINE's are there for a reason,
> especially for the main decompression and compression functions which are
> basically "templates" that take in different sets of constant parameters, and
> should be left in. We should #define FORCEINLINE to __always_inline somewhere,
> or just do a s/FORCEINLINE/__always_inline/g.
>
I generally just replaced "FORCE_INLINE" by "static inline". At least I thought so.
I rechecked and realised, I missed at least two of them (why did I not just use "search+replace"?).
So I think it's maybe safer and easier to eventually just use "FORCE_INLINE"
with the definition you suggested. Will try that.
> Note that the upstream LZ4 code is very carefully optimized, so we should not,
> in general, be changing things like when functions are force-inlined, what the
> hash table size is, etc.
>
> [Also, for some reason linux-crypto is apparently still not receiving patch 1/5
> in the series. It's missing from the linux-crypto archive at
> http://www.spinics.net/lists/linux-crypto/, so it's not just me.]
>
I don't really know what to do about this. I think the matter is the size of the E-Mail.
Are there filters or something like that? Since in linux-kernel the patch seems to get delivered.
I could otherwise CC you if you wish.
Thanks,
Sven
^ permalink raw reply
* Re: [PATCH v7 0/5] Update LZ4 compressor module
From: Sven Schmidt @ 2017-02-09 10:56 UTC (permalink / raw)
To: Minchan Kim
Cc: akpm, bongkyu.kim, rsalvaterra, sergey.senozhatsky, gregkh,
linux-kernel, herbert, davem, linux-crypto, anton, ccross,
keescook, tony.luck
In-Reply-To: <20170208233121.GA16728@bbox>
Hey Minchan,
On Thu, Feb 09, 2017 at 08:31:21AM +0900, Minchan Kim wrote:
> Hello Sven,
>
> On Sun, Feb 05, 2017 at 08:09:03PM +0100, Sven Schmidt wrote:
> >
> > This patchset is for updating the LZ4 compression module to a version based
> > on LZ4 v1.7.3 allowing to use the fast compression algorithm aka LZ4 fast
> > which provides an "acceleration" parameter as a tradeoff between
> > high compression ratio and high compression speed.
> >
> > We want to use LZ4 fast in order to support compression in lustre
> > and (mostly, based on that) investigate data reduction techniques in behalf of
> > storage systems.
> >
> > Also, it will be useful for other users of LZ4 compression, as with LZ4 fast
> > it is possible to enable applications to use fast and/or high compression
> > depending on the usecase.
> > For instance, ZRAM is offering a LZ4 backend and could benefit from an updated
> > LZ4 in the kernel.
> >
> > LZ4 homepage: http://www.lz4.org/
> > LZ4 source repository: https://github.com/lz4/lz4
> > Source version: 1.7.3
> >
> > Benchmark (taken from [1], Core i5-4300U @1.9GHz):
> > ----------------|--------------|----------------|----------
> > Compressor | Compression | Decompression | Ratio
> > ----------------|--------------|----------------|----------
> > memcpy | 4200 MB/s | 4200 MB/s | 1.000
> > LZ4 fast 50 | 1080 MB/s | 2650 MB/s | 1.375
> > LZ4 fast 17 | 680 MB/s | 2220 MB/s | 1.607
> > LZ4 fast 5 | 475 MB/s | 1920 MB/s | 1.886
> > LZ4 default | 385 MB/s | 1850 MB/s | 2.101
> >
> > [1] http://fastcompression.blogspot.de/2015/04/sampling-or-faster-lz4.html
> >
> > [PATCH 1/5] lib: Update LZ4 compressor module
> > [PATCH 2/5] lib/decompress_unlz4: Change module to work with new LZ4 module version
> > [PATCH 3/5] crypto: Change LZ4 modules to work with new LZ4 module version
> > [PATCH 4/5] fs/pstore: fs/squashfs: Change usage of LZ4 to work with new LZ4 version
> > [PATCH 5/5] lib/lz4: Remove back-compat wrappers
>
> Today, I did zram-lz4 performance test with fio in current mmotm and
> found it makes regression about 20%.
>
> "lz4-update" means current mmots(git://git.cmpxchg.org/linux-mmots.git) so
> applied your 5 patches. (But now sure current mmots has recent uptodate
> patches)
> "revert" means I reverted your 5 patches in current mmots.
>
> revert lz4-update
>
> seq-write 1547 1339 86.55%
> rand-write 22775 19381 85.10%
> seq-read 7035 5589 79.45%
> rand-read 78556 68479 87.17%
> mixed-seq(R) 1305 1066 81.69%
> mixed-seq(W) 1205 984 81.66%
> mixed-rand(R) 17421 14993 86.06%
> mixed-rand(W) 17391 14968 86.07%
which parts of the output (as well as units) are these values exactly?
I did not work with fio until now, so I think I might ask before misinterpreting my results.
> My fio description file
>
> [global]
> bs=4k
> ioengine=sync
> size=100m
> numjobs=1
> group_reporting
> buffer_compress_percentage=30
> scramble_buffers=0
> filename=/dev/zram0
> loops=10
> fsync_on_close=1
>
> [seq-write]
> bs=64k
> rw=write
> stonewall
>
> [rand-write]
> rw=randwrite
> stonewall
>
> [seq-read]
> bs=64k
> rw=read
> stonewall
>
> [rand-read]
> rw=randread
> stonewall
>
> [mixed-seq]
> bs=64k
> rw=rw
> stonewall
>
> [mixed-rand]
> rw=randrw
> stonewall
>
Great, this makes it easy for me to reproduce your test.
Thanks,
Sven
^ permalink raw reply
* Re: [PATCH] Revert "hwrng: core - zeroize buffers with random data"
From: Stephan Müller @ 2017-02-09 9:32 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Daney, Linux Crypto Mailing List, Matt Mackall, Herbert Xu,
Linux Kernel Mailing List
In-Reply-To: <CA+55aFwBdG42kk8J0t2tufHE=OUk4qWXEkFyySNgU3Ru2TA-tQ@mail.gmail.com>
Am Mittwoch, 8. Februar 2017, 17:57:23 CET schrieb Linus Torvalds:
Hi Linus,
> Stephan, Herbert? The zeroes in /dev/hwrng output are obviously
> complete crap, so there's something badly wrong somewhere.
>
> The locking, for example, is completely buggered. There's even a
> comment about it, but that comment makes the correct observation of
> "but y'know: randomness". But the memset() also being outside the lock
> makes a complete joke of the whole thing.
That is correct, the patch is broken and should be reverted.
May I ask, however, why the add_device_randomness is invoked outside the lock
as well. Shouldn't it be moved into the lock?
Besides, I still would think that a memset(0) is needed because we have long-
living memory locations (rng_buffer and rng_fillbuf) which may be overwritten
sporadically. As these memory locations are expected to hold entropy, they
should be overwritten as soon as the data is processed. Obviously, such memset
must be done within the lock.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v2 2/5] async_tx: Handle DMA devices having support for fewer PQ coefficients
From: Anup Patel @ 2017-02-09 9:29 UTC (permalink / raw)
To: Dan Williams
Cc: Vinod Koul, Rob Herring, Mark Rutland, Herbert Xu,
David S . Miller, Jassi Brar, Ray Jui, Scott Branden, Jon Mason,
Rob Rice, BCM Kernel Feedback,
dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Device Tree,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-crypto-u79uwXL29TY76Z2rM5mHXA, linux-raid
In-Reply-To: <CAPcyv4iFJXxvJFrUs2jtwP9GX5NcJ8LiEDHeZ5b1fwjCAToe5w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Feb 8, 2017 at 9:54 PM, Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> On Wed, Feb 8, 2017 at 12:57 AM, Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>> On Tue, Feb 7, 2017 at 11:46 PM, Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>>> On Tue, Feb 7, 2017 at 1:02 AM, Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>>> On Tue, Feb 7, 2017 at 1:57 PM, Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>>>>> On Tue, Feb 7, 2017 at 12:16 AM, Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>>>>> The DMAENGINE framework assumes that if PQ offload is supported by a
>>>>>> DMA device then all 256 PQ coefficients are supported. This assumption
>>>>>> does not hold anymore because we now have BCM-SBA-RAID offload engine
>>>>>> which supports PQ offload with limited number of PQ coefficients.
>>>>>>
>>>>>> This patch extends async_tx APIs to handle DMA devices with support
>>>>>> for fewer PQ coefficients.
>>>>>>
>>>>>> Signed-off-by: Anup Patel <anup.patel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>>>>> Reviewed-by: Scott Branden <scott.branden-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>>>>
>>>>> I don't like this approach. Define an interface for md to query the
>>>>> offload engine once at the beginning of time. We should not be adding
>>>>> any new extensions to async_tx.
>>>>
>>>> Even if we do capability checks in Linux MD, we still need a way
>>>> for DMAENGINE drivers to advertise number of PQ coefficients
>>>> handled by the HW.
>>>>
>>>> I agree capability checks should be done once in Linux MD but I don't
>>>> see why this has to be part of BCM-SBA-RAID driver patches. We need
>>>> separate patchsets to address limitations of async_tx framework.
>>>
>>> Right, separate enabling before we pile on new hardware support to a
>>> known broken framework.
>>
>> Linux Async Tx not broken framework. The issue is:
>> 1. Its not complete enough
>> 2. Its not optimized for very high through-put offload engines
>
> I'm not understanding your point. I'm nak'ing this change to add yet
> more per-transaction capability checking to async_tx. I don't like the
> DMA_HAS_FEWER_PQ_COEF flag, especially since it is equal to
> DMA_HAS_PQ_CONTINUE. I'm not asking for all of async_tx's problems to
> be fixed before this new hardware support, I'm simply saying we should
> start the process of moving offload-engine capability checking to the
> raid code.
The DMA_HAS_FEWER_PQ_COEF is not equal to
DMA_HAS_PQ_CONTINUE.
I will try to drop this patch and take care of unsupported PQ
coefficients in BCM-SBA-RAID driver itself even if this means
doing some computations in BCM-SBA-RAID driver itself.
Regards,
Anup
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] random: Don't overwrite CRNG state in crng_initialize()
From: Stephan Müller @ 2017-02-09 9:26 UTC (permalink / raw)
To: Alden Tondettar
Cc: Greg Kroah-Hartman, Theodore Ts'o, Arnd Bergmann,
linux-crypto, linux-kernel
In-Reply-To: <20170209090432.GA18039@rincewind>
Am Donnerstag, 9. Februar 2017, 02:04:32 CET schrieb Alden Tondettar:
Hi Alden,
> On Thu, Feb 09, 2017 at 07:47:25AM +0100, Greg Kroah-Hartman wrote:
> > On Wed, Feb 08, 2017 at 08:31:26PM -0700, Alden Tondettar wrote:
> > > In short, the situation is:
> > >
> > > A) No usable hardware RNG or arch_get_random() (or we don't trust it...)
> >
> > Wait, why would you not trust arch_get_random()? Is it broken somehow
> > on some arches? If so, why not fix that as well?
>
> arch_get_random() makes use of RDRAND and similar CPU features. Some people
> do not wish to trust black-box RNG implementations.
Furthermore, this function is only implemented on x86. On other arches, it is
a noop.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH] random: Don't overwrite CRNG state in crng_initialize()
From: Greg Kroah-Hartman @ 2017-02-09 9:23 UTC (permalink / raw)
To: Alden Tondettar
Cc: Theodore Ts'o, Arnd Bergmann, linux-crypto, linux-kernel
In-Reply-To: <20170209090432.GA18039@rincewind>
On Thu, Feb 09, 2017 at 02:04:32AM -0700, Alden Tondettar wrote:
> On Thu, Feb 09, 2017 at 07:47:25AM +0100, Greg Kroah-Hartman wrote:
> > On Wed, Feb 08, 2017 at 08:31:26PM -0700, Alden Tondettar wrote:
> > > In short, the situation is:
> > >
> > > A) No usable hardware RNG or arch_get_random() (or we don't trust it...)
> >
> > Wait, why would you not trust arch_get_random()? Is it broken somehow
> > on some arches? If so, why not fix that as well?
>
> arch_get_random() makes use of RDRAND and similar CPU features. Some people
> do not wish to trust black-box RNG implementations.
It does not use those features "raw", it uses the output to feed the
entropy, which, from my understanding, should be the same as feeding any
other form of data into the system, right?
So while it is always nice to worry about different things, I think this
is something that is very low on the "possible to cause an issue" scale.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] random: Don't overwrite CRNG state in crng_initialize()
From: Alden Tondettar @ 2017-02-09 9:04 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Theodore Ts'o, Arnd Bergmann, linux-crypto, linux-kernel
In-Reply-To: <20170209064725.GA4363@kroah.com>
On Thu, Feb 09, 2017 at 07:47:25AM +0100, Greg Kroah-Hartman wrote:
> On Wed, Feb 08, 2017 at 08:31:26PM -0700, Alden Tondettar wrote:
> > In short, the situation is:
> >
> > A) No usable hardware RNG or arch_get_random() (or we don't trust it...)
>
> Wait, why would you not trust arch_get_random()? Is it broken somehow
> on some arches? If so, why not fix that as well?
arch_get_random() makes use of RDRAND and similar CPU features. Some people
do not wish to trust black-box RNG implementations.
Alden
^ permalink raw reply
* Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt
From: Binoy Jayan @ 2017-02-09 8:30 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Oded, Ofir, Herbert Xu, David S. Miller, linux-crypto, Mark Brown,
Arnd Bergmann, Linux kernel mailing list, Alasdair Kergon,
Mike Snitzer, dm-devel, Shaohua Li, linux-raid, Rajendra,
Milan Broz
In-Reply-To: <CAOtvUMcN8s886978vE6T=AE79KkZsLBnXsmhewnb5PB0UyAG3A@mail.gmail.com>
On 8 February 2017 at 13:02, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
>
> Ran Bonnie++ on it last night (Luks mode, plain64, Qemu Virt platform
> Arm64) and it works just fine.
>
> Tested-by: Gilad Ben-Yossef <gilad@benyossef.com>
>
Hi Gilad,
Thank you for testing it. Do you have access to a device having crypto
hardware with IV generation capability and associated drivers for let
say, aes with cbc or any another mode? I was wondering if I can customize
it to work with dm-crypt by generating IVs automatically. Please let
me know your thoughts.
Thanks,
Binoy
^ permalink raw reply
* Re: [PATCH] random: Don't overwrite CRNG state in crng_initialize()
From: Alden Tondettar @ 2017-02-09 8:13 UTC (permalink / raw)
To: Theodore Ts'o
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, linux-kernel
In-Reply-To: <20170209041931.xgkmysquazppiewx@thunk.org>
On Wed, Feb 08, 2017 at 11:19:31PM -0500, Theodore Ts'o wrote:
> How did you determine when crng_initialize() was being called? On a
> VM generally there are fewer interrupts than on real hardware. On
> KVM, for I see the random: fast_init message being printed 3.6 seconds
> into the boot.
>
> On Google Compute Engine, the fast_init message happens 52 seconds into the
> boot.
>
> So what VM where you using? I'm trying to figure out whether this is
> hypothetical or real problem, and on what systems.
Adding a few printk()s to the latest kernel:
@@ -778,6 +778,8 @@ static void crng_initialize(struct crng_state *crng)
int i;
unsigned long rv;
+ printk("crng_initialize called\n");
+
memcpy(&crng->state[0], "expand 32-byte k", 16);
if (crng == &primary_crng)
_extract_entropy(&input_pool, &crng->state[4],
@@ -1149,6 +1151,9 @@ void add_interrupt_randomness(int irq, int irq_flags)
fast_mix(fast_pool);
add_interrupt_bench(cycles);
+ if (fast_pool->count >= 64)
+ printk("add_interrupt_randomness: fast_pool->count >= 64, dumping entropy");
+
if (!crng_ready()) {
if ((fast_pool->count >= 64) &&
crng_fast_load((char *) fast_pool->pool,
And using:
$ qemu-system-x86_64 --version
QEMU emulator version 2.1.2 (Debian 1:2.1+dfsg-12+deb8u6), Copyright (c) 2003-2008 Fabrice Bellard
$ qemu-system-x86_64 -nographic -enable-kvm -m 1024M -kernel bzImage -append "root=/dev/sda1 loglevel=3 console=ttyS0" hd3
I get:
[ 0.010247] mce: CPU supports 10 MCE banks
[ 0.010317] Last level iTLB entries: 4KB 0, 2MB 0, 4MB 0
[ 0.010318] Last level dTLB entries: 4KB 0, 2MB 0, 4MB 0, 1GB 0
[ 0.064002] add_interrupt_randomness: fast_pool->count >= 64, dumping entropy
[ 0.128003] add_interrupt_randomness: fast_pool->count >= 64, dumping entropy
[ 0.160364] Freeing SMP alternatives memory: 36K
[ 0.160428] ftrace: allocating 35771 entries in 140 pages
[ 0.172384] smpboot: Max logical packages: 1
[ 0.173964] ..TIMER: vector=0x30 apic1=0 pin1=2 apic2=-1 pin2=-1
[ 0.184000] add_interrupt_randomness: fast_pool->count >= 64, dumping entropy
[ 0.184000] add_interrupt_randomness: fast_pool->count >= 64, dumping entropy
[ 0.184000] random: fast init done
[ 0.184000] smpboot: CPU0: Intel QEMU Virtual CPU version 2.1.2 (family: 0x6,
model: 0x6, stepping: 0x3)
[ 0.184000] Performance Events: PMU not available due to virtualization, usin
g software events only.
[ 0.184000] crng_initialize called
[ 0.184000] crng_initialize called
[ 0.184000] smp: Bringing up secondary CPUs ...
[ 0.184000] smp: Brought up 1 node, 1 CPU
Sometimes I get three add_interrupt_randomness lines instead of four which
is fine but still cutting things awfully close.
^ permalink raw reply
* Re: [PATCH] random: Don't overwrite CRNG state in crng_initialize()
From: Greg Kroah-Hartman @ 2017-02-09 6:47 UTC (permalink / raw)
To: Alden Tondettar
Cc: Theodore Ts'o, Arnd Bergmann, linux-crypto, linux-kernel
In-Reply-To: <1486611086-2290-1-git-send-email-alden.tondettar@gmail.com>
On Wed, Feb 08, 2017 at 08:31:26PM -0700, Alden Tondettar wrote:
> In short, the situation is:
>
> A) No usable hardware RNG or arch_get_random() (or we don't trust it...)
Wait, why would you not trust arch_get_random()? Is it broken somehow
on some arches? If so, why not fix that as well?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v7 0/5] Update LZ4 compressor module
From: Eric Biggers @ 2017-02-09 5:24 UTC (permalink / raw)
To: Minchan Kim
Cc: Sven Schmidt, akpm, bongkyu.kim, rsalvaterra, sergey.senozhatsky,
gregkh, linux-kernel, herbert, davem, linux-crypto, anton, ccross,
keescook, tony.luck
In-Reply-To: <20170209002436.GA103792@gmail.com>
Also I noticed another bug, this time in LZ4_count():
> #if defined(CONFIG_64BIT)
> #define LZ4_ARCH64 1
> #else
> #define LZ4_ARCH64 0
> #endif
...
> #ifdef LZ4_ARCH64
> if ((pIn < (pInLimit-3))
> && (LZ4_read32(pMatch) == LZ4_read32(pIn))) {
> pIn += 4; pMatch += 4;
> }
> #endif
Because of how LZ4_ARCH64 is defined, it needs to be '#if LZ4_ARCH64'.
But I also think the way upstream LZ4 does 64-bit detection could have just been
left as-is; it has a function which gets inlined:
static unsigned LZ4_64bits(void) { return sizeof(void*)==8; }
Eric
^ permalink raw reply
* Re: [PATCH] random: Don't overwrite CRNG state in crng_initialize()
From: Theodore Ts'o @ 2017-02-09 4:19 UTC (permalink / raw)
To: Alden Tondettar
Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-crypto, linux-kernel
In-Reply-To: <1486611086-2290-1-git-send-email-alden.tondettar@gmail.com>
On Wed, Feb 08, 2017 at 08:31:26PM -0700, Alden Tondettar wrote:
> The new non-blocking system introduced in commit e192be9d9a30 ("random:
> replace non-blocking pool with a Chacha20-based CRNG") can under
> some circumstances report itself initialized while it still contains
> dangerously little entropy, as follows:
>
> Approximately every 64th call to add_interrupt_randomness(), the "fast"
> pool of interrupt-timing-based entropy is fed into one of two places. At
> calls numbered <= 256, the fast pool is XORed into the primary CRNG state.
> At call 256, the CRNG is deemed initialized, getrandom(2) is unblocked,
> and reading from /dev/urandom no longer gives warnings.
>
> At calls > 256, the fast pool is fed into the input pool, leaving the CRNG
> untouched.
>
> The problem arises between call number 256 and 320. If crng_initialize()
> is called at this time, it will overwrite the _entire_ CRNG state with
> 48 bytes generated from the input pool.
So in practice this isn't a problem because crng_initialize is called
in early init. For reference, the ordering of init calls are:
"early", <--- crng_initialize is here()
"core", <---- ftrace is initialized here()
"postcore",
"arch",
"subsys", <---- acpi_init is here()
"fs",
"device", <---- device probing is here
"late",
So in practice, call 256 typically happens **well** after
crng_initialize. You can see where it is the boot messages, which is
after 2.5 seconds into the boot:
[ 2.570733] rtc_cmos 00:02: alarms up to one month, y3k, 114 bytes nvram, hpet irqs
[ 2.570863] usbcore: registered new interface driver i2c-tiny-usb
[ 2.571035] device-mapper: uevent: version 1.0.3
[ 2.571215] random: fast init done <-------------
[ 2.571316] device-mapper: ioctl: 4.35.0-ioctl (2016-06-23) initialised: dm-devel@redhat.com
[ 2.571678] device-mapper: multipath round-robin: version 1.1.0 loaded
[ 2.571728] intel_pstate: Intel P-state driver initializing
[ 2.572331] input: AT Translated Set 2 keyboard as /devices/platform/i8042/serio0/input/input3
[ 2.572462] intel_pstate: HWP enabled
[ 2.572464] sdhci: Secure Digital Host Controller Interface driver
When is crng_initialize() called? Sometime *before* 0.05 seconds into
the boot on my laptop:
[ 0.054529] ftrace: allocating 29140 entries in 114 pages
> In short, the situation is:
>
> A) No usable hardware RNG or arch_get_random() (or we don't trust it...)
> B) add_interrupt_randomness() called 256-320 times but other
> add_*_randomness() functions aren't adding much entropy.
> C) then crng_initialize() is called
> D) not enough calls to add_*_randomness() to push the entropy
> estimate over 128 (yet)
> E) getrandom(2) or /dev/urandom used for something important
>
> Based on a few experiments with VMs, A) through D) can occur easily in
> practice. And with no HDD we have a window of about a minute or two for
> E) to happen before add_interrupt_randomness() finally pushes the
> estimate over 128 on its own.
How did you determine when crng_initialize() was being called? On a
VM generally there are fewer interrupts than on real hardware. On
KVM, for I see the random: fast_init message being printed 3.6 seconds
into the boot.
On Google Compute Engine, the fast_init message happens 52 seconds into the
boot.
So what VM where you using? I'm trying to figure out whether this is
hypothetical or real problem, and on what systems.
- Ted
^ permalink raw reply
* [PATCH] random: Don't overwrite CRNG state in crng_initialize()
From: Alden Tondettar @ 2017-02-09 3:31 UTC (permalink / raw)
To: Theodore Ts'o
Cc: Alden Tondettar, Arnd Bergmann, Greg Kroah-Hartman, linux-crypto,
linux-kernel
The new non-blocking system introduced in commit e192be9d9a30 ("random:
replace non-blocking pool with a Chacha20-based CRNG") can under
some circumstances report itself initialized while it still contains
dangerously little entropy, as follows:
Approximately every 64th call to add_interrupt_randomness(), the "fast"
pool of interrupt-timing-based entropy is fed into one of two places. At
calls numbered <= 256, the fast pool is XORed into the primary CRNG state.
At call 256, the CRNG is deemed initialized, getrandom(2) is unblocked,
and reading from /dev/urandom no longer gives warnings.
At calls > 256, the fast pool is fed into the input pool, leaving the CRNG
untouched.
The problem arises between call number 256 and 320. If crng_initialize()
is called at this time, it will overwrite the _entire_ CRNG state with
48 bytes generated from the input pool. But the add_interrupt_randomness()
entropy was never _in_ the input pool, so instead we destroy all of
add_interrupt_randomness()'s hard work and replace it with the possibly
feeble entropy from a few calls to add_device_randomness(),
init_std_data(), etc.
Nevertheless crng_ready() will happily inform us that getrandom(2) and
/dev/urandom are ready to go. This state of affairs will continue until
the next call to crng_reseed() dumps more entropy into the CRNG and _that_
won't happen until the input pool entropy estimate exceeds 128 bits. On a
system with no rotational drives and little or no user input it could be
a long wait (minutes).
Dumping /var/foo/random-seed into /dev/urandom won't help here because
that only adds entropy to the pool without increasing the estimate.
In short, the situation is:
A) No usable hardware RNG or arch_get_random() (or we don't trust it...)
B) add_interrupt_randomness() called 256-320 times but other
add_*_randomness() functions aren't adding much entropy.
C) then crng_initialize() is called
D) not enough calls to add_*_randomness() to push the entropy
estimate over 128 (yet)
E) getrandom(2) or /dev/urandom used for something important
Based on a few experiments with VMs, A) through D) can occur easily in
practice. And with no HDD we have a window of about a minute or two for
E) to happen before add_interrupt_randomness() finally pushes the
estimate over 128 on its own.
The fix is simple enough: XOR the input pool randomness into the CRNG state
instead of overwriting it.
Fixes: e192be9d9a30 ("random: replace non-blocking pool with a Chacha20-based CRNG")
Signed-off-by: Alden Tondettar <alden.tondettar@gmail.com>
---
drivers/char/random.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 1ef2640..bda30df 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -777,20 +777,22 @@ static void crng_initialize(struct crng_state *crng)
{
int i;
unsigned long rv;
+ __u32 tmp[12];
memcpy(&crng->state[0], "expand 32-byte k", 16);
if (crng == &primary_crng)
- _extract_entropy(&input_pool, &crng->state[4],
- sizeof(__u32) * 12, 0);
+ _extract_entropy(&input_pool, tmp, sizeof(__u32) * 12, 0);
else
- get_random_bytes(&crng->state[4], sizeof(__u32) * 12);
+ get_random_bytes(tmp, sizeof(__u32) * 12);
for (i = 4; i < 16; i++) {
if (!arch_get_random_seed_long(&rv) &&
!arch_get_random_long(&rv))
rv = random_get_entropy();
- crng->state[i] ^= rv;
+ crng->state[i] ^= tmp[i - 4] ^ rv;
}
crng->init_time = jiffies - CRNG_RESEED_INTERVAL - 1;
+
+ memzero_explicit(tmp, sizeof(tmp));
}
static int crng_fast_load(const char *cp, size_t len)
--
2.1.4
^ permalink raw reply related
* Re: [PATCH] Revert "hwrng: core - zeroize buffers with random data"
From: Linus Torvalds @ 2017-02-09 1:57 UTC (permalink / raw)
To: David Daney
Cc: Linux Crypto Mailing List, Matt Mackall, Herbert Xu,
Stephan Mueller, Linux Kernel Mailing List
In-Reply-To: <20170208002331.29408-1-david.daney@cavium.com>
Stephan, Herbert? The zeroes in /dev/hwrng output are obviously
complete crap, so there's something badly wrong somewhere.
The locking, for example, is completely buggered. There's even a
comment about it, but that comment makes the correct observation of
"but y'know: randomness". But the memset() also being outside the lock
makes a complete joke of the whole thing.
Is the hwrng thing even worth maintaining? Compared to something like
/dev/urandom, it clearly does not do a very good job.
So I'm inclined to take the revert, but I'm also somewhat inclined to
simply mark this crud broken when we have other things that clearly do
a lot better.
Linus
On Tue, Feb 7, 2017 at 4:23 PM, David Daney <david.daney@cavium.com> wrote:
> This reverts commit 2cc751545854d7bd7eedf4d7e377bb52e176cd07.
^ permalink raw reply
* Re: [PATCH v7 0/5] Update LZ4 compressor module
From: Eric Biggers @ 2017-02-09 0:24 UTC (permalink / raw)
To: Minchan Kim
Cc: Sven Schmidt, akpm, bongkyu.kim, rsalvaterra, sergey.senozhatsky,
gregkh, linux-kernel, herbert, davem, linux-crypto, anton, ccross,
keescook, tony.luck
In-Reply-To: <20170208233121.GA16728@bbox>
On Thu, Feb 09, 2017 at 08:31:21AM +0900, Minchan Kim wrote:
>
> Today, I did zram-lz4 performance test with fio in current mmotm and
> found it makes regression about 20%.
>
This may or may not be the cause of the specific regression you're observing,
but I just noticed that the proposed patch drops a lot of FORCEINLINE
annotations from upstream LZ4. The FORCEINLINE's are there for a reason,
especially for the main decompression and compression functions which are
basically "templates" that take in different sets of constant parameters, and
should be left in. We should #define FORCEINLINE to __always_inline somewhere,
or just do a s/FORCEINLINE/__always_inline/g.
Note that the upstream LZ4 code is very carefully optimized, so we should not,
in general, be changing things like when functions are force-inlined, what the
hash table size is, etc.
[Also, for some reason linux-crypto is apparently still not receiving patch 1/5
in the series. It's missing from the linux-crypto archive at
http://www.spinics.net/lists/linux-crypto/, so it's not just me.]
Thanks!
Eric
^ permalink raw reply
* Re: [PATCH v7 0/5] Update LZ4 compressor module
From: Minchan Kim @ 2017-02-08 23:31 UTC (permalink / raw)
To: Sven Schmidt
Cc: akpm, bongkyu.kim, rsalvaterra, sergey.senozhatsky, gregkh,
linux-kernel, herbert, davem, linux-crypto, anton, ccross,
keescook, tony.luck
In-Reply-To: <1486321748-19085-1-git-send-email-4sschmid@informatik.uni-hamburg.de>
Hello Sven,
On Sun, Feb 05, 2017 at 08:09:03PM +0100, Sven Schmidt wrote:
>
> This patchset is for updating the LZ4 compression module to a version based
> on LZ4 v1.7.3 allowing to use the fast compression algorithm aka LZ4 fast
> which provides an "acceleration" parameter as a tradeoff between
> high compression ratio and high compression speed.
>
> We want to use LZ4 fast in order to support compression in lustre
> and (mostly, based on that) investigate data reduction techniques in behalf of
> storage systems.
>
> Also, it will be useful for other users of LZ4 compression, as with LZ4 fast
> it is possible to enable applications to use fast and/or high compression
> depending on the usecase.
> For instance, ZRAM is offering a LZ4 backend and could benefit from an updated
> LZ4 in the kernel.
>
> LZ4 homepage: http://www.lz4.org/
> LZ4 source repository: https://github.com/lz4/lz4
> Source version: 1.7.3
>
> Benchmark (taken from [1], Core i5-4300U @1.9GHz):
> ----------------|--------------|----------------|----------
> Compressor | Compression | Decompression | Ratio
> ----------------|--------------|----------------|----------
> memcpy | 4200 MB/s | 4200 MB/s | 1.000
> LZ4 fast 50 | 1080 MB/s | 2650 MB/s | 1.375
> LZ4 fast 17 | 680 MB/s | 2220 MB/s | 1.607
> LZ4 fast 5 | 475 MB/s | 1920 MB/s | 1.886
> LZ4 default | 385 MB/s | 1850 MB/s | 2.101
>
> [1] http://fastcompression.blogspot.de/2015/04/sampling-or-faster-lz4.html
>
> [PATCH 1/5] lib: Update LZ4 compressor module
> [PATCH 2/5] lib/decompress_unlz4: Change module to work with new LZ4 module version
> [PATCH 3/5] crypto: Change LZ4 modules to work with new LZ4 module version
> [PATCH 4/5] fs/pstore: fs/squashfs: Change usage of LZ4 to work with new LZ4 version
> [PATCH 5/5] lib/lz4: Remove back-compat wrappers
Today, I did zram-lz4 performance test with fio in current mmotm and
found it makes regression about 20%.
"lz4-update" means current mmots(git://git.cmpxchg.org/linux-mmots.git) so
applied your 5 patches. (But now sure current mmots has recent uptodate
patches)
"revert" means I reverted your 5 patches in current mmots.
revert lz4-update
seq-write 1547 1339 86.55%
rand-write 22775 19381 85.10%
seq-read 7035 5589 79.45%
rand-read 78556 68479 87.17%
mixed-seq(R) 1305 1066 81.69%
mixed-seq(W) 1205 984 81.66%
mixed-rand(R) 17421 14993 86.06%
mixed-rand(W) 17391 14968 86.07%
My fio description file
[global]
bs=4k
ioengine=sync
size=100m
numjobs=1
group_reporting
buffer_compress_percentage=30
scramble_buffers=0
filename=/dev/zram0
loops=10
fsync_on_close=1
[seq-write]
bs=64k
rw=write
stonewall
[rand-write]
rw=randwrite
stonewall
[seq-read]
bs=64k
rw=read
stonewall
[rand-read]
rw=randread
stonewall
[mixed-seq]
bs=64k
rw=rw
stonewall
[mixed-rand]
rw=randrw
stonewall
^ permalink raw reply
* [PATCH v2 2/2] crypto: aead AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-02-08 20:16 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
In-Reply-To: <2626546.tAxUg930Nc@positron.chronox.de>
The updated memory management is described in the top part of the code.
As one benefit of the changed memory management, the AIO and synchronous
operation is now implemented in one common function. The AF_ALG
operation uses the async kernel crypto API interface for each cipher
operation. Thus, the only difference between the AIO and sync operation
types visible from user space is:
1. the callback function to be invoked when the asynchronous operation
is completed
2. whether to wait for the completion of the kernel crypto API operation
or not
The change includes the overhaul of the TX and RX SGL handling. The TX
SGL holding the data sent from user space to the kernel is now dynamic
similar to algif_skcipher. This dynamic nature allows a continuous
operation of a thread sending data and a second thread receiving the
data. These threads do not need to synchronize as the kernel processes
as much data from the TX SGL to fill the RX SGL.
The caller reading the data from the kernel defines the amount of data
to be processed. Considering that the interface covers AEAD
authenticating ciphers, the reader must provide the buffer in the
correct size. Thus the reader defines the encryption size.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
crypto/algif_aead.c | 668 +++++++++++++++++++++++++++-------------------------
1 file changed, 351 insertions(+), 317 deletions(-)
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 533265f..6c24c81 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -11,6 +11,26 @@
* under the terms of the GNU General Public License as published by the Free
* Software Foundation; either version 2 of the License, or (at your option)
* any later version.
+ *
+ * The following concept of the memory management is used:
+ *
+ * The kernel maintains two SGLs, the TX SGL and the RX SGL. The TX SGL is
+ * filled by user space with the data submitted via sendpage/sendmsg. Filling
+ * up the TX SGL does not cause a crypto operation -- the data will only be
+ * tracked by the kernel. Upon receipt of one recvmsg call, the caller must
+ * provide a buffer which is tracked with the RX SGL.
+ *
+ * During the processing of the recvmsg operation, the cipher request is
+ * allocated and prepared. To support multiple recvmsg operations operating
+ * on one TX SGL, an offset pointer into the TX SGL is maintained. The TX SGL
+ * that is used for the crypto request is scatterwalk_ffwd by the offset
+ * pointer to obtain the start address the crypto operation shall use for
+ * the input data.
+ *
+ * After the completion of the crypto operation, the RX SGL and the cipher
+ * request is released. The processed TX SGL parts are released together with
+ * the RX SGL release and the offset pointer is reduced by the released
+ * data.
*/
#include <crypto/internal/aead.h>
@@ -24,45 +44,55 @@
#include <linux/net.h>
#include <net/sock.h>
-struct aead_sg_list {
- unsigned int cur;
- struct scatterlist sg[ALG_MAX_PAGES];
+struct aead_tsgl {
+ struct list_head list;
+ unsigned int cur; /* Last processed SG entry */
+ struct scatterlist sg[0]; /* Array of SGs forming the SGL */
};
-struct aead_async_rsgl {
+struct aead_rsgl {
struct af_alg_sgl sgl;
struct list_head list;
};
struct aead_async_req {
- struct scatterlist *tsgl;
- struct aead_async_rsgl first_rsgl;
- struct list_head list;
struct kiocb *iocb;
- unsigned int tsgls;
- char iv[];
+ struct sock *sk;
+
+ struct aead_rsgl first_rsgl; /* First RX SG */
+ struct list_head rsgl_list; /* Track RX SGs */
+
+ unsigned int outlen; /* Filled output buf length */
+
+ unsigned int areqlen; /* Length of this data struct */
+ struct aead_request aead_req; /* req ctx trails this struct */
};
struct aead_ctx {
- struct aead_sg_list tsgl;
- struct aead_async_rsgl first_rsgl;
- struct list_head list;
+ struct list_head tsgl_list; /* Link to TX SGL */
void *iv;
+ size_t aead_assoclen;
- struct af_alg_completion completion;
+ struct af_alg_completion completion; /* sync work queue */
- unsigned long used;
+ unsigned int inflight; /* Outstanding AIO ops */
+ size_t used; /* TX bytes sent to kernel */
+ size_t processed; /* Processed TX bytes */
- unsigned int len;
- bool more;
- bool merge;
- bool enc;
+ bool more; /* More data to be expected? */
+ bool merge; /* Merge new data into existing SG */
+ bool enc; /* Crypto operation: enc, dec */
- size_t aead_assoclen;
- struct aead_request aead_req;
+ unsigned int len; /* Length of allocated memory for this struct */
+ struct crypto_aead *aead_tfm;
};
+static DECLARE_WAIT_QUEUE_HEAD(aead_aio_finish_wait);
+
+#define MAX_SGL_ENTS ((4096 - sizeof(struct aead_tsgl)) / \
+ sizeof(struct scatterlist) - 1)
+
static inline int aead_sndbuf(struct sock *sk)
{
struct alg_sock *ask = alg_sk(sk);
@@ -79,42 +109,128 @@ static inline bool aead_writable(struct sock *sk)
static inline bool aead_sufficient_data(struct aead_ctx *ctx)
{
- unsigned as = crypto_aead_authsize(crypto_aead_reqtfm(&ctx->aead_req));
+ unsigned int as = crypto_aead_authsize(ctx->aead_tfm);
/*
* The minimum amount of memory needed for an AEAD cipher is
* the AAD and in case of decryption the tag.
+ *
+ * Also, sufficient data must be available after disregarding the
+ * already processed data.
*/
- return ctx->used >= ctx->aead_assoclen + (ctx->enc ? 0 : as);
+ return ctx->used >= ctx->aead_assoclen + (ctx->enc ? 0 : as) +
+ ctx->processed;
}
-static void aead_reset_ctx(struct aead_ctx *ctx)
+static int aead_alloc_tsgl(struct sock *sk)
{
- struct aead_sg_list *sgl = &ctx->tsgl;
+ struct alg_sock *ask = alg_sk(sk);
+ struct aead_ctx *ctx = ask->private;
+ struct aead_tsgl *sgl;
+ struct scatterlist *sg = NULL;
- sg_init_table(sgl->sg, ALG_MAX_PAGES);
- sgl->cur = 0;
- ctx->used = 0;
- ctx->more = 0;
- ctx->merge = 0;
+ sgl = list_entry(ctx->tsgl_list.prev, struct aead_tsgl, list);
+ if (!list_empty(&ctx->tsgl_list))
+ sg = sgl->sg;
+
+ if (!sg || sgl->cur >= MAX_SGL_ENTS) {
+ sgl = sock_kmalloc(sk, sizeof(*sgl) +
+ sizeof(sgl->sg[0]) * (MAX_SGL_ENTS + 1),
+ GFP_KERNEL);
+ if (!sgl)
+ return -ENOMEM;
+
+ sg_init_table(sgl->sg, MAX_SGL_ENTS + 1);
+ sgl->cur = 0;
+
+ if (sg)
+ sg_chain(sg, MAX_SGL_ENTS + 1, sgl->sg);
+
+ list_add_tail(&sgl->list, &ctx->tsgl_list);
+ }
+
+ return 0;
}
-static void aead_put_sgl(struct sock *sk)
+static void aead_pull_tsgl(struct sock *sk, size_t used)
{
struct alg_sock *ask = alg_sk(sk);
struct aead_ctx *ctx = ask->private;
- struct aead_sg_list *sgl = &ctx->tsgl;
- struct scatterlist *sg = sgl->sg;
+ struct aead_tsgl *sgl;
+ struct scatterlist *sg;
unsigned int i;
- for (i = 0; i < sgl->cur; i++) {
- if (!sg_page(sg + i))
- continue;
+ while (!list_empty(&ctx->tsgl_list)) {
+ sgl = list_first_entry(&ctx->tsgl_list, struct aead_tsgl,
+ list);
+ sg = sgl->sg;
+
+ for (i = 0; i < sgl->cur; i++) {
+ size_t plen = min_t(size_t, used, sg[i].length);
+
+ if (!sg_page(sg + i))
+ continue;
+
+ sg[i].length -= plen;
+ sg[i].offset += plen;
+
+ used -= plen;
+ ctx->used -= plen;
+ ctx->processed -= plen;
- put_page(sg_page(sg + i));
- sg_assign_page(sg + i, NULL);
+ if (sg[i].length)
+ return;
+
+ put_page(sg_page(sg + i));
+ sg_assign_page(sg + i, NULL);
+ }
+
+ list_del(&sgl->list);
+ sock_kfree_s(sk, sgl, sizeof(*sgl) + sizeof(sgl->sg[0]) *
+ (MAX_SGL_ENTS + 1));
+ }
+
+ if (!ctx->used)
+ ctx->merge = 0;
+}
+
+static void aead_free_rsgl(struct aead_async_req *areq)
+{
+ struct sock *sk = areq->sk;
+ struct aead_rsgl *rsgl, *tmp;
+
+ list_for_each_entry_safe(rsgl, tmp, &areq->rsgl_list, list) {
+ af_alg_free_sg(&rsgl->sgl);
+ list_del(&rsgl->list);
+ if (rsgl != &areq->first_rsgl)
+ sock_kfree_s(sk, rsgl, sizeof(*rsgl));
+ }
+}
+
+static int aead_wait_for_wmem(struct sock *sk, unsigned flags)
+{
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
+ int err = -ERESTARTSYS;
+ long timeout;
+
+ if (flags & MSG_DONTWAIT)
+ return -EAGAIN;
+
+ sk_set_bit(SOCKWQ_ASYNC_NOSPACE, sk);
+
+ add_wait_queue(sk_sleep(sk), &wait);
+ for (;;) {
+ if (signal_pending(current))
+ break;
+ timeout = MAX_SCHEDULE_TIMEOUT;
+ if (sk_wait_event(sk, &timeout, aead_writable(sk), &wait)) {
+ err = 0;
+ break;
+ }
}
- aead_reset_ctx(ctx);
+ remove_wait_queue(sk_sleep(sk), &wait);
+
+ return err;
}
static void aead_wmem_wakeup(struct sock *sk)
@@ -146,6 +262,7 @@ static int aead_wait_for_data(struct sock *sk, unsigned flags)
return -EAGAIN;
sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
+
add_wait_queue(sk_sleep(sk), &wait);
for (;;) {
if (signal_pending(current))
@@ -169,8 +286,6 @@ static void aead_data_wakeup(struct sock *sk)
struct aead_ctx *ctx = ask->private;
struct socket_wq *wq;
- if (ctx->more)
- return;
if (!ctx->used)
return;
@@ -189,14 +304,13 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
struct sock *sk = sock->sk;
struct alg_sock *ask = alg_sk(sk);
struct aead_ctx *ctx = ask->private;
- unsigned ivsize =
- crypto_aead_ivsize(crypto_aead_reqtfm(&ctx->aead_req));
- struct aead_sg_list *sgl = &ctx->tsgl;
+ unsigned int ivsize = crypto_aead_ivsize(ctx->aead_tfm);
+ struct aead_tsgl *sgl;
struct af_alg_control con = {};
long copied = 0;
bool enc = 0;
bool init = 0;
- int err = -EINVAL;
+ int err = 0;
if (msg->msg_controllen) {
err = af_alg_cmsg_send(msg, &con);
@@ -220,8 +334,10 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
}
lock_sock(sk);
- if (!ctx->more && ctx->used)
+ if (!ctx->more && ctx->used) {
+ err = -EINVAL;
goto unlock;
+ }
if (init) {
ctx->enc = enc;
@@ -232,11 +348,14 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
}
while (size) {
+ struct scatterlist *sg;
size_t len = size;
- struct scatterlist *sg = NULL;
+ size_t plen;
/* use the existing memory in an allocated page */
if (ctx->merge) {
+ sgl = list_entry(ctx->tsgl_list.prev,
+ struct aead_tsgl, list);
sg = sgl->sg + sgl->cur - 1;
len = min_t(unsigned long, len,
PAGE_SIZE - sg->offset - sg->length);
@@ -257,57 +376,60 @@ static int aead_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
}
if (!aead_writable(sk)) {
- /* user space sent too much data */
- aead_put_sgl(sk);
- err = -EMSGSIZE;
- goto unlock;
+ err = aead_wait_for_wmem(sk, msg->msg_flags);
+ if (err)
+ goto unlock;
}
/* allocate a new page */
len = min_t(unsigned long, size, aead_sndbuf(sk));
- while (len) {
- size_t plen = 0;
- if (sgl->cur >= ALG_MAX_PAGES) {
- aead_put_sgl(sk);
- err = -E2BIG;
- goto unlock;
- }
+ err = aead_alloc_tsgl(sk);
+ if (err)
+ goto unlock;
+
+ sgl = list_entry(ctx->tsgl_list.prev, struct aead_tsgl,
+ list);
+ sg = sgl->sg;
+ if (sgl->cur)
+ sg_unmark_end(sg + sgl->cur - 1);
+
+ do {
+ unsigned int i = sgl->cur;
- sg = sgl->sg + sgl->cur;
plen = min_t(size_t, len, PAGE_SIZE);
- sg_assign_page(sg, alloc_page(GFP_KERNEL));
- err = -ENOMEM;
- if (!sg_page(sg))
+ sg_assign_page(sg + i, alloc_page(GFP_KERNEL));
+ if (!sg_page(sg + i)) {
+ err = -ENOMEM;
goto unlock;
+ }
- err = memcpy_from_msg(page_address(sg_page(sg)),
+ err = memcpy_from_msg(page_address(sg_page(sg + i)),
msg, plen);
if (err) {
- __free_page(sg_page(sg));
- sg_assign_page(sg, NULL);
+ __free_page(sg_page(sg + i));
+ sg_assign_page(sg + i, NULL);
goto unlock;
}
- sg->offset = 0;
- sg->length = plen;
+ sg[i].length = plen;
len -= plen;
ctx->used += plen;
copied += plen;
- sgl->cur++;
size -= plen;
- ctx->merge = plen & (PAGE_SIZE - 1);
- }
+ sgl->cur++;
+ } while (len && sgl->cur < MAX_SGL_ENTS);
+
+ if (!size)
+ sg_mark_end(sg + sgl->cur - 1);
+
+ ctx->merge = plen & (PAGE_SIZE - 1);
}
err = 0;
ctx->more = msg->msg_flags & MSG_MORE;
- if (!ctx->more && !aead_sufficient_data(ctx)) {
- aead_put_sgl(sk);
- err = -EMSGSIZE;
- }
unlock:
aead_data_wakeup(sk);
@@ -322,15 +444,12 @@ static ssize_t aead_sendpage(struct socket *sock, struct page *page,
struct sock *sk = sock->sk;
struct alg_sock *ask = alg_sk(sk);
struct aead_ctx *ctx = ask->private;
- struct aead_sg_list *sgl = &ctx->tsgl;
+ struct aead_tsgl *sgl;
int err = -EINVAL;
if (flags & MSG_SENDPAGE_NOTLAST)
flags |= MSG_MORE;
- if (sgl->cur >= ALG_MAX_PAGES)
- return -E2BIG;
-
lock_sock(sk);
if (!ctx->more && ctx->used)
goto unlock;
@@ -339,13 +458,22 @@ static ssize_t aead_sendpage(struct socket *sock, struct page *page,
goto done;
if (!aead_writable(sk)) {
- /* user space sent too much data */
- aead_put_sgl(sk);
- err = -EMSGSIZE;
- goto unlock;
+ err = aead_wait_for_wmem(sk, flags);
+ if (err)
+ goto unlock;
}
+ err = aead_alloc_tsgl(sk);
+ if (err)
+ goto unlock;
+
ctx->merge = 0;
+ sgl = list_entry(ctx->tsgl_list.prev, struct aead_tsgl, list);
+
+ if (sgl->cur)
+ sg_unmark_end(sgl->sg + sgl->cur - 1);
+
+ sg_mark_end(sgl->sg + sgl->cur);
get_page(page);
sg_set_page(sgl->sg + sgl->cur, page, size, offset);
@@ -356,11 +484,6 @@ static ssize_t aead_sendpage(struct socket *sock, struct page *page,
done:
ctx->more = flags & MSG_MORE;
- if (!ctx->more && !aead_sufficient_data(ctx)) {
- aead_put_sgl(sk);
- err = -EMSGSIZE;
- }
-
unlock:
aead_data_wakeup(sk);
release_sock(sk);
@@ -368,206 +491,62 @@ static ssize_t aead_sendpage(struct socket *sock, struct page *page,
return err ?: size;
}
-#define GET_ASYM_REQ(req, tfm) (struct aead_async_req *) \
- ((char *)req + sizeof(struct aead_request) + \
- crypto_aead_reqsize(tfm))
-
- #define GET_REQ_SIZE(tfm) sizeof(struct aead_async_req) + \
- crypto_aead_reqsize(tfm) + crypto_aead_ivsize(tfm) + \
- sizeof(struct aead_request)
-
static void aead_async_cb(struct crypto_async_request *_req, int err)
{
- struct sock *sk = _req->data;
+ struct aead_async_req *areq = _req->data;
+ struct sock *sk = areq->sk;
struct alg_sock *ask = alg_sk(sk);
struct aead_ctx *ctx = ask->private;
- struct crypto_aead *tfm = crypto_aead_reqtfm(&ctx->aead_req);
- struct aead_request *req = aead_request_cast(_req);
- struct aead_async_req *areq = GET_ASYM_REQ(req, tfm);
- struct scatterlist *sg = areq->tsgl;
- struct aead_async_rsgl *rsgl;
struct kiocb *iocb = areq->iocb;
- unsigned int i, reqlen = GET_REQ_SIZE(tfm);
-
- list_for_each_entry(rsgl, &areq->list, list) {
- af_alg_free_sg(&rsgl->sgl);
- if (rsgl != &areq->first_rsgl)
- sock_kfree_s(sk, rsgl, sizeof(*rsgl));
- }
-
- for (i = 0; i < areq->tsgls; i++)
- put_page(sg_page(sg + i));
-
- sock_kfree_s(sk, areq->tsgl, sizeof(*areq->tsgl) * areq->tsgls);
- sock_kfree_s(sk, req, reqlen);
- __sock_put(sk);
- iocb->ki_complete(iocb, err, err);
-}
-
-static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
- int flags)
-{
- struct sock *sk = sock->sk;
- struct alg_sock *ask = alg_sk(sk);
- struct aead_ctx *ctx = ask->private;
- struct crypto_aead *tfm = crypto_aead_reqtfm(&ctx->aead_req);
- struct aead_async_req *areq;
- struct aead_request *req = NULL;
- struct aead_sg_list *sgl = &ctx->tsgl;
- struct aead_async_rsgl *last_rsgl = NULL, *rsgl;
- unsigned int as = crypto_aead_authsize(tfm);
- unsigned int i, reqlen = GET_REQ_SIZE(tfm);
- int err = -ENOMEM;
- unsigned long used;
- size_t outlen = 0;
- size_t usedpages = 0;
+ unsigned int resultlen;
lock_sock(sk);
- if (ctx->more) {
- err = aead_wait_for_data(sk, flags);
- if (err)
- goto unlock;
- }
-
- if (!aead_sufficient_data(ctx))
- goto unlock;
-
- used = ctx->used;
- if (ctx->enc)
- outlen = used + as;
- else
- outlen = used - as;
-
- req = sock_kmalloc(sk, reqlen, GFP_KERNEL);
- if (unlikely(!req))
- goto unlock;
-
- areq = GET_ASYM_REQ(req, tfm);
- memset(&areq->first_rsgl, '\0', sizeof(areq->first_rsgl));
- INIT_LIST_HEAD(&areq->list);
- areq->iocb = msg->msg_iocb;
- memcpy(areq->iv, ctx->iv, crypto_aead_ivsize(tfm));
- aead_request_set_tfm(req, tfm);
- aead_request_set_ad(req, ctx->aead_assoclen);
- aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- aead_async_cb, sk);
- used -= ctx->aead_assoclen;
-
- /* take over all tx sgls from ctx */
- areq->tsgl = sock_kmalloc(sk,
- sizeof(*areq->tsgl) * max_t(u32, sgl->cur, 1),
- GFP_KERNEL);
- if (unlikely(!areq->tsgl))
- goto free;
-
- sg_init_table(areq->tsgl, max_t(u32, sgl->cur, 1));
- for (i = 0; i < sgl->cur; i++)
- sg_set_page(&areq->tsgl[i], sg_page(&sgl->sg[i]),
- sgl->sg[i].length, sgl->sg[i].offset);
-
- areq->tsgls = sgl->cur;
-
- /* create rx sgls */
- while (outlen > usedpages && iov_iter_count(&msg->msg_iter)) {
- size_t seglen = min_t(size_t, iov_iter_count(&msg->msg_iter),
- (outlen - usedpages));
-
- if (list_empty(&areq->list)) {
- rsgl = &areq->first_rsgl;
-
- } else {
- rsgl = sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
- if (unlikely(!rsgl)) {
- err = -ENOMEM;
- goto free;
- }
- }
- rsgl->sgl.npages = 0;
- list_add_tail(&rsgl->list, &areq->list);
-
- /* make one iovec available as scatterlist */
- err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen);
- if (err < 0)
- goto free;
- usedpages += err;
-
- /* chain the new scatterlist with previous one */
- if (last_rsgl)
- af_alg_link_sg(&last_rsgl->sgl, &rsgl->sgl);
+ BUG_ON(!ctx->inflight);
- last_rsgl = rsgl;
+ /* Buffer size written by crypto operation. */
+ resultlen = areq->outlen;
- iov_iter_advance(&msg->msg_iter, err);
- }
+ aead_free_rsgl(areq);
+ aead_pull_tsgl(sk, areq->aead_req.cryptlen);
+ sock_kfree_s(sk, areq, areq->areqlen);
+ __sock_put(sk);
+ ctx->inflight--;
- /* ensure output buffer is sufficiently large */
- if (usedpages < outlen) {
- err = -EINVAL;
- goto unlock;
- }
+ iocb->ki_complete(iocb, err ? err : resultlen, 0);
- aead_request_set_crypt(req, areq->tsgl, areq->first_rsgl.sgl.sg, used,
- areq->iv);
- err = ctx->enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
- if (err) {
- if (err == -EINPROGRESS) {
- sock_hold(sk);
- err = -EIOCBQUEUED;
- aead_reset_ctx(ctx);
- goto unlock;
- } else if (err == -EBADMSG) {
- aead_put_sgl(sk);
- }
- goto free;
- }
- aead_put_sgl(sk);
-
-free:
- list_for_each_entry(rsgl, &areq->list, list) {
- af_alg_free_sg(&rsgl->sgl);
- if (rsgl != &areq->first_rsgl)
- sock_kfree_s(sk, rsgl, sizeof(*rsgl));
- }
- if (areq->tsgl)
- sock_kfree_s(sk, areq->tsgl, sizeof(*areq->tsgl) * areq->tsgls);
- if (req)
- sock_kfree_s(sk, req, reqlen);
-unlock:
- aead_wmem_wakeup(sk);
release_sock(sk);
- return err ? err : outlen;
+
+ wake_up_interruptible(&aead_aio_finish_wait);
}
-static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
+static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t ignored,
+ int flags)
{
struct sock *sk = sock->sk;
struct alg_sock *ask = alg_sk(sk);
struct aead_ctx *ctx = ask->private;
- unsigned as = crypto_aead_authsize(crypto_aead_reqtfm(&ctx->aead_req));
- struct aead_sg_list *sgl = &ctx->tsgl;
- struct aead_async_rsgl *last_rsgl = NULL;
- struct aead_async_rsgl *rsgl, *tmp;
+ struct crypto_aead *tfm = ctx->aead_tfm;
+ unsigned int as = crypto_aead_authsize(tfm);
+ unsigned int areqlen =
+ sizeof(struct aead_async_req) + crypto_aead_reqsize(tfm);
+ struct aead_tsgl *tsgl;
+ struct aead_async_req *areq;
+ struct aead_rsgl *last_rsgl = NULL;
+ struct scatterlist tsgl_head[2], *tsgl_head_p;
int err = -EINVAL;
- unsigned long used = 0;
- size_t outlen = 0;
- size_t usedpages = 0;
+ size_t used = 0; /* [in] TX bufs to be en/decrypted */
+ size_t outlen = 0; /* [out] RX bufs produced by kernel */
+ size_t usedpages = 0; /* [in] RX bufs to be used from user */
+ size_t processed = 0;
lock_sock(sk);
/*
- * Please see documentation of aead_request_set_crypt for the
- * description of the AEAD memory structure expected from the caller.
+ * Data length provided by caller via sendmsg/sendpage that has not
+ * yet been processed.
*/
-
- if (ctx->more) {
- err = aead_wait_for_data(sk, flags);
- if (err)
- goto unlock;
- }
-
- /* data length provided by caller via sendmsg/sendpage */
- used = ctx->used;
+ used = ctx->used - ctx->processed;
/*
* Make sure sufficient data is present -- note, the same check is
@@ -600,96 +579,151 @@ static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
*/
used -= ctx->aead_assoclen;
- /* convert iovecs of output buffers into scatterlists */
+ /* Allocate cipher request for current operation. */
+ areq = sock_kmalloc(sk, areqlen, GFP_KERNEL);
+ if (unlikely(!areq)) {
+ err = -ENOMEM;
+ goto unlock;
+ }
+ areq->areqlen = areqlen;
+ areq->sk = sk;
+ INIT_LIST_HEAD(&areq->rsgl_list);
+
+ /* convert iovecs of output buffers into RX SGL */
while (outlen > usedpages && iov_iter_count(&msg->msg_iter)) {
- size_t seglen = min_t(size_t, iov_iter_count(&msg->msg_iter),
- (outlen - usedpages));
+ struct aead_rsgl *rsgl;
+ size_t seglen;
+
+ if (!ctx->used) {
+ err = aead_wait_for_data(sk, flags);
+ if (err)
+ goto free;
+ }
+
+ seglen = min_t(size_t, (outlen - usedpages),
+ iov_iter_count(&msg->msg_iter));
- if (list_empty(&ctx->list)) {
- rsgl = &ctx->first_rsgl;
+ if (list_empty(&areq->rsgl_list)) {
+ rsgl = &areq->first_rsgl;
} else {
rsgl = sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
if (unlikely(!rsgl)) {
err = -ENOMEM;
- goto unlock;
+ goto free;
}
}
+
rsgl->sgl.npages = 0;
- list_add_tail(&rsgl->list, &ctx->list);
+ list_add_tail(&rsgl->list, &areq->rsgl_list);
/* make one iovec available as scatterlist */
err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen);
if (err < 0)
- goto unlock;
- usedpages += err;
+ goto free;
+
/* chain the new scatterlist with previous one */
if (last_rsgl)
af_alg_link_sg(&last_rsgl->sgl, &rsgl->sgl);
last_rsgl = rsgl;
-
+ usedpages += err;
iov_iter_advance(&msg->msg_iter, err);
}
- /* ensure output buffer is sufficiently large */
+ /*
+ * Ensure output buffer is sufficiently large. If the caller provides
+ * less buffer space, only use the relative required input size. This
+ * allows AIO operation where the caller sent all data to be processed
+ * and the AIO operation performs the operation on the different chunks
+ * of the input data.
+ */
if (usedpages < outlen) {
- err = -EINVAL;
- goto unlock;
- }
+ size_t less = outlen - usedpages;
- sg_mark_end(sgl->sg + sgl->cur - 1);
- aead_request_set_crypt(&ctx->aead_req, sgl->sg, ctx->first_rsgl.sgl.sg,
- used, ctx->iv);
- aead_request_set_ad(&ctx->aead_req, ctx->aead_assoclen);
+ if (used < less) {
+ err = -EINVAL;
+ goto free;
+ }
+ used -= less;
+ outlen -= less;
+ }
- err = af_alg_wait_for_completion(ctx->enc ?
- crypto_aead_encrypt(&ctx->aead_req) :
- crypto_aead_decrypt(&ctx->aead_req),
+ tsgl = list_first_entry(&ctx->tsgl_list, struct aead_tsgl, list);
+ /* Get the head of the SGL we want to process */
+ tsgl_head_p = scatterwalk_ffwd(tsgl_head, tsgl->sg, ctx->processed);
+ BUG_ON(!tsgl_head_p);
+
+ /* Initialize the crypto operation */
+ aead_request_set_crypt(&areq->aead_req, tsgl_head_p,
+ areq->first_rsgl.sgl.sg, used, ctx->iv);
+ aead_request_set_ad(&areq->aead_req, ctx->aead_assoclen);
+ aead_request_set_tfm(&areq->aead_req, tfm);
+
+ if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) {
+ /* AIO operation */
+ areq->iocb = msg->msg_iocb;
+ aead_request_set_callback(&areq->aead_req,
+ CRYPTO_TFM_REQ_MAY_BACKLOG,
+ aead_async_cb, areq);
+ err = ctx->enc ? crypto_aead_encrypt(&areq->aead_req) :
+ crypto_aead_decrypt(&areq->aead_req);
+ } else {
+ /* Synchronous operation */
+ aead_request_set_callback(&areq->aead_req,
+ CRYPTO_TFM_REQ_MAY_BACKLOG,
+ af_alg_complete, &ctx->completion);
+ err = af_alg_wait_for_completion(ctx->enc ?
+ crypto_aead_encrypt(&areq->aead_req) :
+ crypto_aead_decrypt(&areq->aead_req),
&ctx->completion);
+ }
if (err) {
- /* EBADMSG implies a valid cipher operation took place */
- if (err == -EBADMSG)
- aead_put_sgl(sk);
+ /* AIO operation in progress */
+ if (err == -EINPROGRESS) {
+ sock_hold(sk);
+ err = -EIOCBQUEUED;
- goto unlock;
+ /* Remember the TX bytes that were processed. */
+ ctx->processed += ctx->enc ? (outlen - as) :
+ (outlen + as);
+ ctx->inflight++;
+
+ /* Remember output size that will be generated. */
+ areq->outlen = outlen;
+
+ goto unlock;
+ }
+ /* EBADMSG implies a valid cipher operation took place */
+ else if (err != -EBADMSG)
+ goto free;
}
- aead_put_sgl(sk);
- err = 0;
+ processed = ctx->enc ? (outlen - as) : (outlen + as);
+ /* Remember the TX bytes that were processed. */
+ ctx->processed += processed;
+
+free:
+ aead_free_rsgl(areq);
+ if (areq)
+ sock_kfree_s(sk, areq, areqlen);
+ aead_pull_tsgl(sk, processed);
unlock:
- list_for_each_entry_safe(rsgl, tmp, &ctx->list, list) {
- af_alg_free_sg(&rsgl->sgl);
- list_del(&rsgl->list);
- if (rsgl != &ctx->first_rsgl)
- sock_kfree_s(sk, rsgl, sizeof(*rsgl));
- }
- INIT_LIST_HEAD(&ctx->list);
aead_wmem_wakeup(sk);
release_sock(sk);
-
return err ? err : outlen;
}
-static int aead_recvmsg(struct socket *sock, struct msghdr *msg, size_t ignored,
- int flags)
-{
- return (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) ?
- aead_recvmsg_async(sock, msg, flags) :
- aead_recvmsg_sync(sock, msg, flags);
-}
-
static unsigned int aead_poll(struct file *file, struct socket *sock,
poll_table *wait)
{
struct sock *sk = sock->sk;
struct alg_sock *ask = alg_sk(sk);
struct aead_ctx *ctx = ask->private;
- unsigned int mask;
+ unsigned int mask = 0;
sock_poll_wait(file, sk_sleep(sk), wait);
- mask = 0;
if (!ctx->more)
mask |= POLLIN | POLLRDNORM;
@@ -746,11 +780,14 @@ static void aead_sock_destruct(struct sock *sk)
{
struct alg_sock *ask = alg_sk(sk);
struct aead_ctx *ctx = ask->private;
- unsigned int ivlen = crypto_aead_ivsize(
- crypto_aead_reqtfm(&ctx->aead_req));
+ unsigned int ivlen = crypto_aead_ivsize(ctx->aead_tfm);
WARN_ON(atomic_read(&sk->sk_refcnt) != 0);
- aead_put_sgl(sk);
+
+ /* Suspend caller if AIO operations are in flight. */
+ wait_event_interruptible(aead_aio_finish_wait, (ctx->inflight == 0));
+
+ aead_pull_tsgl(sk, ctx->used);
sock_kzfree_s(sk, ctx->iv, ivlen);
sock_kfree_s(sk, ctx, ctx->len);
af_alg_release_parent(sk);
@@ -760,7 +797,7 @@ static int aead_accept_parent(void *private, struct sock *sk)
{
struct aead_ctx *ctx;
struct alg_sock *ask = alg_sk(sk);
- unsigned int len = sizeof(*ctx) + crypto_aead_reqsize(private);
+ unsigned int len = sizeof(*ctx);
unsigned int ivlen = crypto_aead_ivsize(private);
ctx = sock_kmalloc(sk, len, GFP_KERNEL);
@@ -775,23 +812,20 @@ static int aead_accept_parent(void *private, struct sock *sk)
}
memset(ctx->iv, 0, ivlen);
+ INIT_LIST_HEAD(&ctx->tsgl_list);
ctx->len = len;
ctx->used = 0;
+ ctx->processed = 0;
ctx->more = 0;
ctx->merge = 0;
ctx->enc = 0;
- ctx->tsgl.cur = 0;
+ ctx->inflight = 0;
ctx->aead_assoclen = 0;
af_alg_init_completion(&ctx->completion);
- sg_init_table(ctx->tsgl.sg, ALG_MAX_PAGES);
- INIT_LIST_HEAD(&ctx->list);
+ ctx->aead_tfm = private;
ask->private = ctx;
- aead_request_set_tfm(&ctx->aead_req, private);
- aead_request_set_callback(&ctx->aead_req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- af_alg_complete, &ctx->completion);
-
sk->sk_destruct = aead_sock_destruct;
return 0;
--
2.9.3
^ permalink raw reply related
* [PATCH v2 1/2] crypto: skcipher AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-02-08 20:15 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
In-Reply-To: <2626546.tAxUg930Nc@positron.chronox.de>
The updated memory management is described in the top part of the code.
As one benefit of the changed memory management, the AIO and synchronous
operation is now implemented in one common function. The AF_ALG
operation uses the async kernel crypto API interface for each cipher
operation. Thus, the only difference between the AIO and sync operation
types visible from user space is:
1. the callback function to be invoked when the asynchronous operation
is completed
2. whether to wait for the completion of the kernel crypto API operation
or not
In addition, the code structure is adjusted to match the structure of
algif_aead for easier code assessment.
The user space interface changed slightly as follows: the old AIO
operation returned zero upon success and < 0 in case of an error to user
space. As all other AF_ALG interfaces (including the sync skcipher
interface) returned the number of processed bytes upon success and < 0
in case of an error, the new skcipher interface (regardless of AIO or
sync) returns the number of processed bytes in case of success.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
crypto/algif_skcipher.c | 472 ++++++++++++++++++++----------------------------
1 file changed, 193 insertions(+), 279 deletions(-)
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index a9e79d8..e65ebc4 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -10,6 +10,25 @@
* Software Foundation; either version 2 of the License, or (at your option)
* any later version.
*
+ * The following concept of the memory management is used:
+ *
+ * The kernel maintains two SGLs, the TX SGL and the RX SGL. The TX SGL is
+ * filled by user space with the data submitted via sendpage/sendmsg. Filling
+ * up the TX SGL does not cause a crypto operation -- the data will only be
+ * tracked by the kernel. Upon receipt of one recvmsg call, the caller must
+ * provide a buffer which is tracked with the RX SGL.
+ *
+ * During the processing of the recvmsg operation, the cipher request is
+ * allocated and prepared. To support multiple recvmsg operations operating
+ * on one TX SGL, an offset pointer into the TX SGL is maintained. The TX SGL
+ * that is used for the crypto request is scatterwalk_ffwd by the offset
+ * pointer to obtain the start address the crypto operation shall use for
+ * the input data.
+ *
+ * After the completion of the crypto operation, the RX SGL and the cipher
+ * request is released. The processed TX SGL parts are released together with
+ * the RX SGL release and the offset pointer is reduced by the released
+ * data.
*/
#include <crypto/scatterwalk.h>
@@ -31,78 +50,50 @@ struct skcipher_sg_list {
struct scatterlist sg[0];
};
+struct skcipher_rsgl {
+ struct af_alg_sgl sgl;
+ struct list_head list;
+};
+
+struct skcipher_async_req {
+ struct kiocb *iocb;
+ struct sock *sk;
+
+ struct skcipher_rsgl first_sgl;
+ struct list_head rsgl_list;
+
+ unsigned int areqlen;
+ struct skcipher_request req;
+};
+
struct skcipher_tfm {
struct crypto_skcipher *skcipher;
bool has_key;
};
struct skcipher_ctx {
- struct list_head tsgl;
- struct af_alg_sgl rsgl;
+ struct list_head tsgl_list;
void *iv;
struct af_alg_completion completion;
- atomic_t inflight;
+ unsigned int inflight;
size_t used;
+ size_t processed;
- unsigned int len;
bool more;
bool merge;
bool enc;
- struct skcipher_request req;
-};
-
-struct skcipher_async_rsgl {
- struct af_alg_sgl sgl;
- struct list_head list;
+ unsigned int len;
};
-struct skcipher_async_req {
- struct kiocb *iocb;
- struct skcipher_async_rsgl first_sgl;
- struct list_head list;
- struct scatterlist *tsg;
- atomic_t *inflight;
- struct skcipher_request req;
-};
+static DECLARE_WAIT_QUEUE_HEAD(skcipher_aio_finish_wait);
#define MAX_SGL_ENTS ((4096 - sizeof(struct skcipher_sg_list)) / \
sizeof(struct scatterlist) - 1)
-static void skcipher_free_async_sgls(struct skcipher_async_req *sreq)
-{
- struct skcipher_async_rsgl *rsgl, *tmp;
- struct scatterlist *sgl;
- struct scatterlist *sg;
- int i, n;
-
- list_for_each_entry_safe(rsgl, tmp, &sreq->list, list) {
- af_alg_free_sg(&rsgl->sgl);
- if (rsgl != &sreq->first_sgl)
- kfree(rsgl);
- }
- sgl = sreq->tsg;
- n = sg_nents(sgl);
- for_each_sg(sgl, sg, n, i)
- put_page(sg_page(sg));
-
- kfree(sreq->tsg);
-}
-
-static void skcipher_async_cb(struct crypto_async_request *req, int err)
-{
- struct skcipher_async_req *sreq = req->data;
- struct kiocb *iocb = sreq->iocb;
-
- atomic_dec(sreq->inflight);
- skcipher_free_async_sgls(sreq);
- kzfree(sreq);
- iocb->ki_complete(iocb, err, err);
-}
-
static inline int skcipher_sndbuf(struct sock *sk)
{
struct alg_sock *ask = alg_sk(sk);
@@ -117,15 +108,15 @@ static inline bool skcipher_writable(struct sock *sk)
return PAGE_SIZE <= skcipher_sndbuf(sk);
}
-static int skcipher_alloc_sgl(struct sock *sk)
+static int skcipher_alloc_tsgl(struct sock *sk)
{
struct alg_sock *ask = alg_sk(sk);
struct skcipher_ctx *ctx = ask->private;
struct skcipher_sg_list *sgl;
struct scatterlist *sg = NULL;
- sgl = list_entry(ctx->tsgl.prev, struct skcipher_sg_list, list);
- if (!list_empty(&ctx->tsgl))
+ sgl = list_entry(ctx->tsgl_list.prev, struct skcipher_sg_list, list);
+ if (!list_empty(&ctx->tsgl_list))
sg = sgl->sg;
if (!sg || sgl->cur >= MAX_SGL_ENTS) {
@@ -141,13 +132,13 @@ static int skcipher_alloc_sgl(struct sock *sk)
if (sg)
sg_chain(sg, MAX_SGL_ENTS + 1, sgl->sg);
- list_add_tail(&sgl->list, &ctx->tsgl);
+ list_add_tail(&sgl->list, &ctx->tsgl_list);
}
return 0;
}
-static void skcipher_pull_sgl(struct sock *sk, size_t used, int put)
+static void skcipher_pull_tsgl(struct sock *sk, size_t used)
{
struct alg_sock *ask = alg_sk(sk);
struct skcipher_ctx *ctx = ask->private;
@@ -155,8 +146,8 @@ static void skcipher_pull_sgl(struct sock *sk, size_t used, int put)
struct scatterlist *sg;
int i;
- while (!list_empty(&ctx->tsgl)) {
- sgl = list_first_entry(&ctx->tsgl, struct skcipher_sg_list,
+ while (!list_empty(&ctx->tsgl_list)) {
+ sgl = list_first_entry(&ctx->tsgl_list, struct skcipher_sg_list,
list);
sg = sgl->sg;
@@ -171,30 +162,35 @@ static void skcipher_pull_sgl(struct sock *sk, size_t used, int put)
used -= plen;
ctx->used -= plen;
+ ctx->processed -= plen;
if (sg[i].length)
return;
- if (put)
- put_page(sg_page(sg + i));
+
+ put_page(sg_page(sg + i));
sg_assign_page(sg + i, NULL);
}
list_del(&sgl->list);
- sock_kfree_s(sk, sgl,
- sizeof(*sgl) + sizeof(sgl->sg[0]) *
- (MAX_SGL_ENTS + 1));
+ sock_kfree_s(sk, sgl, sizeof(*sgl) + sizeof(sgl->sg[0]) *
+ (MAX_SGL_ENTS + 1));
}
if (!ctx->used)
ctx->merge = 0;
}
-static void skcipher_free_sgl(struct sock *sk)
+static void skcipher_free_rsgl(struct skcipher_async_req *areq)
{
- struct alg_sock *ask = alg_sk(sk);
- struct skcipher_ctx *ctx = ask->private;
+ struct sock *sk = areq->sk;
+ struct skcipher_rsgl *rsgl, *tmp;
- skcipher_pull_sgl(sk, ctx->used, 1);
+ list_for_each_entry_safe(rsgl, tmp, &areq->rsgl_list, list) {
+ af_alg_free_sg(&rsgl->sgl);
+ list_del(&rsgl->list);
+ if (rsgl != &areq->first_sgl)
+ sock_kfree_s(sk, rsgl, sizeof(*rsgl));
+ }
}
static int skcipher_wait_for_wmem(struct sock *sk, unsigned flags)
@@ -348,7 +344,7 @@ static int skcipher_sendmsg(struct socket *sock, struct msghdr *msg,
size_t plen;
if (ctx->merge) {
- sgl = list_entry(ctx->tsgl.prev,
+ sgl = list_entry(ctx->tsgl_list.prev,
struct skcipher_sg_list, list);
sg = sgl->sg + sgl->cur - 1;
len = min_t(unsigned long, len,
@@ -378,11 +374,12 @@ static int skcipher_sendmsg(struct socket *sock, struct msghdr *msg,
len = min_t(unsigned long, len, skcipher_sndbuf(sk));
- err = skcipher_alloc_sgl(sk);
+ err = skcipher_alloc_tsgl(sk);
if (err)
goto unlock;
- sgl = list_entry(ctx->tsgl.prev, struct skcipher_sg_list, list);
+ sgl = list_entry(ctx->tsgl_list.prev, struct skcipher_sg_list,
+ list);
sg = sgl->sg;
if (sgl->cur)
sg_unmark_end(sg + sgl->cur - 1);
@@ -453,12 +450,12 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
goto unlock;
}
- err = skcipher_alloc_sgl(sk);
+ err = skcipher_alloc_tsgl(sk);
if (err)
goto unlock;
ctx->merge = 0;
- sgl = list_entry(ctx->tsgl.prev, struct skcipher_sg_list, list);
+ sgl = list_entry(ctx->tsgl_list.prev, struct skcipher_sg_list, list);
if (sgl->cur)
sg_unmark_end(sgl->sg + sgl->cur - 1);
@@ -479,25 +476,37 @@ static ssize_t skcipher_sendpage(struct socket *sock, struct page *page,
return err ?: size;
}
-static int skcipher_all_sg_nents(struct skcipher_ctx *ctx)
+static void skcipher_async_cb(struct crypto_async_request *req, int err)
{
- struct skcipher_sg_list *sgl;
- struct scatterlist *sg;
- int nents = 0;
+ struct skcipher_async_req *areq = req->data;
+ struct sock *sk = areq->sk;
+ struct alg_sock *ask = alg_sk(sk);
+ struct skcipher_ctx *ctx = ask->private;
+ struct kiocb *iocb = areq->iocb;
+ unsigned int resultlen;
- list_for_each_entry(sgl, &ctx->tsgl, list) {
- sg = sgl->sg;
+ lock_sock(sk);
- while (!sg->length)
- sg++;
+ BUG_ON(!ctx->inflight);
- nents += sg_nents(sg);
- }
- return nents;
+ /* Buffer size written by crypto operation. */
+ resultlen = areq->req.cryptlen;
+
+ skcipher_free_rsgl(areq);
+ skcipher_pull_tsgl(sk, areq->req.cryptlen);
+ sock_kfree_s(sk, areq, areq->areqlen);
+ __sock_put(sk);
+ ctx->inflight--;
+
+ iocb->ki_complete(iocb, err ? err : resultlen, 0);
+
+ release_sock(sk);
+
+ wake_up_interruptible(&skcipher_aio_finish_wait);
}
-static int skcipher_recvmsg_async(struct socket *sock, struct msghdr *msg,
- int flags)
+static int skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
+ size_t ignored, int flags)
{
struct sock *sk = sock->sk;
struct alg_sock *ask = alg_sk(sk);
@@ -506,215 +515,131 @@ static int skcipher_recvmsg_async(struct socket *sock, struct msghdr *msg,
struct skcipher_ctx *ctx = ask->private;
struct skcipher_tfm *skc = pask->private;
struct crypto_skcipher *tfm = skc->skcipher;
- struct skcipher_sg_list *sgl;
- struct scatterlist *sg;
- struct skcipher_async_req *sreq;
- struct skcipher_request *req;
- struct skcipher_async_rsgl *last_rsgl = NULL;
- unsigned int txbufs = 0, len = 0, tx_nents;
- unsigned int reqsize = crypto_skcipher_reqsize(tfm);
- unsigned int ivsize = crypto_skcipher_ivsize(tfm);
+ unsigned int bs = crypto_skcipher_blocksize(tfm);
+ unsigned int areqlen = sizeof(struct skcipher_async_req) +
+ crypto_skcipher_reqsize(tfm);
+ struct skcipher_sg_list *tsgl;
+ struct skcipher_async_req *areq;
+ struct skcipher_rsgl *last_rsgl = NULL;
+ struct scatterlist tsgl_head[2], *tsgl_head_p;
int err = -ENOMEM;
- bool mark = false;
- char *iv;
-
- sreq = kzalloc(sizeof(*sreq) + reqsize + ivsize, GFP_KERNEL);
- if (unlikely(!sreq))
- goto out;
-
- req = &sreq->req;
- iv = (char *)(req + 1) + reqsize;
- sreq->iocb = msg->msg_iocb;
- INIT_LIST_HEAD(&sreq->list);
- sreq->inflight = &ctx->inflight;
+ size_t len = 0;
lock_sock(sk);
- tx_nents = skcipher_all_sg_nents(ctx);
- sreq->tsg = kcalloc(tx_nents, sizeof(*sg), GFP_KERNEL);
- if (unlikely(!sreq->tsg))
+
+ /* Allocate cipher request for current operation. */
+ areq = sock_kmalloc(sk, areqlen, GFP_KERNEL);
+ if (unlikely(!areq))
goto unlock;
- sg_init_table(sreq->tsg, tx_nents);
- memcpy(iv, ctx->iv, ivsize);
- skcipher_request_set_tfm(req, tfm);
- skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP,
- skcipher_async_cb, sreq);
+ areq->areqlen = areqlen;
+ areq->sk = sk;
+ INIT_LIST_HEAD(&areq->rsgl_list);
- while (iov_iter_count(&msg->msg_iter)) {
- struct skcipher_async_rsgl *rsgl;
- int used;
+ /* convert iovecs of output buffers into RX SGL */
+ while (len < ctx->used && iov_iter_count(&msg->msg_iter)) {
+ struct skcipher_rsgl *rsgl;
+ size_t seglen;
if (!ctx->used) {
err = skcipher_wait_for_data(sk, flags);
if (err)
goto free;
}
- sgl = list_first_entry(&ctx->tsgl,
- struct skcipher_sg_list, list);
- sg = sgl->sg;
- while (!sg->length)
- sg++;
-
- used = min_t(unsigned long, ctx->used,
- iov_iter_count(&msg->msg_iter));
- used = min_t(unsigned long, used, sg->length);
-
- if (txbufs == tx_nents) {
- struct scatterlist *tmp;
- int x;
- /* Ran out of tx slots in async request
- * need to expand */
- tmp = kcalloc(tx_nents * 2, sizeof(*tmp),
- GFP_KERNEL);
- if (!tmp) {
- err = -ENOMEM;
- goto free;
- }
+ seglen = min_t(size_t, ctx->used,
+ iov_iter_count(&msg->msg_iter));
- sg_init_table(tmp, tx_nents * 2);
- for (x = 0; x < tx_nents; x++)
- sg_set_page(&tmp[x], sg_page(&sreq->tsg[x]),
- sreq->tsg[x].length,
- sreq->tsg[x].offset);
- kfree(sreq->tsg);
- sreq->tsg = tmp;
- tx_nents *= 2;
- mark = true;
- }
- /* Need to take over the tx sgl from ctx
- * to the asynch req - these sgls will be freed later */
- sg_set_page(sreq->tsg + txbufs++, sg_page(sg), sg->length,
- sg->offset);
-
- if (list_empty(&sreq->list)) {
- rsgl = &sreq->first_sgl;
- list_add_tail(&rsgl->list, &sreq->list);
+ if (list_empty(&areq->rsgl_list)) {
+ rsgl = &areq->first_sgl;
} else {
- rsgl = kmalloc(sizeof(*rsgl), GFP_KERNEL);
+ rsgl = sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
if (!rsgl) {
err = -ENOMEM;
goto free;
}
- list_add_tail(&rsgl->list, &sreq->list);
}
- used = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, used);
- err = used;
- if (used < 0)
+ rsgl->sgl.npages = 0;
+ list_add_tail(&rsgl->list, &areq->rsgl_list);
+
+ /* make one iovec available as scatterlist */
+ err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen);
+ if (err < 0)
goto free;
+
+ /* chain the new scatterlist with previous one */
if (last_rsgl)
af_alg_link_sg(&last_rsgl->sgl, &rsgl->sgl);
last_rsgl = rsgl;
- len += used;
- skcipher_pull_sgl(sk, used, 0);
- iov_iter_advance(&msg->msg_iter, used);
+ len += err;
+ iov_iter_advance(&msg->msg_iter, err);
}
- if (mark)
- sg_mark_end(sreq->tsg + txbufs - 1);
+ /* Process only as much RX buffers for which we have TX data */
+ if (len > ctx->used)
+ len = ctx->used;
+
+ /*
+ * If more buffers are to be expected to be processed, process only
+ * full block size buffers.
+ */
+ if (ctx->more || len < ctx->used)
+ len -= len % bs;
+
+ tsgl = list_first_entry(&ctx->tsgl_list, struct skcipher_sg_list, list);
+ /* Get the head of the SGL we want to process */
+ tsgl_head_p = scatterwalk_ffwd(tsgl_head, tsgl->sg, ctx->processed);
+ BUG_ON(!tsgl_head_p);
+
+ /* Initialize the crypto operation */
+ skcipher_request_set_tfm(&areq->req, tfm);
+ skcipher_request_set_crypt(&areq->req, tsgl_head_p,
+ areq->first_sgl.sgl.sg, len, ctx->iv);
+
+ if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) {
+ /* AIO operation */
+ areq->iocb = msg->msg_iocb;
+ skcipher_request_set_callback(&areq->req,
+ CRYPTO_TFM_REQ_MAY_SLEEP,
+ skcipher_async_cb, areq);
+ err = ctx->enc ? crypto_skcipher_encrypt(&areq->req) :
+ crypto_skcipher_decrypt(&areq->req);
+ } else {
+ /* Synchronous operation */
+ skcipher_request_set_callback(&areq->req,
+ CRYPTO_TFM_REQ_MAY_SLEEP |
+ CRYPTO_TFM_REQ_MAY_BACKLOG,
+ af_alg_complete,
+ &ctx->completion);
+ err = af_alg_wait_for_completion(ctx->enc ?
+ crypto_skcipher_encrypt(&areq->req) :
+ crypto_skcipher_decrypt(&areq->req),
+ &ctx->completion);
+ }
- skcipher_request_set_crypt(req, sreq->tsg, sreq->first_sgl.sgl.sg,
- len, iv);
- err = ctx->enc ? crypto_skcipher_encrypt(req) :
- crypto_skcipher_decrypt(req);
+ /* AIO operation in progress */
if (err == -EINPROGRESS) {
- atomic_inc(&ctx->inflight);
+ sock_hold(sk);
err = -EIOCBQUEUED;
- sreq = NULL;
+ ctx->inflight++;
+ /* Remember the TX bytes that were processed. */
+ ctx->processed += len;
goto unlock;
- }
-free:
- skcipher_free_async_sgls(sreq);
-unlock:
- skcipher_wmem_wakeup(sk);
- release_sock(sk);
- kzfree(sreq);
-out:
- return err;
-}
-
-static int skcipher_recvmsg_sync(struct socket *sock, struct msghdr *msg,
- int flags)
-{
- struct sock *sk = sock->sk;
- struct alg_sock *ask = alg_sk(sk);
- struct sock *psk = ask->parent;
- struct alg_sock *pask = alg_sk(psk);
- struct skcipher_ctx *ctx = ask->private;
- struct skcipher_tfm *skc = pask->private;
- struct crypto_skcipher *tfm = skc->skcipher;
- unsigned bs = crypto_skcipher_blocksize(tfm);
- struct skcipher_sg_list *sgl;
- struct scatterlist *sg;
- int err = -EAGAIN;
- int used;
- long copied = 0;
-
- lock_sock(sk);
- while (msg_data_left(msg)) {
- if (!ctx->used) {
- err = skcipher_wait_for_data(sk, flags);
- if (err)
- goto unlock;
- }
-
- used = min_t(unsigned long, ctx->used, msg_data_left(msg));
-
- used = af_alg_make_sg(&ctx->rsgl, &msg->msg_iter, used);
- err = used;
- if (err < 0)
- goto unlock;
-
- if (ctx->more || used < ctx->used)
- used -= used % bs;
-
- err = -EINVAL;
- if (!used)
- goto free;
-
- sgl = list_first_entry(&ctx->tsgl,
- struct skcipher_sg_list, list);
- sg = sgl->sg;
-
- while (!sg->length)
- sg++;
-
- skcipher_request_set_crypt(&ctx->req, sg, ctx->rsgl.sg, used,
- ctx->iv);
-
- err = af_alg_wait_for_completion(
- ctx->enc ?
- crypto_skcipher_encrypt(&ctx->req) :
- crypto_skcipher_decrypt(&ctx->req),
- &ctx->completion);
+ } else if (!err)
+ /* Remember the TX bytes that were processed. */
+ ctx->processed += len;
free:
- af_alg_free_sg(&ctx->rsgl);
-
- if (err)
- goto unlock;
-
- copied += used;
- skcipher_pull_sgl(sk, used, 1);
- iov_iter_advance(&msg->msg_iter, used);
- }
-
- err = 0;
+ skcipher_free_rsgl(areq);
+ if (areq)
+ sock_kfree_s(sk, areq, areqlen);
+ skcipher_pull_tsgl(sk, len);
unlock:
skcipher_wmem_wakeup(sk);
release_sock(sk);
-
- return copied ?: err;
-}
-
-static int skcipher_recvmsg(struct socket *sock, struct msghdr *msg,
- size_t ignored, int flags)
-{
- return (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) ?
- skcipher_recvmsg_async(sock, msg, flags) :
- skcipher_recvmsg_sync(sock, msg, flags);
+ return err ? err : len;
}
static unsigned int skcipher_poll(struct file *file, struct socket *sock,
@@ -723,10 +648,9 @@ static unsigned int skcipher_poll(struct file *file, struct socket *sock,
struct sock *sk = sock->sk;
struct alg_sock *ask = alg_sk(sk);
struct skcipher_ctx *ctx = ask->private;
- unsigned int mask;
+ unsigned int mask = 0;
sock_poll_wait(file, sk_sleep(sk), wait);
- mask = 0;
if (ctx->used)
mask |= POLLIN | POLLRDNORM;
@@ -894,26 +818,20 @@ static int skcipher_setkey(void *private, const u8 *key, unsigned int keylen)
return err;
}
-static void skcipher_wait(struct sock *sk)
-{
- struct alg_sock *ask = alg_sk(sk);
- struct skcipher_ctx *ctx = ask->private;
- int ctr = 0;
-
- while (atomic_read(&ctx->inflight) && ctr++ < 100)
- msleep(100);
-}
-
static void skcipher_sock_destruct(struct sock *sk)
{
struct alg_sock *ask = alg_sk(sk);
struct skcipher_ctx *ctx = ask->private;
- struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(&ctx->req);
+ struct sock *psk = ask->parent;
+ struct alg_sock *pask = alg_sk(psk);
+ struct skcipher_tfm *skc = pask->private;
+ struct crypto_skcipher *tfm = skc->skcipher;
- if (atomic_read(&ctx->inflight))
- skcipher_wait(sk);
+ /* Suspend caller if AIO operations are in flight. */
+ wait_event_interruptible(skcipher_aio_finish_wait,
+ (ctx->inflight == 0));
- skcipher_free_sgl(sk);
+ skcipher_pull_tsgl(sk, ctx->used);
sock_kzfree_s(sk, ctx->iv, crypto_skcipher_ivsize(tfm));
sock_kfree_s(sk, ctx, ctx->len);
af_alg_release_parent(sk);
@@ -925,7 +843,7 @@ static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
struct alg_sock *ask = alg_sk(sk);
struct skcipher_tfm *tfm = private;
struct crypto_skcipher *skcipher = tfm->skcipher;
- unsigned int len = sizeof(*ctx) + crypto_skcipher_reqsize(skcipher);
+ unsigned int len = sizeof(*ctx);
ctx = sock_kmalloc(sk, len, GFP_KERNEL);
if (!ctx)
@@ -940,22 +858,18 @@ static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
memset(ctx->iv, 0, crypto_skcipher_ivsize(skcipher));
- INIT_LIST_HEAD(&ctx->tsgl);
+ INIT_LIST_HEAD(&ctx->tsgl_list);
ctx->len = len;
ctx->used = 0;
+ ctx->processed = 0;
+ ctx->inflight = 0;
ctx->more = 0;
ctx->merge = 0;
ctx->enc = 0;
- atomic_set(&ctx->inflight, 0);
af_alg_init_completion(&ctx->completion);
ask->private = ctx;
- skcipher_request_set_tfm(&ctx->req, skcipher);
- skcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_SLEEP |
- CRYPTO_TFM_REQ_MAY_BACKLOG,
- af_alg_complete, &ctx->completion);
-
sk->sk_destruct = skcipher_sock_destruct;
return 0;
--
2.9.3
^ permalink raw reply related
* [PATCH v2 0/2] crypto: AF_ALG memory management fix
From: Stephan Müller @ 2017-02-08 20:14 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
Hi Herbert,
Changes v2:
* import fix from Harsh Jain <harsh@chelsio.com> to remove SG
from list before freeing
* fix return code used for ki_complete to match AIO behavior
with sync behavior
* rename variable list -> tsgl_list
* update the algif_aead patch to include a dynamic TX SGL
allocation similar to what algif_skcipher does. This allows
concurrent continuous read/write operations to the extent
you requested. Although I have not implemented "pairs of
TX/RX SGLs" as I think that is even more overhead, the
implementation conceptually defines such pairs. The recvmsg
call defines how much from the input data is processed.
The caller can have arbitrary number of sendmsg calls
where the data is added to the TX SGL before an recvmsg
asks the kernel to process a given amount (or all) of the
TX SGL.
With the changes, you will see a lot of code duplication now
as I deliberately tried to use the same struct and variable names,
the same function names and even the same oder of functions.
If you agree to this patch, I volunteer to provide a followup
patch that will extract the code duplication into common
functions.
Please find attached memory management updates to
- simplify the code: the old AIO memory management is very
complex and seemingly very fragile -- the update now
eliminates all reported bugs in the skcipher and AEAD
interfaces which allowed the kernel to be crashed by
an unprivileged user
- streamline the code: there is one code path for AIO and sync
operation; the code between algif_skcipher and algif_aead
is very similar (if that patch set is accepted, I volunteer
to reduce code duplication by moving service operations
into af_alg.c and to further unify the TX SGL handling)
- unify the AIO and sync operation which only differ in the
kernel crypto API callback and whether to wait for the
crypto operation or not
- fix all reported bugs regarding the handling of multiple
IOCBs.
The following testing was performed:
- stress testing to verify that no memleaks exist
- testing using Tadeusz Struck AIO test tool (see
https://github.com/tstruk/afalg_async_test) -- the AEAD test
is not applicable any more due to the changed user space
interface; the skcipher test works once the user space
interface change is honored in the test code
- using the libkcapi test suite, all tests including the
originally failing ones (AIO with multiple IOCBs) work now --
the current libkcapi code artificially limits the AEAD
operation to one IOCB. After altering the libkcapi code
to allow multiple IOCBs, the testing works flawless.
Stephan Mueller (2):
crypto: skcipher AF_ALG - overhaul memory management
crypto: aead AF_ALG - overhaul memory management
crypto/algif_aead.c | 668 +++++++++++++++++++++++++-----------------------
crypto/algif_skcipher.c | 472 ++++++++++++++--------------------
2 files changed, 544 insertions(+), 596 deletions(-)
--
2.9.3
^ permalink raw reply
* [PATCH] crypto: ccp - Set the AES size field for all modes
From: Gary R Hook @ 2017-02-08 19:07 UTC (permalink / raw)
To: linux-crypto; +Cc: thomas.lendacky, herbert, davem
Ensure that the size field is correctly populated for
all AES modes.
Signed-off-by: Gary R Hook <gary.hook@amd.com>
---
drivers/crypto/ccp/ccp-dev-v5.c | 3 +--
drivers/crypto/ccp/ccp-dev.h | 1 +
drivers/crypto/ccp/ccp-ops.c | 8 ++++++++
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/ccp/ccp-dev-v5.c b/drivers/crypto/ccp/ccp-dev-v5.c
index 612898b..9c6ff8b8 100644
--- a/drivers/crypto/ccp/ccp-dev-v5.c
+++ b/drivers/crypto/ccp/ccp-dev-v5.c
@@ -284,8 +284,7 @@ static int ccp5_perform_aes(struct ccp_op *op)
CCP_AES_ENCRYPT(&function) = op->u.aes.action;
CCP_AES_MODE(&function) = op->u.aes.mode;
CCP_AES_TYPE(&function) = op->u.aes.type;
- if (op->u.aes.mode == CCP_AES_MODE_CFB)
- CCP_AES_SIZE(&function) = 0x7f;
+ CCP_AES_SIZE(&function) = op->u.aes.size;
CCP5_CMD_FUNCTION(&desc) = function.raw;
diff --git a/drivers/crypto/ccp/ccp-dev.h b/drivers/crypto/ccp/ccp-dev.h
index 649e561..2b5c01f 100644
--- a/drivers/crypto/ccp/ccp-dev.h
+++ b/drivers/crypto/ccp/ccp-dev.h
@@ -467,6 +467,7 @@ struct ccp_aes_op {
enum ccp_aes_type type;
enum ccp_aes_mode mode;
enum ccp_aes_action action;
+ unsigned int size;
};
struct ccp_xts_aes_op {
diff --git a/drivers/crypto/ccp/ccp-ops.c b/drivers/crypto/ccp/ccp-ops.c
index 50fae44..6878160 100644
--- a/drivers/crypto/ccp/ccp-ops.c
+++ b/drivers/crypto/ccp/ccp-ops.c
@@ -692,6 +692,14 @@ static int ccp_run_aes_cmd(struct ccp_cmd_queue *cmd_q, struct ccp_cmd *cmd)
goto e_ctx;
}
}
+ switch (aes->mode) {
+ case CCP_AES_MODE_CFB: /* CFB128 only */
+ case CCP_AES_MODE_CTR:
+ op.u.aes.size = AES_BLOCK_SIZE * BITS_PER_BYTE - 1;
+ break;
+ default:
+ op.u.aes.size = 0;
+ }
/* Prepare the input and output data workareas. For in-place
* operations we need to set the dma direction to BIDIRECTIONAL
^ permalink raw reply related
* Re: [PATCH v2 2/5] async_tx: Handle DMA devices having support for fewer PQ coefficients
From: Dan Williams @ 2017-02-08 16:24 UTC (permalink / raw)
To: Anup Patel
Cc: Vinod Koul, Rob Herring, Mark Rutland, Herbert Xu,
David S . Miller, Jassi Brar, Ray Jui, Scott Branden, Jon Mason,
Rob Rice, BCM Kernel Feedback, dmaengine@vger.kernel.org,
Device Tree, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-crypto, linux-raid
In-Reply-To: <CAALAos9ovab4x=waRgM7G2adRC7syHMkY0Rhf-UPB_+h4w3yNQ@mail.gmail.com>
On Wed, Feb 8, 2017 at 12:57 AM, Anup Patel <anup.patel@broadcom.com> wrote:
> On Tue, Feb 7, 2017 at 11:46 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>> On Tue, Feb 7, 2017 at 1:02 AM, Anup Patel <anup.patel@broadcom.com> wrote:
>>> On Tue, Feb 7, 2017 at 1:57 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>>>> On Tue, Feb 7, 2017 at 12:16 AM, Anup Patel <anup.patel@broadcom.com> wrote:
>>>>> The DMAENGINE framework assumes that if PQ offload is supported by a
>>>>> DMA device then all 256 PQ coefficients are supported. This assumption
>>>>> does not hold anymore because we now have BCM-SBA-RAID offload engine
>>>>> which supports PQ offload with limited number of PQ coefficients.
>>>>>
>>>>> This patch extends async_tx APIs to handle DMA devices with support
>>>>> for fewer PQ coefficients.
>>>>>
>>>>> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
>>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>>
>>>> I don't like this approach. Define an interface for md to query the
>>>> offload engine once at the beginning of time. We should not be adding
>>>> any new extensions to async_tx.
>>>
>>> Even if we do capability checks in Linux MD, we still need a way
>>> for DMAENGINE drivers to advertise number of PQ coefficients
>>> handled by the HW.
>>>
>>> I agree capability checks should be done once in Linux MD but I don't
>>> see why this has to be part of BCM-SBA-RAID driver patches. We need
>>> separate patchsets to address limitations of async_tx framework.
>>
>> Right, separate enabling before we pile on new hardware support to a
>> known broken framework.
>
> Linux Async Tx not broken framework. The issue is:
> 1. Its not complete enough
> 2. Its not optimized for very high through-put offload engines
I'm not understanding your point. I'm nak'ing this change to add yet
more per-transaction capability checking to async_tx. I don't like the
DMA_HAS_FEWER_PQ_COEF flag, especially since it is equal to
DMA_HAS_PQ_CONTINUE. I'm not asking for all of async_tx's problems to
be fixed before this new hardware support, I'm simply saying we should
start the process of moving offload-engine capability checking to the
raid code.
^ permalink raw reply
* [PATCH] crypto: arm/aes-ce: assign err return conditionally
From: Nicholas Mc Guire @ 2017-02-08 13:36 UTC (permalink / raw)
To: Herbert Xu
Cc: linux-kernel, Russell King, Nicholas Mc Guire, linux-crypto,
David S. Miller, linux-arm-kernel
As the err value is not used unless there was an error it can be assigned
conditionally here.
Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
Not sure if this is really relevant and worth changing, effectively it
is practically no change as gcc would move the err = PTR_ERR(simd);
below unregister_simds: anyway (based on inspection of .lst/.s files)
- so it is more of an adjust C-level to object level for readability.
Patch was compile-tested with multi_v7_defconfig
(implies CONFIG_CRYPTO_AES_ARM_CE=m)
Patch is against 4.10-rc7 (localversion-next is next-20170208)
arch/arm/crypto/aes-ce-glue.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm/crypto/aes-ce-glue.c b/arch/arm/crypto/aes-ce-glue.c
index 883b84d..8f65030 100644
--- a/arch/arm/crypto/aes-ce-glue.c
+++ b/arch/arm/crypto/aes-ce-glue.c
@@ -437,9 +437,10 @@ static int __init aes_init(void)
drvname = aes_algs[i].base.cra_driver_name + 2;
basename = aes_algs[i].base.cra_driver_name;
simd = simd_skcipher_create_compat(algname, drvname, basename);
- err = PTR_ERR(simd);
- if (IS_ERR(simd))
+ if (IS_ERR(simd)) {
+ err = PTR_ERR(simd);
goto unregister_simds;
+ }
aes_simd_algs[i] = simd;
}
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v2 2/5] async_tx: Handle DMA devices having support for fewer PQ coefficients
From: Anup Patel @ 2017-02-08 8:57 UTC (permalink / raw)
To: Dan Williams
Cc: Vinod Koul, Rob Herring, Mark Rutland, Herbert Xu,
David S . Miller, Jassi Brar, Ray Jui, Scott Branden, Jon Mason,
Rob Rice, BCM Kernel Feedback, dmaengine@vger.kernel.org,
Device Tree, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-crypto, linux-raid
In-Reply-To: <CAPcyv4hK_1f8bryq6smPQy7vTnP+QEzmT9wnAEE1xpxq7EAnjQ@mail.gmail.com>
On Tue, Feb 7, 2017 at 11:46 PM, Dan Williams <dan.j.williams@intel.com> wrote:
> On Tue, Feb 7, 2017 at 1:02 AM, Anup Patel <anup.patel@broadcom.com> wrote:
>> On Tue, Feb 7, 2017 at 1:57 PM, Dan Williams <dan.j.williams@intel.com> wrote:
>>> On Tue, Feb 7, 2017 at 12:16 AM, Anup Patel <anup.patel@broadcom.com> wrote:
>>>> The DMAENGINE framework assumes that if PQ offload is supported by a
>>>> DMA device then all 256 PQ coefficients are supported. This assumption
>>>> does not hold anymore because we now have BCM-SBA-RAID offload engine
>>>> which supports PQ offload with limited number of PQ coefficients.
>>>>
>>>> This patch extends async_tx APIs to handle DMA devices with support
>>>> for fewer PQ coefficients.
>>>>
>>>> Signed-off-by: Anup Patel <anup.patel@broadcom.com>
>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>
>>> I don't like this approach. Define an interface for md to query the
>>> offload engine once at the beginning of time. We should not be adding
>>> any new extensions to async_tx.
>>
>> Even if we do capability checks in Linux MD, we still need a way
>> for DMAENGINE drivers to advertise number of PQ coefficients
>> handled by the HW.
>>
>> I agree capability checks should be done once in Linux MD but I don't
>> see why this has to be part of BCM-SBA-RAID driver patches. We need
>> separate patchsets to address limitations of async_tx framework.
>
> Right, separate enabling before we pile on new hardware support to a
> known broken framework.
Linux Async Tx not broken framework. The issue is:
1. Its not complete enough
2. Its not optimized for very high through-put offload engines
Regards,
Anup
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox