Linux cryptographic layer development
 help / color / mirror / Atom feed
* [PATCH v2 06/10] x86: crypto, annotate local functions
From: Jiri Slaby @ 2017-03-20 12:32 UTC (permalink / raw)
  To: mingo
  Cc: tglx, hpa, x86, jpoimboe, linux-kernel, Jiri Slaby, Herbert Xu,
	David S. Miller, linux-crypto
In-Reply-To: <20170320123222.15453-1-jslaby@suse.cz>

Use the newly added SYM_FUNC_START_LOCAL to annotate starts of all
functions which do not have ".globl" annotation, but their ends are
annotated by ENDPROC. This is needed to balance ENDPROC for tools that
are about to generate debuginfo.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: <x86@kernel.org>
Cc: <linux-crypto@vger.kernel.org>
---
 arch/x86/crypto/aesni-intel_asm.S            | 29 ++++++++++------------------
 arch/x86/crypto/camellia-aesni-avx-asm_64.S  | 10 +++++-----
 arch/x86/crypto/camellia-aesni-avx2-asm_64.S | 10 +++++-----
 arch/x86/crypto/cast5-avx-x86_64-asm_64.S    |  4 ++--
 arch/x86/crypto/cast6-avx-x86_64-asm_64.S    |  4 ++--
 arch/x86/crypto/ghash-clmulni-intel_asm.S    |  2 +-
 arch/x86/crypto/serpent-avx-x86_64-asm_64.S  |  4 ++--
 arch/x86/crypto/serpent-avx2-asm_64.S        |  4 ++--
 arch/x86/crypto/twofish-avx-x86_64-asm_64.S  |  4 ++--
 9 files changed, 31 insertions(+), 40 deletions(-)

diff --git a/arch/x86/crypto/aesni-intel_asm.S b/arch/x86/crypto/aesni-intel_asm.S
index 3c465184ff8a..8913ea70323c 100644
--- a/arch/x86/crypto/aesni-intel_asm.S
+++ b/arch/x86/crypto/aesni-intel_asm.S
@@ -1746,7 +1746,7 @@ ENDPROC(aesni_gcm_enc)
 
 .align 4
 _key_expansion_128:
-_key_expansion_256a:
+SYM_FUNC_START_LOCAL(_key_expansion_256a)
 	pshufd $0b11111111, %xmm1, %xmm1
 	shufps $0b00010000, %xmm0, %xmm4
 	pxor %xmm4, %xmm0
@@ -1759,8 +1759,7 @@ _key_expansion_256a:
 ENDPROC(_key_expansion_128)
 ENDPROC(_key_expansion_256a)
 
-.align 4
-_key_expansion_192a:
+SYM_FUNC_START_LOCAL(_key_expansion_192a)
 	pshufd $0b01010101, %xmm1, %xmm1
 	shufps $0b00010000, %xmm0, %xmm4
 	pxor %xmm4, %xmm0
@@ -1784,8 +1783,7 @@ _key_expansion_192a:
 	ret
 ENDPROC(_key_expansion_192a)
 
-.align 4
-_key_expansion_192b:
+SYM_FUNC_START_LOCAL(_key_expansion_192b)
 	pshufd $0b01010101, %xmm1, %xmm1
 	shufps $0b00010000, %xmm0, %xmm4
 	pxor %xmm4, %xmm0
@@ -1804,8 +1802,7 @@ _key_expansion_192b:
 	ret
 ENDPROC(_key_expansion_192b)
 
-.align 4
-_key_expansion_256b:
+SYM_FUNC_START_LOCAL(_key_expansion_256b)
 	pshufd $0b10101010, %xmm1, %xmm1
 	shufps $0b00010000, %xmm2, %xmm4
 	pxor %xmm4, %xmm2
@@ -1968,8 +1965,7 @@ ENDPROC(aesni_enc)
  *	KEY
  *	TKEYP (T1)
  */
-.align 4
-_aesni_enc1:
+SYM_FUNC_START_LOCAL(_aesni_enc1)
 	movaps (KEYP), KEY		# key
 	mov KEYP, TKEYP
 	pxor KEY, STATE		# round 0
@@ -2032,8 +2028,7 @@ ENDPROC(_aesni_enc1)
  *	KEY
  *	TKEYP (T1)
  */
-.align 4
-_aesni_enc4:
+SYM_FUNC_START_LOCAL(_aesni_enc4)
 	movaps (KEYP), KEY		# key
 	mov KEYP, TKEYP
 	pxor KEY, STATE1		# round 0
@@ -2160,8 +2155,7 @@ ENDPROC(aesni_dec)
  *	KEY
  *	TKEYP (T1)
  */
-.align 4
-_aesni_dec1:
+SYM_FUNC_START_LOCAL(_aesni_dec1)
 	movaps (KEYP), KEY		# key
 	mov KEYP, TKEYP
 	pxor KEY, STATE		# round 0
@@ -2224,8 +2218,7 @@ ENDPROC(_aesni_dec1)
  *	KEY
  *	TKEYP (T1)
  */
-.align 4
-_aesni_dec4:
+SYM_FUNC_START_LOCAL(_aesni_dec4)
 	movaps (KEYP), KEY		# key
 	mov KEYP, TKEYP
 	pxor KEY, STATE1		# round 0
@@ -2591,8 +2584,7 @@ ENDPROC(aesni_cbc_dec)
  *	INC:	== 1, in little endian
  *	BSWAP_MASK == endian swapping mask
  */
-.align 4
-_aesni_inc_init:
+SYM_FUNC_START_LOCAL(_aesni_inc_init)
 	movaps .Lbswap_mask, BSWAP_MASK
 	movaps IV, CTR
 	PSHUFB_XMM BSWAP_MASK CTR
@@ -2617,8 +2609,7 @@ ENDPROC(_aesni_inc_init)
  *	CTR:	== output IV, in little endian
  *	TCTR_LOW: == lower qword of CTR
  */
-.align 4
-_aesni_inc:
+SYM_FUNC_START_LOCAL(_aesni_inc)
 	paddq INC, CTR
 	add $1, TCTR_LOW
 	jnc .Linc_low
diff --git a/arch/x86/crypto/camellia-aesni-avx-asm_64.S b/arch/x86/crypto/camellia-aesni-avx-asm_64.S
index f7c495e2863c..5964d98c4448 100644
--- a/arch/x86/crypto/camellia-aesni-avx-asm_64.S
+++ b/arch/x86/crypto/camellia-aesni-avx-asm_64.S
@@ -188,7 +188,7 @@
  * larger and would only be 0.5% faster (on sandy-bridge).
  */
 .align 8
-roundsm16_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd:
+SYM_FUNC_START_LOCAL(roundsm16_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd)
 	roundsm16(%xmm0, %xmm1, %xmm2, %xmm3, %xmm4, %xmm5, %xmm6, %xmm7,
 		  %xmm8, %xmm9, %xmm10, %xmm11, %xmm12, %xmm13, %xmm14, %xmm15,
 		  %rcx, (%r9));
@@ -196,7 +196,7 @@ roundsm16_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd:
 ENDPROC(roundsm16_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd)
 
 .align 8
-roundsm16_x4_x5_x6_x7_x0_x1_x2_x3_y4_y5_y6_y7_y0_y1_y2_y3_ab:
+SYM_FUNC_START_LOCAL(roundsm16_x4_x5_x6_x7_x0_x1_x2_x3_y4_y5_y6_y7_y0_y1_y2_y3_ab)
 	roundsm16(%xmm4, %xmm5, %xmm6, %xmm7, %xmm0, %xmm1, %xmm2, %xmm3,
 		  %xmm12, %xmm13, %xmm14, %xmm15, %xmm8, %xmm9, %xmm10, %xmm11,
 		  %rax, (%r9));
@@ -721,7 +721,7 @@ ENDPROC(roundsm16_x4_x5_x6_x7_x0_x1_x2_x3_y4_y5_y6_y7_y0_y1_y2_y3_ab)
 .text
 
 .align 8
-__camellia_enc_blk16:
+SYM_FUNC_START_LOCAL(__camellia_enc_blk16)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	%rax: temporary storage, 256 bytes
@@ -808,7 +808,7 @@ __camellia_enc_blk16:
 ENDPROC(__camellia_enc_blk16)
 
 .align 8
-__camellia_dec_blk16:
+SYM_FUNC_START_LOCAL(__camellia_dec_blk16)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	%rax: temporary storage, 256 bytes
@@ -1119,7 +1119,7 @@ ENDPROC(camellia_ctr_16way)
 	vpxor tmp, iv, iv;
 
 .align 8
-camellia_xts_crypt_16way:
+SYM_FUNC_START_LOCAL(camellia_xts_crypt_16way)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	%rsi: dst (16 blocks)
diff --git a/arch/x86/crypto/camellia-aesni-avx2-asm_64.S b/arch/x86/crypto/camellia-aesni-avx2-asm_64.S
index eee5b3982cfd..b3dc6fd6868a 100644
--- a/arch/x86/crypto/camellia-aesni-avx2-asm_64.S
+++ b/arch/x86/crypto/camellia-aesni-avx2-asm_64.S
@@ -227,7 +227,7 @@
  * larger and would only marginally faster.
  */
 .align 8
-roundsm32_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd:
+SYM_FUNC_START_LOCAL(roundsm32_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd)
 	roundsm32(%ymm0, %ymm1, %ymm2, %ymm3, %ymm4, %ymm5, %ymm6, %ymm7,
 		  %ymm8, %ymm9, %ymm10, %ymm11, %ymm12, %ymm13, %ymm14, %ymm15,
 		  %rcx, (%r9));
@@ -235,7 +235,7 @@ roundsm32_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd:
 ENDPROC(roundsm32_x0_x1_x2_x3_x4_x5_x6_x7_y0_y1_y2_y3_y4_y5_y6_y7_cd)
 
 .align 8
-roundsm32_x4_x5_x6_x7_x0_x1_x2_x3_y4_y5_y6_y7_y0_y1_y2_y3_ab:
+SYM_FUNC_START_LOCAL(roundsm32_x4_x5_x6_x7_x0_x1_x2_x3_y4_y5_y6_y7_y0_y1_y2_y3_ab)
 	roundsm32(%ymm4, %ymm5, %ymm6, %ymm7, %ymm0, %ymm1, %ymm2, %ymm3,
 		  %ymm12, %ymm13, %ymm14, %ymm15, %ymm8, %ymm9, %ymm10, %ymm11,
 		  %rax, (%r9));
@@ -764,7 +764,7 @@ ENDPROC(roundsm32_x4_x5_x6_x7_x0_x1_x2_x3_y4_y5_y6_y7_y0_y1_y2_y3_ab)
 .text
 
 .align 8
-__camellia_enc_blk32:
+SYM_FUNC_START_LOCAL(__camellia_enc_blk32)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	%rax: temporary storage, 512 bytes
@@ -851,7 +851,7 @@ __camellia_enc_blk32:
 ENDPROC(__camellia_enc_blk32)
 
 .align 8
-__camellia_dec_blk32:
+SYM_FUNC_START_LOCAL(__camellia_dec_blk32)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	%rax: temporary storage, 512 bytes
@@ -1226,7 +1226,7 @@ ENDPROC(camellia_ctr_32way)
 	vpxor tmp1, iv, iv;
 
 .align 8
-camellia_xts_crypt_32way:
+SYM_FUNC_START_LOCAL(camellia_xts_crypt_32way)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	%rsi: dst (32 blocks)
diff --git a/arch/x86/crypto/cast5-avx-x86_64-asm_64.S b/arch/x86/crypto/cast5-avx-x86_64-asm_64.S
index b4a8806234ea..eb99d6b810f7 100644
--- a/arch/x86/crypto/cast5-avx-x86_64-asm_64.S
+++ b/arch/x86/crypto/cast5-avx-x86_64-asm_64.S
@@ -224,7 +224,7 @@
 .text
 
 .align 16
-__cast5_enc_blk16:
+SYM_FUNC_START_LOCAL(__cast5_enc_blk16)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RL1: blocks 1 and 2
@@ -296,7 +296,7 @@ __cast5_enc_blk16:
 ENDPROC(__cast5_enc_blk16)
 
 .align 16
-__cast5_dec_blk16:
+SYM_FUNC_START_LOCAL(__cast5_dec_blk16)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RL1: encrypted blocks 1 and 2
diff --git a/arch/x86/crypto/cast6-avx-x86_64-asm_64.S b/arch/x86/crypto/cast6-avx-x86_64-asm_64.S
index 952d3156a933..4f15c83e0118 100644
--- a/arch/x86/crypto/cast6-avx-x86_64-asm_64.S
+++ b/arch/x86/crypto/cast6-avx-x86_64-asm_64.S
@@ -262,7 +262,7 @@
 .text
 
 .align 8
-__cast6_enc_blk8:
+SYM_FUNC_START_LOCAL(__cast6_enc_blk8)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RA1, RB1, RC1, RD1, RA2, RB2, RC2, RD2: blocks
@@ -308,7 +308,7 @@ __cast6_enc_blk8:
 ENDPROC(__cast6_enc_blk8)
 
 .align 8
-__cast6_dec_blk8:
+SYM_FUNC_START_LOCAL(__cast6_dec_blk8)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RA1, RB1, RC1, RD1, RA2, RB2, RC2, RD2: encrypted blocks
diff --git a/arch/x86/crypto/ghash-clmulni-intel_asm.S b/arch/x86/crypto/ghash-clmulni-intel_asm.S
index f94375a8dcd1..6d5d80074394 100644
--- a/arch/x86/crypto/ghash-clmulni-intel_asm.S
+++ b/arch/x86/crypto/ghash-clmulni-intel_asm.S
@@ -47,7 +47,7 @@
  *	T2
  *	T3
  */
-__clmul_gf128mul_ble:
+SYM_FUNC_START_LOCAL(__clmul_gf128mul_ble)
 	movaps DATA, T1
 	pshufd $0b01001110, DATA, T2
 	pshufd $0b01001110, SHASH, T3
diff --git a/arch/x86/crypto/serpent-avx-x86_64-asm_64.S b/arch/x86/crypto/serpent-avx-x86_64-asm_64.S
index 2925077f8c6a..8301f918c50f 100644
--- a/arch/x86/crypto/serpent-avx-x86_64-asm_64.S
+++ b/arch/x86/crypto/serpent-avx-x86_64-asm_64.S
@@ -570,7 +570,7 @@
 	transpose_4x4(x0, x1, x2, x3, t0, t1, t2)
 
 .align 8
-__serpent_enc_blk8_avx:
+SYM_FUNC_START_LOCAL(__serpent_enc_blk8_avx)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RA1, RB1, RC1, RD1, RA2, RB2, RC2, RD2: blocks
@@ -624,7 +624,7 @@ __serpent_enc_blk8_avx:
 ENDPROC(__serpent_enc_blk8_avx)
 
 .align 8
-__serpent_dec_blk8_avx:
+SYM_FUNC_START_LOCAL(__serpent_dec_blk8_avx)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RA1, RB1, RC1, RD1, RA2, RB2, RC2, RD2: encrypted blocks
diff --git a/arch/x86/crypto/serpent-avx2-asm_64.S b/arch/x86/crypto/serpent-avx2-asm_64.S
index d67888f2a52a..b011a7ff1d9c 100644
--- a/arch/x86/crypto/serpent-avx2-asm_64.S
+++ b/arch/x86/crypto/serpent-avx2-asm_64.S
@@ -566,7 +566,7 @@
 	transpose_4x4(x0, x1, x2, x3, t0, t1, t2)
 
 .align 8
-__serpent_enc_blk16:
+SYM_FUNC_START_LOCAL(__serpent_enc_blk16)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RA1, RB1, RC1, RD1, RA2, RB2, RC2, RD2: plaintext
@@ -620,7 +620,7 @@ __serpent_enc_blk16:
 ENDPROC(__serpent_enc_blk16)
 
 .align 8
-__serpent_dec_blk16:
+SYM_FUNC_START_LOCAL(__serpent_dec_blk16)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RA1, RB1, RC1, RD1, RA2, RB2, RC2, RD2: ciphertext
diff --git a/arch/x86/crypto/twofish-avx-x86_64-asm_64.S b/arch/x86/crypto/twofish-avx-x86_64-asm_64.S
index b3f49d286348..cf771166b04a 100644
--- a/arch/x86/crypto/twofish-avx-x86_64-asm_64.S
+++ b/arch/x86/crypto/twofish-avx-x86_64-asm_64.S
@@ -249,7 +249,7 @@
 	vpxor		x3, wkey, x3;
 
 .align 8
-__twofish_enc_blk8:
+SYM_FUNC_START_LOCAL(__twofish_enc_blk8)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RA1, RB1, RC1, RD1, RA2, RB2, RC2, RD2: blocks
@@ -291,7 +291,7 @@ __twofish_enc_blk8:
 ENDPROC(__twofish_enc_blk8)
 
 .align 8
-__twofish_dec_blk8:
+SYM_FUNC_START_LOCAL(__twofish_dec_blk8)
 	/* input:
 	 *	%rdi: ctx, CTX
 	 *	RC1, RD1, RA1, RB1, RC2, RD2, RA2, RB2: encrypted blocks
-- 
2.12.0

^ permalink raw reply related

* Re: Question - seeding the hw pseudo random number generator
From: PrasannaKumar Muralidharan @ 2017-03-20  6:49 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Matt Mackall, Herbert Xu, linux-crypto, linux-arm-kernel
In-Reply-To: <20170318092554.lggkhfg5eko23o3k@kozik-lap>

> I looked at Exynos Pseudo Random Nubmer Generator driver
> (drivers/char/hw_random/exynos-rng.c) and noticed that it always seeds
> the device with jiffies.  Then I looked at few other drivers and found
> that they do not seed themself (or at least I couldn't find this).

HW random interface is meant for true RNG, not pseudo RNG. Actually
PRNGs should use AF_ALG interface. I think exynos-rng.c should follow
the same.

> I think the hw_random API does not provide generic infrastructure for
> seeding.
>
> What is the preferred approach for seeding a PRNG device? Use jiffies or
> a fixed value?
>
> Or maybe the interface should be abandoned in favor of crypto API?

AF_ALG interface for rng does have seeding support. I think hw_random
does not provide seeding support intentionally as I understand that
True RNG need not require seeding (please correct me if I am wrong).

Regards,
PrasannaKumar

^ permalink raw reply

* Re: [PATCH v5 1/2] crypto: skcipher AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-03-19  4:34 UTC (permalink / raw)
  To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20170316095248.GA11996@gondor.apana.org.au>

Am Donnerstag, 16. März 2017, 10:52:48 CET schrieb Herbert Xu:

Hi Herbert,
> 
> First of all you're only limiting the amount of memory occupied
> by the SG list which is not the same thing as the memory pinned
> down by the actual recvmsg.

When considering af_alg_make_sg, the function iov_iter_get_pages is used to 
obtain the user space pages for recvmsg. When looking closely at that 
function, I would understand that the user space pages are pinned into memory 
and made accessible from kernel space. That said, I would infer from the code 
that no kernel-local memory is allocated for the real data. Similar to zero-
copy, pages are shared between kernel and user that will hold the crypto 
operation result (ciphertext for enc or plaintext for dec).

Therefore, I would infer that the memory usage of the discussed code is 
limited to the skcipher_rsgl meta data which is limited by sock_kmalloc. This 
finally would imply that the kernel will not occupy more memory than allocated 
by sock_kmalloc.

Or am I missing something?

Thanks
Stephan

^ permalink raw reply

* Re: [ANNOUNCE] /dev/random - a new approach (code for 4.11-rc1)
From: Stephan Müller @ 2017-03-18 16:36 UTC (permalink / raw)
  To: noloader; +Cc: LKML, linux-crypto
In-Reply-To: <CAH8yC8nHEQfZCf0j0rcW9KV-DNzkv-Sowch4ROK07HXzT5bFCA@mail.gmail.com>

Am Samstag, 18. März 2017, 14:43:18 CET schrieb Jeffrey Walton:

Hi Jeffrey,

> > I am not sure how this statement relates to the quote above. RDSEED is the
> > CBC-MACed output of the flip-flop providing the raw noise.
> > 
> > RDRAND is the output of the SP800-90A CTR DRBG that is seeded by the
> > CBC-MAC that also feeds RDSEED. Thus, RDSEED is as fast as the noise
> > source where RDRAND is a pure deterministic RNG that tries to be
> > (re)seeded as often as possible.
> > 
> > Both instructions are totally unrelated to the SP800-90A DRBG available to
> > the Linux kernel.
> 
> SP800-90A requires an entropy source to bootstrap the Hash, HMAC and
> CTR generators. That is, the Instantiate and Reseed functions need an
> approved source of entropy. Both RDRAND and RDSEED are approved for
> Intel chips. See SP800-90A, Section 8.6.5
> (http://csrc.nist.gov/publications/nistpubs/800-90A/SP800-90A.pdf).

I am aware that SP800-90A makes the claim of having an approved noise source. 
But as of now, there is no such thing.

NIST is aware of that issue. To cover that issue during a FIPS 140-2 
validation, you have to prove your noise sources to be compliant to SP800-90B. 
I performed such noise source assessments as part of the FIPS 140-2 
validations of the Intel Sunrise Point or the Qualcomm HW DRBG FIPS 140-2 
validations. Also, I completed such assessments for the FIPS 140-2 validations 
of the legady /dev/random covering numerous Linux-based cryptographic modules 
over the last couple of years.

To get a glimpse of how such assessments for FIPS 140-2 are conducted, please 
have a look at the assessment [1] section 5.3.2.1 starting on page 72 in the 
lower half (note that I was the main author of this study). To be honest, the 
assessment in [1] section 5.5 was the main motivation for my LRNG 
implementation.

That said, [2] section 3.4.1, starting at page 34 bottom, you see the same 
SP800-90B test approach that was equally accepted by NIST during formal FIPS 
140-2 validations of other noise sources. Hence, I would conclude that my 
suggested implementation of the noise source is appropriate for a DRBG to be 
compliant to section 8.6.5 of SP800-90A.

But you mention a very important topic: is it and how is it ensured that the 
DRBG is appropriately seeded. This issue is discussed in [2] section 2.1 which 
explains the initial, minimal and full seeded stages of the DRBG.

[1] https://www.bsi.bund.de/SharedDocs/Downloads/DE/BSI/Publikationen/Studien/
ZufallinVMS/Randomness-in-VMs.pdf

[2] http://www.chronox.de/lrng/doc/lrng.pdf

Ciao
Stephan

^ permalink raw reply

* Re: [ANNOUNCE] /dev/random - a new approach (code for 4.11-rc1)
From: Jeffrey Walton @ 2017-03-18 13:43 UTC (permalink / raw)
  To: Stephan Müller; +Cc: LKML, linux-crypto
In-Reply-To: <4792500.irvFnm0WRl@positron.chronox.de>

>> > The design and implementation is driven by a set of goals described in [2]
>> > that the LRNG completely implements. Furthermore, [2] includes a
>> > comparison with RNG design suggestions such as SP800-90B, SP800-90C, and
>> > AIS20/31.
>>
>> A quick comment about SP800 and the hardware instructions... RDSEED is
>> 2 to 5 times slower than RDRAND on Intel hardware, depending on the
>> architecture and microarchitecture.
>
> I am not sure how this statement relates to the quote above. RDSEED is the
> CBC-MACed output of the flip-flop providing the raw noise.
>
> RDRAND is the output of the SP800-90A CTR DRBG that is seeded by the CBC-MAC
> that also feeds RDSEED. Thus, RDSEED is as fast as the noise source where
> RDRAND is a pure deterministic RNG that tries to be (re)seeded as often as
> possible.
>
> Both instructions are totally unrelated to the SP800-90A DRBG available to the
> Linux kernel.

SP800-90A requires an entropy source to bootstrap the Hash, HMAC and
CTR generators. That is, the Instantiate and Reseed functions need an
approved source of entropy. Both RDRAND and RDSEED are approved for
Intel chips. See SP800-90A, Section 8.6.5
(http://csrc.nist.gov/publications/nistpubs/800-90A/SP800-90A.pdf).

Jeff

^ permalink raw reply

* Re: [ANNOUNCE] /dev/random - a new approach (code for 4.11-rc1)
From: Stephan Müller @ 2017-03-18 13:31 UTC (permalink / raw)
  To: noloader; +Cc: LKML, linux-crypto
In-Reply-To: <CAH8yC8=+K76rwRfYk=92sBE0v9fPEWB=2bUQqF=tyY5C1nrvKQ@mail.gmail.com>

Am Samstag, 18. März 2017, 11:11:57 CET schrieb Jeffrey Walton:

Hi Jeffrey,

> > The design and implementation is driven by a set of goals described in [2]
> > that the LRNG completely implements. Furthermore, [2] includes a
> > comparison with RNG design suggestions such as SP800-90B, SP800-90C, and
> > AIS20/31.
> 
> A quick comment about SP800 and the hardware instructions... RDSEED is
> 2 to 5 times slower than RDRAND on Intel hardware, depending on the
> architecture and microarchitecture.

I am not sure how this statement relates to the quote above. RDSEED is the 
CBC-MACed output of the flip-flop providing the raw noise.

RDRAND is the output of the SP800-90A CTR DRBG that is seeded by the CBC-MAC 
that also feeds RDSEED. Thus, RDSEED is as fast as the noise source where 
RDRAND is a pure deterministic RNG that tries to be (re)seeded as often as 
possible.

Both instructions are totally unrelated to the SP800-90A DRBG available to the 
Linux kernel.

> AMD's implementation of RDRAND is
> orders of magnitude slower than Intel's. Testing on an Athlon 845 X4
> (Bulldozer v4) @ 3.5 GHz shows it runs between 4100 and 4500 cycles
> per byte. It works out to be about 1 MiB/s.

Please consider my speed measurements given in [1] section 3.4.6. The DRBG is 
just slightly slower than the ChaCha20 on small block sizes and twice as fast 
on larger block sizes using AES-NI on x86. As the DRBG implementation has no 
relationship to the RDRAND DRBG, I am not sure about your argument.

When I refer to hardware instructions and SP800-90A, I consider the SP800-90A 
DRBG implementation in crypto/drbg.c provided with the kernel crypto API which 
uses the raw AES/SHA cipher implementation provided by the kernel crypto API. 
Here, the implementation uses the fastest one, such as the AES-NI raw AES 
implementation on x86. Or it uses the ARM NEON SHA implementation for the 
HMAC/Hash DRBG.
> 
> While the LRNG may reach a cryptographically acceptable seed level
> much earlier then the existing /dev/random, it may not be early
> enough.

The LRNG will initialize as a DRBG during late_initcall. Thus, the DRBG is 
always present if user space calls.

However, during kernel boot, there is of course a need for earlier randomness. 
This is covered by the init DRNG documented in [1] section 2.10.

> Some components, like systemd, will ask for random numbers and
> truck-on even if they are not available. Systemd does not block or
> wait if get_random_bytes fails to produce. In the bigger picture,
> don't expect that software layered above will do the expected thing in
> all cases.

The LRNG works as a full ABI and API replacement for the current /dev/random 
implementation. I run it on my servers. It delivers random data for all use 
cases, during early kernel and user space boot as well as during runtime.

[1] http://www.chronox.de/lrng/doc/lrng.pdf
> 
> Jeff


Ciao
Stephan

^ permalink raw reply

* Re: [PATCH] crypto: zip - Memory corruption in zip_clear_stats()
From: Dan Carpenter @ 2017-03-18 10:59 UTC (permalink / raw)
  To: walter harms
  Cc: Herbert Xu, Mahipal Challa, David S. Miller, Jan Glauber,
	linux-crypto, kernel-janitors
In-Reply-To: <58CD0AE2.3070006@bfs.de>

On Sat, Mar 18, 2017 at 11:24:34AM +0100, walter harms wrote:
> 
> 
> Am 17.03.2017 21:46, schrieb Dan Carpenter:
> > There is a typo here.  It should be "stats" instead of "state".  The
> > impact is that we clear 224 bytes instead of 80 and we zero out memory
> > that we shouldn't.
> > 
> > Fixes: 09ae5d37e093 ("crypto: zip - Add Compression/Decompression statistics")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > 
> > diff --git a/drivers/crypto/cavium/zip/zip_main.c b/drivers/crypto/cavium/zip/zip_main.c
> > index 0951e20b395b..6ff13d80d82e 100644
> > --- a/drivers/crypto/cavium/zip/zip_main.c
> > +++ b/drivers/crypto/cavium/zip/zip_main.c
> > @@ -530,7 +530,7 @@ static int zip_clear_stats(struct seq_file *s, void *unused)
> >  	for (index = 0; index < MAX_ZIP_DEVICES; index++) {
> >  		if (zip_dev[index]) {
> >  			memset(&zip_dev[index]->stats, 0,
> > -			       sizeof(struct zip_state));
> > +			       sizeof(struct zip_stats));
> 
> 
> as future FIXME some show find a name that differ in more than just the last char.
> NTL maybe
>  sizeof(zip_dev[index]->stats)
> can be used here ?

That's sort of unweildy.  I don't fear that change because I'm confident
I would catch it with static analysis.

regards,
dan carpenter


^ permalink raw reply

* Re: [PATCH] crypto: zip - Memory corruption in zip_clear_stats()
From: walter harms @ 2017-03-18 10:24 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Herbert Xu, Mahipal Challa, David S. Miller, Jan Glauber,
	linux-crypto, kernel-janitors
In-Reply-To: <20170317204621.GD16505@mwanda>



Am 17.03.2017 21:46, schrieb Dan Carpenter:
> There is a typo here.  It should be "stats" instead of "state".  The
> impact is that we clear 224 bytes instead of 80 and we zero out memory
> that we shouldn't.
> 
> Fixes: 09ae5d37e093 ("crypto: zip - Add Compression/Decompression statistics")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/crypto/cavium/zip/zip_main.c b/drivers/crypto/cavium/zip/zip_main.c
> index 0951e20b395b..6ff13d80d82e 100644
> --- a/drivers/crypto/cavium/zip/zip_main.c
> +++ b/drivers/crypto/cavium/zip/zip_main.c
> @@ -530,7 +530,7 @@ static int zip_clear_stats(struct seq_file *s, void *unused)
>  	for (index = 0; index < MAX_ZIP_DEVICES; index++) {
>  		if (zip_dev[index]) {
>  			memset(&zip_dev[index]->stats, 0,
> -			       sizeof(struct zip_state));
> +			       sizeof(struct zip_stats));


as future FIXME some show find a name that differ in more than just the last char.
NTL maybe
 sizeof(zip_dev[index]->stats)
can be used here ?

re,
 wh

>  			seq_printf(s, "Cleared stats for zip %d\n", index);
>  		}
>  	}
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: [ANNOUNCE] /dev/random - a new approach (code for 4.11-rc1)
From: Jeffrey Walton @ 2017-03-18 10:11 UTC (permalink / raw)
  To: Stephan Müller; +Cc: LKML, linux-crypto
In-Reply-To: <2785457.pDyvZpZC2q@positron.chronox.de>

> The design and implementation is driven by a set of goals described in [2]
> that the LRNG completely implements. Furthermore, [2] includes a
> comparison with RNG design suggestions such as SP800-90B, SP800-90C, and
> AIS20/31.

A quick comment about SP800 and the hardware instructions... RDSEED is
2 to 5 times slower than RDRAND on Intel hardware, depending on the
architecture and microarchitecture. AMD's implementation of RDRAND is
orders of magnitude slower than Intel's. Testing on an Athlon 845 X4
(Bulldozer v4) @ 3.5 GHz shows it runs between 4100 and 4500 cycles
per byte. It works out to be about 1 MiB/s.

While the LRNG may reach a cryptographically acceptable seed level
much earlier then the existing /dev/random, it may not be early
enough. Some components, like systemd, will ask for random numbers and
truck-on even if they are not available. Systemd does not block or
wait if get_random_bytes fails to produce. In the bigger picture,
don't expect that software layered above will do the expected thing in
all cases.

Jeff

^ permalink raw reply

* Question - seeding the hw pseudo random number generator
From: Krzysztof Kozlowski @ 2017-03-18  9:25 UTC (permalink / raw)
  To: Matt Mackall, Herbert Xu, linux-crypto; +Cc: linux-arm-kernel

Hi,

I looked at Exynos Pseudo Random Nubmer Generator driver
(drivers/char/hw_random/exynos-rng.c) and noticed that it always seeds
the device with jiffies.  Then I looked at few other drivers and found
that they do not seed themself (or at least I couldn't find this).

I think the hw_random API does not provide generic infrastructure for
seeding.

What is the preferred approach for seeding a PRNG device? Use jiffies or
a fixed value?

Or maybe the interface should be abandoned in favor of crypto API?

Best regards,
Krzysztof

^ permalink raw reply

* Re: [ANNOUNCE] /dev/random - a new approach (code for 4.11-rc1)
From: Stephan Müller @ 2017-03-18  8:18 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: LKML, Linux Crypto Mailing List
In-Reply-To: <CAHmME9oopUxc3fdRERdhHk=GYJuqAonhRBhmyUQE81fPdLn9Zw@mail.gmail.com>

Am Freitag, 17. März 2017, 16:31:29 CET schrieb Jason A. Donenfeld:

Hi Jason,

> Hey Stephan,
> 
> Have you considered submitting this without so many options? For
> example -- just unconditionally using ChaCha20 instead of the
> configurable crypto API functions? And either removing the FIPS140
> compliance code, and either unconditionally including it, or just
> getting rid of it? And finally just making this a part of the kernel
> directly, instead of adding this as a standalone optional component?

Submitting that with various options removed is no problem as the core concept 
of my implementation is to be flexible to allow folks to easily add new noise 
sources or a DRNG replacement.

Yet, there are reasons for the different options:

- some folks want to use a known good and proven DRNG for post-processing 
(hence the offer for using an SP800-90A DRBG)

- some folks want a DRNG that is not tied to the kernel crypto API (hence the 
offer for the ChaCha20 DRNG)

- some folks need the FIPS continuous self test and some do not.

The idea for the solution in the LRNG code is that a user shall not be 
involved with these compile-time decisions. All options that are present are 
based on other kernel configuration options:

- if the kernel crypto API is present and the SP800-90A DRBG is compiled, then 
the LRNG uses the SP800-90A DRBG

- as a user may compile in different types of the SP800-90A DRBG, the LRNG 
will use the one that is available

- if no kernel crypto API is compiled, it uses the ChaCha20 DRNG

- if FIPS support is not compiled, the FIPS continuous test is not compiled

Thus, a user does not get in touch with all the options in the LRNG code. 
Shouldn't that be a good argument to keep these options?

I would like to add the LRNG code directly to the kernel and I can offer an 
official patch instead of such out-of-tree code. However, in the past I got 
shot down when I suggested an inclusion.

Ciao
Stephan

^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Fengguang Wu @ 2017-03-18  0:41 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Borislav Petkov, Alexander Potapenko, Peter Zijlstra,
	Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
	Matthias Kaehlcke, x86@kernel.org, open list:KERNEL BUILD + fi...,
	LKML, linux-crypto, linux-raid, kbuild-all
In-Reply-To: <CACT4Y+bybdpqB71=inx8amwr188Mb2QyBeRTDdWZ3AFyDwVX0A@mail.gmail.com>

Hi Dmitry,
 
On Fri, Mar 17, 2017 at 08:05:16PM +0100, Dmitry Vyukov wrote:
>On Fri, Mar 17, 2017 at 7:57 PM, Borislav Petkov <bp@alien8.de> wrote:
>> On Fri, Mar 17, 2017 at 07:47:33PM +0100, Dmitry Vyukov wrote:
>>> This problem is more general and is not specific to clang. It equally
>>> applies to different versions of gcc, different arches and different
>>> configs (namely, anything else than what a developer used for
>>> testing).
>>
>> I guess. We do carry a bunch of gcc workarounds along with the cc-*
>> macros in scripts/Kbuild.include.
>>
>>> A known, reasonably well working solution to this problem is
>>> a system of try bots that test patches before commit with different
>>> compilers/configs/archs. We already have such system in the form of
>>> 0-day bots. It would be useful to extend it with clang as soon as
>>> kernel builds.
>>
>> Has someone actually already talked to Fengguang about it?
>
>+Fengguang

I've actually tried clang long time ago. It quickly fails the build
for vanilla kernel. So it really depends on when the various clang
build fix patches can be accepted into mainline kernel.

Thanks,
Fengguang

^ permalink raw reply

* Re: [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses
From: hpa @ 2017-03-17 23:50 UTC (permalink / raw)
  To: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid
In-Reply-To: <20170317001520.85223-4-md@google.com>

On March 16, 2017 5:15:16 PM PDT, Michael Davidson <md@google.com> wrote:
>Suppress clang warnings about potential unaliged accesses
>to members in packed structs. This gets rid of almost 10,000
>warnings about accesses to the ring 0 stack pointer in the TSS.
>
>Signed-off-by: Michael Davidson <md@google.com>
>---
> arch/x86/Makefile | 5 +++++
> 1 file changed, 5 insertions(+)
>
>diff --git a/arch/x86/Makefile b/arch/x86/Makefile
>index 894a8d18bf97..7f21703c475d 100644
>--- a/arch/x86/Makefile
>+++ b/arch/x86/Makefile
>@@ -128,6 +128,11 @@ endif
>         KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
> endif
> 
>+ifeq ($(cc-name),clang)
>+# Suppress clang warnings about potential unaligned accesses.
>+KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
>+endif
>+
> ifdef CONFIG_X86_X32
> 	x32_ld_ok := $(call try-run,\
> 			/bin/echo -e '1: .quad 1b' | \

Why conditional on clang?
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

^ permalink raw reply

* Re: [PATCH v3 1/3] clk: meson-gxbb: expose clock CLKID_RNG0
From: Kevin Hilman @ 2017-03-17 23:05 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Heiner Kallweit, Jerome Brunet, Neil Armstrong,
	linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA, Stephen Boyd,
	Michael Turquette, linux-clk-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170317083032.GA18512-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>

Herbert Xu <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org> writes:

> On Thu, Mar 16, 2017 at 11:24:31AM -0700, Kevin Hilman wrote:
>> Hi Herbert,
>> 
>> Herbert Xu <herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org> writes:
>> 
>> > On Wed, Feb 22, 2017 at 07:55:24AM +0100, Heiner Kallweit wrote:
>> >> Expose clock CLKID_RNG0 which is needed for the HW random number generator.
>> >> 
>> >> Signed-off-by: Heiner Kallweit <hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> >
>> > All patches applied.  Thanks.
>> 
>> Actually, can you just apply [PATCH 4/4] to your tree?
>> 
>> The clock and DT patches need to go through their respective trees or
>> will otherwise have conflicts with other things going in via those
>> trees.
>
> It's too late now.  Please speak up sooner next time.  These
> patches were posted a month ago.

Sorry, I didn't realize you would be applying everything.  Also, I'm not
the original author, just the platform maintainer that noticed it and
now has to deal with the conflicts. :(

Most other driver maintainers are only applying patches that directly
apply to their subsystem and leave patches to other drivers (e.g. clk) and
platform-specific stuff (e.g. DT) to go in via their proper trees, so
that's what I was expecting to happen here too.

Kevin





--
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 1/1] ARM: dts: NSP: Add crypto (SPU) to dtsi
From: Florian Fainelli @ 2017-03-17 22:50 UTC (permalink / raw)
  To: Steve Lin, herbert, robh+dt, linux, ray.jui, scott.branden,
	jon.mason, mark.rutland
  Cc: devicetree, rob.rice, linux-kernel, bcm-kernel-feedback-list,
	linux-crypto, linux-arm-kernel
In-Reply-To: <fd877ab2-48db-a677-b539-ba5ed55e75c6@gmail.com>

On 03/06/2017 11:22 AM, Florian Fainelli wrote:
> On 02/28/2017 12:31 PM, Florian Fainelli wrote:
>> On 02/22/2017 01:22 PM, Steve Lin wrote:
>>> Adds crypto hardware (SPU) to Northstar Plus device tree file.
>>>
>>> Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>>
>> Applied, thanks!
> 
> And dropped, since there is a dependency on "ARM: dts: NSP: Add mailbox
> (PDC) to NSP" to be applied first.
> 
> Let's wait for the mailbox maintainer to chime in before I apply the
> following patches (in that order):
> 
> ARM: dts: NSP: Add mailbox (PDC) to NSP
> ARM: dts: NSP: Add crypto (SPU) to dtsi

Applied again.
-- 
Florian

^ permalink raw reply

* Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
From: H. Peter Anvin @ 2017-03-17 21:34 UTC (permalink / raw)
  To: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid
In-Reply-To: <ad94b647-4ba0-4ae9-be8f-6fe7e9406def@zytor.com>

On 03/17/17 14:32, H. Peter Anvin wrote:
> 
> NAK.  Fix your compiler, or use a wrapper script or something.  It is
> absolutely *not* acceptable to disable this since future versions of
> clang *should* support that.
> 
> That being said, it might make sense to look for a key pattern like
> "(un|not )supported" on stderr the try-run macro.  Is there really no
> -Wno- or -Werror= option to turn off this craziness?
> 

Well, guess what... I found it myself.

-W{no-,error=}ignored-optimization-argument

Either variant will make this sane.

	-hpa



^ permalink raw reply

* Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags
From: H. Peter Anvin @ 2017-03-17 21:32 UTC (permalink / raw)
  To: Michael Davidson, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li
  Cc: Alexander Potapenko, Dmitry Vyukov, Matthias Kaehlcke, x86,
	linux-kbuild, linux-kernel, linux-crypto, linux-raid
In-Reply-To: <20170317001520.85223-3-md@google.com>

On 03/16/17 17:15, Michael Davidson wrote:
> Unfortunately, while clang generates a warning about these flags
> being unsupported it still exits with a status of 0 so we have
> to explicitly disable them instead of just using a cc-option check.
> 
> Signed-off-by: Michael Davidson <md@google.com>
> ---
>  Makefile          | 2 ++
>  arch/x86/Makefile | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index b21fd0ca2946..5e97e5fc1eea 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -629,7 +629,9 @@ ARCH_AFLAGS :=
>  ARCH_CFLAGS :=
>  include arch/$(SRCARCH)/Makefile
>  
> +ifneq ($(cc-name),clang)
>  KBUILD_CFLAGS	+= $(call cc-option,-fno-delete-null-pointer-checks,)
> +endif
>  KBUILD_CFLAGS	+= $(call cc-disable-warning,frame-address,)
>  
>  ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 2d449337a360..894a8d18bf97 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -87,11 +87,13 @@ else
>          KBUILD_AFLAGS += -m64
>          KBUILD_CFLAGS += -m64
>  
> +ifneq ($(cc-name),clang)
>          # Align jump targets to 1 byte, not the default 16 bytes:
>          KBUILD_CFLAGS += -falign-jumps=1
>  
>          # Pack loops tightly as well:
>          KBUILD_CFLAGS += -falign-loops=1
> +endif
>  
>          # Don't autogenerate traditional x87 instructions
>          KBUILD_CFLAGS += $(call cc-option,-mno-80387)
> 

NAK.  Fix your compiler, or use a wrapper script or something.  It is
absolutely *not* acceptable to disable this since future versions of
clang *should* support that.

That being said, it might make sense to look for a key pattern like
"(un|not )supported" on stderr the try-run macro.  Is there really no
-Wno- or -Werror= option to turn off this craziness?

	-hpa


^ permalink raw reply

* [PATCH] crypto: zip - Memory corruption in zip_clear_stats()
From: Dan Carpenter @ 2017-03-17 20:46 UTC (permalink / raw)
  To: Herbert Xu, Mahipal Challa
  Cc: David S. Miller, Jan Glauber, linux-crypto, kernel-janitors

There is a typo here.  It should be "stats" instead of "state".  The
impact is that we clear 224 bytes instead of 80 and we zero out memory
that we shouldn't.

Fixes: 09ae5d37e093 ("crypto: zip - Add Compression/Decompression statistics")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/crypto/cavium/zip/zip_main.c b/drivers/crypto/cavium/zip/zip_main.c
index 0951e20b395b..6ff13d80d82e 100644
--- a/drivers/crypto/cavium/zip/zip_main.c
+++ b/drivers/crypto/cavium/zip/zip_main.c
@@ -530,7 +530,7 @@ static int zip_clear_stats(struct seq_file *s, void *unused)
 	for (index = 0; index < MAX_ZIP_DEVICES; index++) {
 		if (zip_dev[index]) {
 			memset(&zip_dev[index]->stats, 0,
-			       sizeof(struct zip_state));
+			       sizeof(struct zip_stats));
 			seq_printf(s, "Cleared stats for zip %d\n", index);
 		}
 	}

^ permalink raw reply related

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: hpa @ 2017-03-17 20:04 UTC (permalink / raw)
  To: Peter Zijlstra, Michael Davidson
  Cc: Alexander Potapenko, Michal Marek, Thomas Gleixner, Ingo Molnar,
	Herbert Xu, David S. Miller, Shaohua Li, Dmitry Vyukov,
	Matthias Kaehlcke, x86, linux-kbuild, LKML, linux-crypto,
	linux-raid
In-Reply-To: <20170317192746.y5ade3bymifdni2p@hirez.programming.kicks-ass.net>

On March 17, 2017 12:27:46 PM PDT, Peter Zijlstra <peterz@infradead.org> wrote:
>On Fri, Mar 17, 2017 at 11:52:01AM -0700, Michael Davidson wrote:
>> On Fri, Mar 17, 2017 at 5:44 AM, Peter Zijlstra
><peterz@infradead.org> wrote:
>> >
>> > Be that as it may; what you construct above is disgusting. Surely
>the
>> > code can be refactored to not look like dog vomit?
>> >
>> > Also; its not immediately obvious conf->copies is 'small' and this
>> > doesn't blow up the stack; I feel that deserves a comment
>somewhere.
>> >
>> 
>> I agree that the code is horrible.
>> 
>> It is, in fact, exactly the same solution that was used to remove
>> variable length arrays in structs from several of the crypto drivers
>a
>> few years ago - see the definition of SHASH_DESC_ON_STACK() in
>> "crypto/hash.h" - I did not, however, hide the horrors in a macro
>> preferring to leave the implementation visible as a warning to
>whoever
>> might touch the code next.
>> 
>> I believe that the actual stack usage is exactly the same as it was
>previously.
>> 
>> I can certainly wrap this  up in a macro and add comments with
>> appropriately dire warnings in it if you feel that is both necessary
>> and sufficient.
>
>We got away with ugly in the past, so we should get to do it again?

Seriously, you should have taken the hack the first time that this needs to be fixed.  Just because this is a fairly uncommon construct in the kernel doesn't mean it is not in userspace.

I would like to say this falls in the category of "fix your compiler this time".  Once is one thing, twice is unacceptable.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Peter Zijlstra @ 2017-03-17 19:29 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Borislav Petkov, Alexander Potapenko, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke,
	x86@kernel.org, open list:KERNEL BUILD + fi..., LKML,
	linux-crypto, linux-raid, kbuild-all, Fengguang Wu
In-Reply-To: <20170317192642.qnrf7xuopxzapl2r@hirez.programming.kicks-ass.net>

On Fri, Mar 17, 2017 at 08:26:42PM +0100, Peter Zijlstra wrote:
> On Fri, Mar 17, 2017 at 08:05:16PM +0100, Dmitry Vyukov wrote:
> > You can also find some reasons in the Why section of LLVM-Linux project:
> > http://llvm.linuxfoundation.org/index.php/Main_Page
> 
> From that:
> 
>  - LLVM/Clang is a fast moving project with many things fixed quickly
>    and features added.
> 
> So what's the deal with that 5 year old bug you want us to work around?
> 
> Also, clang doesn't support asm cc flags output and a few other
> extensions last time I checked.
> 

Another great one:

 - BSD License (some people prefer this license to the GPL)

Seems a very weak argument to make when talking about the Linux Kernel
which is very explicitly GPLv2 (and not later).

^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Peter Zijlstra @ 2017-03-17 19:27 UTC (permalink / raw)
  To: Michael Davidson
  Cc: Alexander Potapenko, Michal Marek, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
	Dmitry Vyukov, Matthias Kaehlcke, x86, linux-kbuild, LKML,
	linux-crypto, linux-raid
In-Reply-To: <CA+=D-XXy9yEQc-jLxSqRUwq=4ADnFqaxnsjSzV4=8jOisym=bQ@mail.gmail.com>

On Fri, Mar 17, 2017 at 11:52:01AM -0700, Michael Davidson wrote:
> On Fri, Mar 17, 2017 at 5:44 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > Be that as it may; what you construct above is disgusting. Surely the
> > code can be refactored to not look like dog vomit?
> >
> > Also; its not immediately obvious conf->copies is 'small' and this
> > doesn't blow up the stack; I feel that deserves a comment somewhere.
> >
> 
> I agree that the code is horrible.
> 
> It is, in fact, exactly the same solution that was used to remove
> variable length arrays in structs from several of the crypto drivers a
> few years ago - see the definition of SHASH_DESC_ON_STACK() in
> "crypto/hash.h" - I did not, however, hide the horrors in a macro
> preferring to leave the implementation visible as a warning to whoever
> might touch the code next.
> 
> I believe that the actual stack usage is exactly the same as it was previously.
> 
> I can certainly wrap this  up in a macro and add comments with
> appropriately dire warnings in it if you feel that is both necessary
> and sufficient.

We got away with ugly in the past, so we should get to do it again?

^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Peter Zijlstra @ 2017-03-17 19:26 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Borislav Petkov, Alexander Potapenko, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke,
	x86@kernel.org, open list:KERNEL BUILD + fi..., LKML,
	linux-crypto, linux-raid, kbuild-all, Fengguang Wu
In-Reply-To: <CACT4Y+bybdpqB71=inx8amwr188Mb2QyBeRTDdWZ3AFyDwVX0A@mail.gmail.com>

On Fri, Mar 17, 2017 at 08:05:16PM +0100, Dmitry Vyukov wrote:
> You can also find some reasons in the Why section of LLVM-Linux project:
> http://llvm.linuxfoundation.org/index.php/Main_Page

>From that:

 - LLVM/Clang is a fast moving project with many things fixed quickly
   and features added.

So what's the deal with that 5 year old bug you want us to work around?

Also, clang doesn't support asm cc flags output and a few other
extensions last time I checked.


^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Dmitry Vyukov @ 2017-03-17 19:05 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Alexander Potapenko, Peter Zijlstra, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke,
	x86@kernel.org, open list:KERNEL BUILD + fi..., LKML,
	linux-crypto, linux-raid, kbuild-all, Fengguang Wu
In-Reply-To: <20170317185720.5s7qa6hl233t24ag@pd.tnic>

On Fri, Mar 17, 2017 at 7:57 PM, Borislav Petkov <bp@alien8.de> wrote:
> On Fri, Mar 17, 2017 at 07:47:33PM +0100, Dmitry Vyukov wrote:
>> This problem is more general and is not specific to clang. It equally
>> applies to different versions of gcc, different arches and different
>> configs (namely, anything else than what a developer used for
>> testing).
>
> I guess. We do carry a bunch of gcc workarounds along with the cc-*
> macros in scripts/Kbuild.include.
>
>> A known, reasonably well working solution to this problem is
>> a system of try bots that test patches before commit with different
>> compilers/configs/archs. We already have such system in the form of
>> 0-day bots. It would be useful to extend it with clang as soon as
>> kernel builds.
>
> Has someone actually already talked to Fengguang about it?

+Fengguang

> Oh, and the stupid question: why the effort to build the kernel
> with clang at all? Just because or are there some actual, palpable
> advantages?

On our side it is:
 - clang make it possible to implement KMSAN (dynamic detection of
uses of uninit memory)
 - better code coverage for fuzzing
 - why simpler and faster development (e.g. we can port our user-space
hardening technologies -- CFI and SafeStack)

You can also find some reasons in the Why section of LLVM-Linux project:
http://llvm.linuxfoundation.org/index.php/Main_Page

^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Borislav Petkov @ 2017-03-17 18:57 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Alexander Potapenko, Peter Zijlstra, Michael Davidson,
	Michal Marek, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
	Herbert Xu, David S. Miller, Shaohua Li, Matthias Kaehlcke,
	x86@kernel.org, open list:KERNEL BUILD + fi..., LKML,
	linux-crypto, linux-raid, kbuild-all
In-Reply-To: <CACT4Y+Ysvjz+UzEi0dTtsNxxnG7WXTiZPr+KSk8HHwSu7vX1ew@mail.gmail.com>

On Fri, Mar 17, 2017 at 07:47:33PM +0100, Dmitry Vyukov wrote:
> This problem is more general and is not specific to clang. It equally
> applies to different versions of gcc, different arches and different
> configs (namely, anything else than what a developer used for
> testing).

I guess. We do carry a bunch of gcc workarounds along with the cc-*
macros in scripts/Kbuild.include.

> A known, reasonably well working solution to this problem is
> a system of try bots that test patches before commit with different
> compilers/configs/archs. We already have such system in the form of
> 0-day bots. It would be useful to extend it with clang as soon as
> kernel builds.

Has someone actually already talked to Fengguang about it?

Oh, and the stupid question: why the effort to build the kernel
with clang at all? Just because or are there some actual, palpable
advantages?

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

^ permalink raw reply

* Re: [PATCH 6/7] md/raid10, LLVM: get rid of variable length array
From: Michael Davidson @ 2017-03-17 18:52 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Alexander Potapenko, Michal Marek, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Herbert Xu, David S. Miller, Shaohua Li,
	Dmitry Vyukov, Matthias Kaehlcke, x86, linux-kbuild, LKML,
	linux-crypto, linux-raid
In-Reply-To: <20170317124404.mt3jd5q5vyk63q2w@hirez.programming.kicks-ass.net>

On Fri, Mar 17, 2017 at 5:44 AM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> Be that as it may; what you construct above is disgusting. Surely the
> code can be refactored to not look like dog vomit?
>
> Also; its not immediately obvious conf->copies is 'small' and this
> doesn't blow up the stack; I feel that deserves a comment somewhere.
>

I agree that the code is horrible.

It is, in fact, exactly the same solution that was used to remove
variable length arrays in structs from several of the crypto drivers a
few years ago - see the definition of SHASH_DESC_ON_STACK() in
"crypto/hash.h" - I did not, however, hide the horrors in a macro
preferring to leave the implementation visible as a warning to whoever
might touch the code next.

I believe that the actual stack usage is exactly the same as it was previously.

I can certainly wrap this  up in a macro and add comments with
appropriately dire warnings in it if you feel that is both necessary
and sufficient.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox