Re: [dm-devel] [PATCH v8 0/9] crypto: Remove VLA usage
On Mon, Sep 3, 2018 at 10:19 PM, Herbert Xu wrote: > On Tue, Aug 07, 2018 at 02:18:34PM -0700, Kees Cook wrote: >> v8 cover letter: >> >> I continue to hope this can land in v4.19, but I realize that's unlikely. >> It would be nice, though, if some of the "trivial" patches could get taken >> (e.g. cbc, xcbc, ccm VLA removals) so I don't have to keep repeating them. >> *fingers crossed* >> >> Series cover letter: >> >> This is nearly the last of the VLA removals[1], but it's one of the >> largest because crypto gets used in lots of places. After looking >> through code, usage, reading the threads Gustavo started, and comparing >> the use-cases to the other VLA removals that have landed in the kernel, >> I think this series is likely the best way forward to shut the door on >> VLAs forever. >> >> For background, the crypto stack usage is for callers to do an immediate >> bit of work that doesn't allocate new memory. This means that other VLA >> removal techniques (like just using kmalloc) aren't workable, and the >> next common technique is needed: examination of maximum stack usage and >> the addition of sanity checks. This series does that, and in several >> cases, these maximums were already implicit in the code. >> >> This series is intended to land via the crypto tree for 4.19, though it >> touches dm, networking, and a few other things as well, since there are >> dependent patches (new crypto #defines being used, etc). > > I have applied patches 1-4 and 6-8. I'd like to get an ack from > the dm folks regarding patch 5. As to patch 9, please fix it so > it doesn't rely on the BUG_ON to catch things. Great! Thanks very much. I'll get a patch prepared to plumb crypto_skcipher_set_reqsize() failures. -Kees -- Kees Cook Pixel Security -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] dm bufio: Reduce dm_bufio_lock contention
On Wed, 1 Aug 2018, jing xia wrote: > We reproduced this issue again and found out the root cause. > dm_bufio_prefetch() with dm_bufio_lock enters the direct reclaim and > takes a long time to do the soft_limit_reclaim, because of the huge > number of memory excess of the memcg. > Then, all the task who do shrink_slab() wait for dm_bufio_lock. > > Any suggestions for this?Thanks. There's hardly any solution because Michal Hocko refuses to change __GFP_NORETRY behavior. The patches 41c73a49df31151f4ff868f28fe4f129f113fa2c and d12067f428c037b4575aaeb2be00847fc214c24a could reduce the lock contention on the dm-bufio lock - the patches don't fix the high CPU consumption inside the memory allocation, but the kernel code should wait less on the bufio lock. Mikulas > On Thu, Jun 14, 2018 at 3:31 PM, Michal Hocko wrote: > > On Thu 14-06-18 15:18:58, jing xia wrote: > > [...] > >> PID: 22920 TASK: ffc0120f1a00 CPU: 1 COMMAND: "kworker/u8:2" > >> #0 [ffc0282af3d0] __switch_to at ff8008085e48 > >> #1 [ffc0282af3f0] __schedule at ff8008850cc8 > >> #2 [ffc0282af450] schedule at ff8008850f4c > >> #3 [ffc0282af470] schedule_timeout at ff8008853a0c > >> #4 [ffc0282af520] schedule_timeout_uninterruptible at ff8008853aa8 > >> #5 [ffc0282af530] wait_iff_congested at ff8008181b40 > > > > This trace doesn't provide the full picture unfortunately. Waiting in > > the direct reclaim means that the underlying bdi is congested. The real > > question is why it doesn't flush IO in time. > > > >> #6 [ffc0282af5b0] shrink_inactive_list at ff8008177c80 > >> #7 [ffc0282af680] shrink_lruvec at ff8008178510 > >> #8 [ffc0282af790] mem_cgroup_shrink_node_zone at ff80081793bc > >> #9 [ffc0282af840] mem_cgroup_soft_limit_reclaim at ff80081b6040 > >> #10 [ffc0282af8f0] do_try_to_free_pages at ff8008178b6c > >> #11 [ffc0282af990] try_to_free_pages at ff8008178f3c > >> #12 [ffc0282afa30] __perform_reclaim at ff8008169130 > >> #13 [ffc0282afab0] __alloc_pages_nodemask at ff800816c9b8 > >> #14 [ffc0282afbd0] __get_free_pages at ff800816cd6c > >> #15 [ffc0282afbe0] alloc_buffer at ff8008591a94 > >> #16 [ffc0282afc20] __bufio_new at ff8008592e94 > >> #17 [ffc0282afc70] dm_bufio_prefetch at ff8008593198 > >> #18 [ffc0282afd20] verity_prefetch_io at ff8008598384 > >> #19 [ffc0282afd70] process_one_work at ff80080b5b3c > >> #20 [ffc0282afdc0] worker_thread at ff80080b64fc > >> #21 [ffc0282afe20] kthread at ff80080bae34 > >> > >> > Mikulas > > > > -- > > Michal Hocko > > SUSE Labs > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [RESEND PATCH v2] configure: Introduce --enable-symvers option
Dne 31.8.2018 v 16:48 Marcin Niestroj napsal(a): Only few libc (e.g. glibc) libraries support full symbol version resolution in runtime. There are lot of standard libraries that do not support that, such as dietlibc, musl and uclibc. Hence there is no reason to generate symbol versions when compiling against them. Hi Before going into depth of patch itself - I'd like to get clear first what is wrong with existing solution. #if defined(__GNUC__) was supposed to be protecting against problematic usage - but it's more towards 'gcc' compiler usage - where the version is tied to compiler infrastructure. So now you say that other libraries do not support symbol versioning at all (so I'm quite wondering how they are able to handle backward compatibility???) One would have to always introduce completely NEW symbols?? Additionally libdevmapper.so was broken when compiled against uclibc. Runtime linker loader caused calling dm_task_get_info_base() function recursively, leading to segmentation fault. Introduce --enable-symvers[=STYLE] option, which allows to choose between gnu and disabled symbol versioning. By default gnu symbol versioning is used to provide backward compatibility. __GNUC__ check is replaced now with GNU_SYMVER, which is generated by configure script. Additionally ld version script is included only in case of gnu option, which slightly reduces output size. Yep - the idea was to support always 'last symbol' for compilers which do not support symbol versioning. But your case seems to be you use 'gcc' compiler, but surrouding libraries are not 'versioning-aware' ? So what's you plan how to solve backward compatibility - is i.e. 'uclibc' always user in a way 'recompile everything from scratch' ?? Why is the system compiled with 'gcc' not supporting versioning? What is the plan how to resolve binary backward compatibility here? (as you clearly cannot run 'old compiled binary' with new build libdm if you provide only latest symbols in some cases) - is the 'I don't care' policy applied regularly on such system ? Zdenek -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [RFC] [PATCH 0/1] Introduce emergency raid0 stop for mounted arrays
On 09/08/2018 20:17, Guilherme G. Piccoli wrote: > [..] > If you have suggestions to improve my approach, or perhaps a totally > different idea than mine, I highly appreciate the feedback. > > Thank you very much for the attention. > Cheers, > > > Guilherme > Hi Neil and Shaohua Li, sorry for the ping - do you have any news on this? Any feedback is much appreciated. Cheers, Guilherme -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH] crypto: xts - Drop use of auxiliary buffer
Hi Herbert, Mikulas,
I noticed the discussion about using kmalloc() inside crypto code and it
made me wonder if the code in xts.c can actually be simplified to not
require kmalloc() at all, while not badly affecting performace. I played
around with it a little and it turns out we can drop the whole caching
of tweak blocks, reducing code size by ~200 lines and not only preserve,
but even improve the performance in some cases. See the full patch
below.
Obviously, this doesn't solve the general issue of using kmalloc() in
crypto API routines, but it does resolve the original reported problem
and also makes the XTS code easier to maintain.
Regards,
Ondrej
->8-
Since commit acb9b159c784 ("crypto: gf128mul - define gf128mul_x_* in
gf128mul.h"), the gf128mul_x_*() functions are very fast and therefore
caching the computed XTS tweaks has only negligible advantage over
computing them twice.
In fact, since the current caching implementation limits the size of
the calls to the child ecb(...) algorithm to PAGE_SIZE (usually 4096 B),
it is often actually slower than the simple recomputing implementation.
This patch simplifies the XTS template to recompute the XTS tweaks from
scratch in the second pass and thus also removes the need to allocate a
dynamic buffer using kmalloc().
As discussed at [1], the use of kmalloc causes deadlocks with dm-crypt.
PERFORMANCE RESULTS
I measured time to encrypt/decrypt a memory buffer of varying sizes with
xts(ecb-aes-aesni) using a tool I wrote ([2]) and the results suggest
that after this patch the performance is either better or comparable for
both small and large buffers. Note that there is a lot of noise in the
measurements, but the overall difference is easy to see.
Old code:
ALGORITHM KEY (b) DATA (B)TIME ENC (ns) TIME DEC (ns)
xts(aes) 256 64 331 328
xts(aes) 384 64 332 333
xts(aes) 512 64 338 348
xts(aes) 256 512 889 920
xts(aes) 384 5121019 993
xts(aes) 512 5121032 990
xts(aes) 256409621522292
xts(aes) 384409624532597
xts(aes) 512409630412641
xts(aes) 256 1638494438027
xts(aes) 384 1638485368925
xts(aes) 512 1638492329417
xts(aes) 256 32768 16383 14897
xts(aes) 384 32768 17527 16102
xts(aes) 512 32768 18483 17322
New code:
ALGORITHM KEY (b) DATA (B)TIME ENC (ns) TIME DEC (ns)
xts(aes) 256 64 328 324
xts(aes) 384 64 324 319
xts(aes) 512 64 320 322
xts(aes) 256 512 476 473
xts(aes) 384 512 509 492
xts(aes) 512 512 531 514
xts(aes) 256409621321829
xts(aes) 384409623572055
xts(aes) 512409621782027
xts(aes) 256 1638469206983
xts(aes) 384 1638485977505
xts(aes) 512 1638478418164
xts(aes) 256 32768 13468 12307
xts(aes) 384 32768 14808 13402
xts(aes) 512 32768 15753 14636
[1] https://lkml.org/lkml/2018/8/23/1315
[2] https://gitlab.com/omos/linux-crypto-bench
Cc: Mikulas Patocka
Signed-off-by: Ondrej Mosnacek
---
crypto/xts.c | 258 ++-
1 file changed, 30 insertions(+), 228 deletions(-)
diff --git a/crypto/xts.c b/crypto/xts.c
index 12284183bd20..6c49e76df8ca 100644
--- a/crypto/xts.c
+++ b/crypto/xts.c
@@ -26,8 +26,6 @@
#include
#include
-#define XTS_BUFFER_SIZE 128u
-
struct priv {
struct crypto_skcipher *child;
struct crypto_cipher *tweak;
@@ -39,19 +37,7 @@ struct xts_instance_ctx {
};
struct rctx {
- le128 buf[XTS_BUFFER_SIZE / sizeof(le128)];
-
le128 t;
-
- le128 *ext;
-
- struct scatterlist srcbuf[2];
- struct scatterlist dstbuf[2];
- struct scatterlist *src;
- struct scatterlist *dst;
-
- unsigned int left;
-
struct
Re: [dm-devel] (2) FW: RE:(2) FW: RE: FW: (2) FW: Use-after-free while dm-verity
On Mon, Sep 3, 2018 at 9:54 AM, 정혜연 wrote: > > Hi Gilad, > > > > I missed test slot because our test milestone is already ended so I have to > wait kernel minor up for applying his patch (aosp common kernel). > > But I tested by myself to print vmalloc in case of is_vmalloc_addr(data) and > dm corrupted right after printing vmalloc in lowmem situation. > > So I think that Mike's patch would be resolve this issue. > > > > We will keep tracking this issue and update you if any. Great! Thanks, Gilad -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] FW: RE:(2) FW: RE: FW: (2) FW: Use-after-free while dm-verity
Hi HyeYeon, On Wed, Aug 22, 2018 at 8:28 PM, Mike Snitzer wrote: > > Can you please see if this patch helps? > > https://patchwork.kernel.org/patch/10573051/ > I'm curious if the patch pointed to by Mike resolved your issue :-) Gilad -- Gilad Ben-Yossef Chief Coffee Drinker values of β will give rise to dom! -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
