We're interested in getting rid of all of the stack allocated arrays in the
kernel [1]. This patch simply hardcodes the iv length to match that of the
hardcoded cipher.
[1]: https://lkml.org/lkml/2018/3/7/621
v2: hardcode the length of the nonce to be the GCM AES IV length, and do a
sanity check in init(), Eric Biggers
Signed-off-by: Tycho Andersen <[email protected]>
CC: David Howells <[email protected]>
CC: James Morris <[email protected]>
CC: "Serge E. Hallyn" <[email protected]>
CC: Jason A. Donenfeld <[email protected]>
CC: Eric Biggers <[email protected]>
---
security/keys/big_key.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index 933623784ccd..75c46786a166 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -22,6 +22,7 @@
#include <keys/user-type.h>
#include <keys/big_key-type.h>
#include <crypto/aead.h>
+#include <crypto/gcm.h>
struct big_key_buf {
unsigned int nr_pages;
@@ -109,7 +110,7 @@ static int big_key_crypt(enum big_key_op op, struct big_key_buf *buf, size_t dat
* an .update function, so there's no chance we'll wind up reusing the
* key to encrypt updated data. Simply put: one key, one encryption.
*/
- u8 zero_nonce[crypto_aead_ivsize(big_key_aead)];
+ u8 zero_nonce[GCM_AES_IV_SIZE];
aead_req = aead_request_alloc(big_key_aead, GFP_KERNEL);
if (!aead_req)
@@ -425,6 +426,12 @@ static int __init big_key_init(void)
pr_err("Can't alloc crypto: %d\n", ret);
return ret;
}
+
+ if (unlikely(crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE)) {
+ WARN(1, "big key algorithm changed?");
+ return -EINVAL;
+ }
+
ret = crypto_aead_setauthsize(big_key_aead, ENC_AUTHTAG_SIZE);
if (ret < 0) {
pr_err("Can't set crypto auth tag len: %d\n", ret);
--
2.17.0
We're interested in getting rid of all of the stack allocated arrays in the
kernel: https://lkml.org/lkml/2018/3/7/621
This particular vla is used as a temporary output buffer in case there is
too much hash output for the destination buffer. Instead, let's just
allocate a buffer that's big enough initially, but only copy back to
userspace the amount that was originally asked for.
v2: allocate enough in the original output buffer vs creating a temporary
output buffer
Signed-off-by: Tycho Andersen <[email protected]>
CC: David Howells <[email protected]>
CC: James Morris <[email protected]>
CC: "Serge E. Hallyn" <[email protected]>
CC: Eric Biggers <[email protected]>
---
security/keys/dh.c | 29 ++++++++++-------------------
1 file changed, 10 insertions(+), 19 deletions(-)
diff --git a/security/keys/dh.c b/security/keys/dh.c
index d1ea9f325f94..9fecaea6c298 100644
--- a/security/keys/dh.c
+++ b/security/keys/dh.c
@@ -183,24 +183,13 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
goto err;
}
- if (dlen < h) {
- u8 tmpbuffer[h];
-
- err = crypto_shash_final(desc, tmpbuffer);
- if (err)
- goto err;
- memcpy(dst, tmpbuffer, dlen);
- memzero_explicit(tmpbuffer, h);
- return 0;
- } else {
- err = crypto_shash_final(desc, dst);
- if (err)
- goto err;
+ err = crypto_shash_final(desc, dst);
+ if (err)
+ goto err;
- dlen -= h;
- dst += h;
- counter = cpu_to_be32(be32_to_cpu(counter) + 1);
- }
+ dlen -= h;
+ dst += h;
+ counter = cpu_to_be32(be32_to_cpu(counter) + 1);
}
return 0;
@@ -216,14 +205,16 @@ static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
{
uint8_t *outbuf = NULL;
int ret;
+ size_t outbuf_len = round_up(buflen,
+ crypto_shash_digestsize(sdesc->shash.tfm));
- outbuf = kmalloc(buflen, GFP_KERNEL);
+ outbuf = kmalloc(outbuf_len, GFP_KERNEL);
if (!outbuf) {
ret = -ENOMEM;
goto err;
}
- ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, buflen, lzero);
+ ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len, lzero);
if (ret)
goto err;
--
2.17.0
We're interested in getting rid of all of the stack allocated arrays in
the kernel: https://lkml.org/lkml/2018/3/7/621
This case is interesting, since we really just need an array of bytes that
are zero. The loop already ensures that if the array isn't exactly the
right size that enough zero bytes will be copied in. So, instead of
choosing this value to be the size of the hash, let's just choose it to be
256, since that is a common size, is not to big, and will not result in too
many extra iterations of the loop.
v2: split out from other patch, just hardcode array size instead of
dynamically allocating something the right size
Signed-off-by: Tycho Andersen <[email protected]>
CC: David Howells <[email protected]>
CC: James Morris <[email protected]>
CC: "Serge E. Hallyn" <[email protected]>
CC: Eric Biggers <[email protected]>
---
security/keys/dh.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/security/keys/dh.c b/security/keys/dh.c
index 9fecaea6c298..74f8a853872e 100644
--- a/security/keys/dh.c
+++ b/security/keys/dh.c
@@ -162,8 +162,8 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
goto err;
if (zlen && h) {
- u8 tmpbuffer[h];
- size_t chunk = min_t(size_t, zlen, h);
+ u8 tmpbuffer[256];
+ size_t chunk = min_t(size_t, zlen, sizeof(tmpbuffer));
memset(tmpbuffer, 0, chunk);
do {
@@ -173,7 +173,7 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
goto err;
zlen -= chunk;
- chunk = min_t(size_t, zlen, h);
+ chunk = min_t(size_t, zlen, sizeof(tmpbuffer));
} while (zlen);
}
--
2.17.0
On Mon, Apr 23, 2018 at 07:03:21PM -0600, Tycho Andersen wrote:
> We're interested in getting rid of all of the stack allocated arrays in
> the kernel: https://lkml.org/lkml/2018/3/7/621
>
> This case is interesting, since we really just need an array of bytes that
> are zero. The loop already ensures that if the array isn't exactly the
> right size that enough zero bytes will be copied in. So, instead of
> choosing this value to be the size of the hash, let's just choose it to be
> 256, since that is a common size, is not to big, and will not result in too
> many extra iterations of the loop.
>
> v2: split out from other patch, just hardcode array size instead of
> dynamically allocating something the right size
>
> Signed-off-by: Tycho Andersen <[email protected]>
> CC: David Howells <[email protected]>
> CC: James Morris <[email protected]>
> CC: "Serge E. Hallyn" <[email protected]>
> CC: Eric Biggers <[email protected]>
> ---
> security/keys/dh.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/security/keys/dh.c b/security/keys/dh.c
> index 9fecaea6c298..74f8a853872e 100644
> --- a/security/keys/dh.c
> +++ b/security/keys/dh.c
> @@ -162,8 +162,8 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
> goto err;
>
> if (zlen && h) {
> - u8 tmpbuffer[h];
> - size_t chunk = min_t(size_t, zlen, h);
> + u8 tmpbuffer[256];
Whoops, this should be 32, not 256. That shouldn't make any runtime
difference, but it'll closer match the allocation patterns from
before.
I'll let this sit for a bit and send v3.
Tycho
Hi Tycho,
On Mon, Apr 23, 2018 at 07:03:19PM -0600, Tycho Andersen wrote:
> We're interested in getting rid of all of the stack allocated arrays in the
> kernel [1]. This patch simply hardcodes the iv length to match that of the
> hardcoded cipher.
>
> [1]: https://lkml.org/lkml/2018/3/7/621
>
> v2: hardcode the length of the nonce to be the GCM AES IV length, and do a
> sanity check in init(), Eric Biggers
>
> Signed-off-by: Tycho Andersen <[email protected]>
> CC: David Howells <[email protected]>
> CC: James Morris <[email protected]>
> CC: "Serge E. Hallyn" <[email protected]>
> CC: Jason A. Donenfeld <[email protected]>
> CC: Eric Biggers <[email protected]>
> ---
> security/keys/big_key.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/security/keys/big_key.c b/security/keys/big_key.c
> index 933623784ccd..75c46786a166 100644
> --- a/security/keys/big_key.c
> +++ b/security/keys/big_key.c
> @@ -22,6 +22,7 @@
> #include <keys/user-type.h>
> #include <keys/big_key-type.h>
> #include <crypto/aead.h>
> +#include <crypto/gcm.h>
>
> struct big_key_buf {
> unsigned int nr_pages;
> @@ -109,7 +110,7 @@ static int big_key_crypt(enum big_key_op op, struct big_key_buf *buf, size_t dat
> * an .update function, so there's no chance we'll wind up reusing the
> * key to encrypt updated data. Simply put: one key, one encryption.
> */
> - u8 zero_nonce[crypto_aead_ivsize(big_key_aead)];
> + u8 zero_nonce[GCM_AES_IV_SIZE];
>
> aead_req = aead_request_alloc(big_key_aead, GFP_KERNEL);
> if (!aead_req)
> @@ -425,6 +426,12 @@ static int __init big_key_init(void)
> pr_err("Can't alloc crypto: %d\n", ret);
> return ret;
> }
> +
> + if (unlikely(crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE)) {
> + WARN(1, "big key algorithm changed?");
> + return -EINVAL;
> + }
> +
'big_key_aead' needs to be freed on error.
err = -EINVAL;
goto free_aead;
Also how about defining the IV size next to the algorithm name?
Then all the algorithm details would be on adjacent lines:
static const char big_key_alg_name[] = "gcm(aes)";
#define BIG_KEY_IV_SIZE GCM_AES_IV_SIZE
- Eric
Hi Eric,
On Mon, Apr 23, 2018 at 09:50:15PM -0700, Eric Biggers wrote:
> Hi Tycho,
>
> On Mon, Apr 23, 2018 at 07:03:19PM -0600, Tycho Andersen wrote:
> > We're interested in getting rid of all of the stack allocated arrays in the
> > kernel [1]. This patch simply hardcodes the iv length to match that of the
> > hardcoded cipher.
> >
> > [1]: https://lkml.org/lkml/2018/3/7/621
> >
> > v2: hardcode the length of the nonce to be the GCM AES IV length, and do a
> > sanity check in init(), Eric Biggers
> >
> > Signed-off-by: Tycho Andersen <[email protected]>
> > CC: David Howells <[email protected]>
> > CC: James Morris <[email protected]>
> > CC: "Serge E. Hallyn" <[email protected]>
> > CC: Jason A. Donenfeld <[email protected]>
> > CC: Eric Biggers <[email protected]>
> > ---
> > security/keys/big_key.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/security/keys/big_key.c b/security/keys/big_key.c
> > index 933623784ccd..75c46786a166 100644
> > --- a/security/keys/big_key.c
> > +++ b/security/keys/big_key.c
> > @@ -22,6 +22,7 @@
> > #include <keys/user-type.h>
> > #include <keys/big_key-type.h>
> > #include <crypto/aead.h>
> > +#include <crypto/gcm.h>
> >
> > struct big_key_buf {
> > unsigned int nr_pages;
> > @@ -109,7 +110,7 @@ static int big_key_crypt(enum big_key_op op, struct big_key_buf *buf, size_t dat
> > * an .update function, so there's no chance we'll wind up reusing the
> > * key to encrypt updated data. Simply put: one key, one encryption.
> > */
> > - u8 zero_nonce[crypto_aead_ivsize(big_key_aead)];
> > + u8 zero_nonce[GCM_AES_IV_SIZE];
> >
> > aead_req = aead_request_alloc(big_key_aead, GFP_KERNEL);
> > if (!aead_req)
> > @@ -425,6 +426,12 @@ static int __init big_key_init(void)
> > pr_err("Can't alloc crypto: %d\n", ret);
> > return ret;
> > }
> > +
> > + if (unlikely(crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE)) {
> > + WARN(1, "big key algorithm changed?");
> > + return -EINVAL;
> > + }
> > +
>
> 'big_key_aead' needs to be freed on error.
>
> err = -EINVAL;
> goto free_aead;
oof, yes, thanks.
> Also how about defining the IV size next to the algorithm name?
> Then all the algorithm details would be on adjacent lines:
>
> static const char big_key_alg_name[] = "gcm(aes)";
> #define BIG_KEY_IV_SIZE GCM_AES_IV_SIZE
Sounds good, I'll fix both of these for v3.
Cheers,
Tycho
Tycho Andersen wrote:
> > > + if (unlikely(crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE)) {
> > > + WARN(1, "big key algorithm changed?");
Please avoid using WARN() WARN_ON() etc.
syzbot would catch it and panic() due to panic_on_warn == 1.
On Tue, Apr 24, 2018 at 11:46:38PM +0900, Tetsuo Handa wrote:
> Tycho Andersen wrote:
> > > > + if (unlikely(crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE)) {
> > > > + WARN(1, "big key algorithm changed?");
>
> Please avoid using WARN() WARN_ON() etc.
> syzbot would catch it and panic() due to panic_on_warn == 1.
But it is really a programming bug in this case (and it seems better
than BUG()...). Isn't this exactly the sort of case we want to catch?
Tycho
Quoting Tycho Andersen ([email protected]):
> On Tue, Apr 24, 2018 at 11:46:38PM +0900, Tetsuo Handa wrote:
> > Tycho Andersen wrote:
> > > > > + if (unlikely(crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE)) {
> > > > > + WARN(1, "big key algorithm changed?");
> >
> > Please avoid using WARN() WARN_ON() etc.
> > syzbot would catch it and panic() due to panic_on_warn == 1.
>
> But it is really a programming bug in this case (and it seems better
> than BUG()...). Isn't this exactly the sort of case we want to catch?
>
> Tycho
Right - is there a url to some discussion about this? Because not
using WARN when WARN should be used, because it troubles a bot, seems
the wrong solution. If this *is* what's been agreed upon, then
what is the new recommended thing to do here?
-serge
On Tue, Apr 24, 2018 at 12:58 PM, Serge E. Hallyn <[email protected]> wrote:
> Quoting Tycho Andersen ([email protected]):
>> On Tue, Apr 24, 2018 at 11:46:38PM +0900, Tetsuo Handa wrote:
>> > Tycho Andersen wrote:
>> > > > > + if (unlikely(crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE)) {
>> > > > > + WARN(1, "big key algorithm changed?");
>> >
>> > Please avoid using WARN() WARN_ON() etc.
>> > syzbot would catch it and panic() due to panic_on_warn == 1.
>>
>> But it is really a programming bug in this case (and it seems better
>> than BUG()...). Isn't this exactly the sort of case we want to catch?
>>
>> Tycho
>
> Right - is there a url to some discussion about this? Because not
> using WARN when WARN should be used, because it troubles a bot, seems
> the wrong solution. If this *is* what's been agreed upon, then
> what is the new recommended thing to do here?
BUG() is basically supposed to never be used, as decreed by Linus.
WARN() here is entirely correct: if we encounter a case where
crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE is not true, we
run the risk of stack memory corruption. If this is an EXPECTED
failure case, then okay, drop the WARN() but we have to keep the
-EINVAL.
-Kees
--
Kees Cook
Pixel Security
On Tue, Apr 24, 2018 at 02:58:45PM -0500, Serge E. Hallyn wrote:
> Quoting Tycho Andersen ([email protected]):
> > On Tue, Apr 24, 2018 at 11:46:38PM +0900, Tetsuo Handa wrote:
> > > Tycho Andersen wrote:
> > > > > > + if (unlikely(crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE)) {
> > > > > > + WARN(1, "big key algorithm changed?");
> > >
> > > Please avoid using WARN() WARN_ON() etc.
> > > syzbot would catch it and panic() due to panic_on_warn == 1.
> >
> > But it is really a programming bug in this case (and it seems better
> > than BUG()...). Isn't this exactly the sort of case we want to catch?
> >
> > Tycho
>
> Right - is there a url to some discussion about this? Because not
> using WARN when WARN should be used, because it troubles a bot, seems
> the wrong solution. If this *is* what's been agreed upon, then
> what is the new recommended thing to do here?
>
> -serge
WARN() is for recoverable kernel bugs, which this is, so WARN() is correct here.
Fuzzers often find cases where WARN() is used on invalid user input or other
cases that are not kernel bugs, and then it has to be removed or replaced with
pr_warn(). But here it is appropriate. Unfortunately a lot of developers still
seem confused; improving the comments in include/asm-generic/bug.h might help.
Eric
Kees Cook wrote:
> On Tue, Apr 24, 2018 at 12:58 PM, Serge E. Hallyn <[email protected]> wrote:
> > Quoting Tycho Andersen ([email protected]):
> >> On Tue, Apr 24, 2018 at 11:46:38PM +0900, Tetsuo Handa wrote:
> >> > Tycho Andersen wrote:
> >> > > > > + if (unlikely(crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE)) {
> >> > > > > + WARN(1, "big key algorithm changed?");
> >> >
> >> > Please avoid using WARN() WARN_ON() etc.
> >> > syzbot would catch it and panic() due to panic_on_warn == 1.
> >>
> >> But it is really a programming bug in this case (and it seems better
> >> than BUG()...). Isn't this exactly the sort of case we want to catch?
> >>
> >> Tycho
> >
> > Right - is there a url to some discussion about this? Because not
> > using WARN when WARN should be used, because it troubles a bot, seems
> > the wrong solution. If this *is* what's been agreed upon, then
> > what is the new recommended thing to do here?
>
> BUG() is basically supposed to never be used, as decreed by Linus.
> WARN() here is entirely correct: if we encounter a case where
> crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE is not true, we
> run the risk of stack memory corruption. If this is an EXPECTED
> failure case, then okay, drop the WARN() but we have to keep the
> -EINVAL.
big_key_init() is __init function of built-in module which will be called
only once upon boot, isn't it? Then, there is no point to continue after
WARN(); BUG() is better here.
Moreover, if this is meant for sanity check in case something went wrong
(e.g. memory corruption), it is better to check at run time like
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index 9336237..bca04f2 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -22,6 +22,7 @@
#include <keys/user-type.h>
#include <keys/big_key-type.h>
#include <crypto/aead.h>
+#include <crypto/gcm.h>
struct big_key_buf {
unsigned int nr_pages;
@@ -109,7 +110,12 @@ static int big_key_crypt(enum big_key_op op, struct big_key_buf *buf, size_t dat
* an .update function, so there's no chance we'll wind up reusing the
* key to encrypt updated data. Simply put: one key, one encryption.
*/
- u8 zero_nonce[crypto_aead_ivsize(big_key_aead)];
+ u8 zero_nonce[GCM_AES_IV_SIZE];
+
+ if (crypto_aead_ivsize(big_key_aead) != sizeof(zero_nonce)) {
+ pr_err("big key algorithm changed?");
+ return -EINVAL;
+ }
aead_req = aead_request_alloc(big_key_aead, GFP_KERNEL);
if (!aead_req)
because crypto_aead_ivsize(big_key_aead) == GCM_AES_IV_SIZE is true
unless something goes wrong at run time, isn't it?
Moreover, zero_nonce[] can be "static" if all actions after memory allocation
are guarded by global big_key_aead_lock mutex?
diff --git a/security/keys/big_key.c b/security/keys/big_key.c
index 9336237..1e7d2d1 100644
--- a/security/keys/big_key.c
+++ b/security/keys/big_key.c
@@ -22,6 +22,7 @@
#include <keys/user-type.h>
#include <keys/big_key-type.h>
#include <crypto/aead.h>
+#include <crypto/gcm.h>
struct big_key_buf {
unsigned int nr_pages;
@@ -109,27 +110,28 @@ static int big_key_crypt(enum big_key_op op, struct big_key_buf *buf, size_t dat
* an .update function, so there's no chance we'll wind up reusing the
* key to encrypt updated data. Simply put: one key, one encryption.
*/
- u8 zero_nonce[crypto_aead_ivsize(big_key_aead)];
+ static u8 zero_nonce[GCM_AES_IV_SIZE];
+
+ if (crypto_aead_ivsize(big_key_aead) != sizeof(zero_nonce)) {
+ pr_err("big key algorithm changed?");
+ return -EINVAL;
+ }
aead_req = aead_request_alloc(big_key_aead, GFP_KERNEL);
if (!aead_req)
return -ENOMEM;
+ mutex_lock(&big_key_aead_lock);
memset(zero_nonce, 0, sizeof(zero_nonce));
aead_request_set_crypt(aead_req, buf->sg, buf->sg, datalen, zero_nonce);
aead_request_set_callback(aead_req, CRYPTO_TFM_REQ_MAY_SLEEP, NULL, NULL);
aead_request_set_ad(aead_req, 0);
-
- mutex_lock(&big_key_aead_lock);
- if (crypto_aead_setkey(big_key_aead, key, ENC_KEY_SIZE)) {
+ if (crypto_aead_setkey(big_key_aead, key, ENC_KEY_SIZE))
ret = -EAGAIN;
- goto error;
- }
- if (op == BIG_KEY_ENC)
+ else if (op == BIG_KEY_ENC)
ret = crypto_aead_encrypt(aead_req);
else
ret = crypto_aead_decrypt(aead_req);
-error:
mutex_unlock(&big_key_aead_lock);
aead_request_free(aead_req);
return ret;
On Wed, Apr 25, 2018 at 07:36:21PM +0900, Tetsuo Handa wrote:
> Kees Cook wrote:
> > On Tue, Apr 24, 2018 at 12:58 PM, Serge E. Hallyn <[email protected]> wrote:
> > > Quoting Tycho Andersen ([email protected]):
> > >> On Tue, Apr 24, 2018 at 11:46:38PM +0900, Tetsuo Handa wrote:
> > >> > Tycho Andersen wrote:
> > >> > > > > + if (unlikely(crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE)) {
> > >> > > > > + WARN(1, "big key algorithm changed?");
> > >> >
> > >> > Please avoid using WARN() WARN_ON() etc.
> > >> > syzbot would catch it and panic() due to panic_on_warn == 1.
> > >>
> > >> But it is really a programming bug in this case (and it seems better
> > >> than BUG()...). Isn't this exactly the sort of case we want to catch?
> > >>
> > >> Tycho
> > >
> > > Right - is there a url to some discussion about this? Because not
> > > using WARN when WARN should be used, because it troubles a bot, seems
> > > the wrong solution. If this *is* what's been agreed upon, then
> > > what is the new recommended thing to do here?
> >
> > BUG() is basically supposed to never be used, as decreed by Linus.
> > WARN() here is entirely correct: if we encounter a case where
> > crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE is not true, we
> > run the risk of stack memory corruption. If this is an EXPECTED
> > failure case, then okay, drop the WARN() but we have to keep the
> > -EINVAL.
>
> big_key_init() is __init function of built-in module which will be called
> only once upon boot, isn't it? Then, there is no point to continue after
> WARN(); BUG() is better here.
I don't think so. The machine can still boot and work just fine, but
big key crypto will not be available. I suspect there are some
machines out there that don't need big key, so there's no reason for
the boot to fail. That's the rub about WARN vs BUG -- that in most
cases things can continue on happily.
> Moreover, if this is meant for sanity check in case something went wrong
> (e.g. memory corruption), it is better to check at run time like
But the algorithm is hard coded at the top of the file, so one check
is enough.
Tycho