On Fri, May 16, 2014 at 10:18:40PM -0400, Theodore Ts'o wrote:
> On Fri, May 16, 2014 at 05:46:22PM -0700, Hannes Frederic Sowa wrote:
> > This should do the trick:
> > dd if=/dev/urandom of=/dev/zero bs=67108707
> >
> > I suspect ee1de406ba6eb1 ("random: simplify accounting logic") as the
> > culprit.
>
> Yep, that it's it. Thanks for noticing this so quickly! I'll push
> the following patch to Linus.
>
> - Ted
>
> commit 29fb0ca5b3922288fba3f4c975a55032a51df0f0
> Author: Theodore Ts'o <[email protected]>
> Date: Fri May 16 21:40:41 2014 -0400
>
> random: fix BUG_ON caused by accounting simplification
I just hit this bug again on 3.16rc5, which has this patch, so
there's still some corner case that isn't happy..
(old thread is here for reference: https://lkml.org/lkml/2014/5/16/724)
kernel BUG at drivers/char/random.c:986!
invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
CPU: 2 PID: 29845 Comm: trinity-c65 Not tainted 3.16.0-rc5+ #22
task: ffff880096f45880 ti: ffff88017dfe8000 task.ti: ffff88017dfe8000
RIP: 0010:[<ffffffff8d420c2c>] [<ffffffff8d420c2c>] account+0x14c/0x150
RSP: 0018:ffff88017dfebd38 EFLAGS: 00010206
RAX: 0000000000002000 RBX: 000000000027ae6c RCX: 0000000000000000
RDX: 0000000000000000 RSI: 000000000027ae6c RDI: ffffffff8dcb6840
RBP: ffff88017dfebd98 R08: ffffffff8d423490 R09: 0000000000000000
R10: ffffffff8dcb68a4 R11: 0000000000000000 R12: 00007fe396cec000
R13: 0000000000000000 R14: ffffffff8dcb6840 R15: ffffffff8da85638
FS: 00007fe39a1e2700(0000) GS:ffff88024d100000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000000000 CR3: 000000012e5a3000 CR4: 00000000001407e0
DR0: 000000000111b000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000600
Stack:
ffffffff8d423340 00000000ffffffff 00000000a4c39e97 ffffffff8dcb6840
0000000000000002 0000000000000000 00000000a4c39e97 00007fe396cec000
000000000027ae6c 000000000000005a 000000000027ae6c ffffffff8d423490
Call Trace:
[<ffffffff8d423340>] ? extract_entropy_user+0x60/0x1b0
[<ffffffff8d423490>] ? extract_entropy_user+0x1b0/0x1b0
[<ffffffff8d4234c7>] urandom_read+0x37/0x110
[<ffffffff8d423490>] ? extract_entropy_user+0x1b0/0x1b0
[<ffffffff8d1d06f3>] do_loop_readv_writev+0x63/0x90
[<ffffffff8d423490>] ? extract_entropy_user+0x1b0/0x1b0
[<ffffffff8d1d2588>] do_readv_writev+0x268/0x280
[<ffffffff8d11d4eb>] ? __acct_update_integrals+0x8b/0x120
[<ffffffff8d0a935b>] ? preempt_count_sub+0xab/0x100
[<ffffffff8d343d53>] ? __this_cpu_preempt_check+0x13/0x20
[<ffffffff8d1d25d6>] vfs_readv+0x36/0x50
[<ffffffff8d1d269c>] SyS_readv+0x5c/0x100
[<ffffffff8d74ff9f>] tracesys+0xdd/0xe2
Code: cb 8d e8 d8 e0 c9 ff ba 02 00 02 00 be 1d 00 00 00 48 c7 c7 40 20 00 8e e8 d2 45 dc ff 48 89 d8 5b 41 5c 41 5d 41 5e 41 5f 5d c3 <0f> 0b 66 90 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 49 89 fe 41
RIP [<ffffffff8d420c2c>] account+0x14c/0x150
RSP <ffff88017dfebd38>
On Di, 2014-07-15 at 00:36 -0400, Dave Jones wrote:
> On Fri, May 16, 2014 at 10:18:40PM -0400, Theodore Ts'o wrote:
> > On Fri, May 16, 2014 at 05:46:22PM -0700, Hannes Frederic Sowa wrote:
> > > This should do the trick:
> > > dd if=/dev/urandom of=/dev/zero bs=67108707
> > >
> > > I suspect ee1de406ba6eb1 ("random: simplify accounting logic") as the
> > > culprit.
> >
> > Yep, that it's it. Thanks for noticing this so quickly! I'll push
> > the following patch to Linus.
> >
> > - Ted
> >
> > commit 29fb0ca5b3922288fba3f4c975a55032a51df0f0
> > Author: Theodore Ts'o <[email protected]>
> > Date: Fri May 16 21:40:41 2014 -0400
> >
> > random: fix BUG_ON caused by accounting simplification
>
> I just hit this bug again on 3.16rc5, which has this patch, so
> there's still some corner case that isn't happy..
>
> (old thread is here for reference: https://lkml.org/lkml/2014/5/16/724)
Looks like an overflow introduced by e33ba5fa7afce1a ("random: fix nasty
entropy accounting bug"). Ted, what do you think about the following
fix?
Bye,
Hannes
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 0a7ac0a..72511e8 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -999,7 +999,9 @@ retry:
}
if (ibytes < min)
ibytes = 0;
- if ((entropy_count -= ibytes << (ENTROPY_SHIFT + 3)) < 0)
+
+ if ((entropy_count -= ibytes << (ENTROPY_SHIFT + 3)) < 0 ||
+ entropy_count > orig)
entropy_count = 0;
if (cmpxchg(&r->entropy_count, orig, entropy_count) != orig)
On Tue, Jul 15, 2014 at 10:29:10PM +0200, Hannes Frederic Sowa wrote:
>
> Looks like an overflow introduced by e33ba5fa7afce1a ("random: fix nasty
> entropy accounting bug"). Ted, what do you think about the following
> fix?
Ah, right. We're only capping ibytes to be no more entropy in the
pool only in the r->limit case.
LGTM. Can you send a patch with a description with a description and
a Signed-off-by?
- Ted
The expression entropy_count -= ibytes << (ENTROPY_SHIFT + 3) could
actually increase entropy_count if during assignment of the unsigned
expression on the RHS (mind the -=) we reduce the value modulo
2^width(int) and assign it to entropy_count. Trinity found this.
Reported-by: Dave Jones <[email protected]>
Cc: Theodore Ts'o <[email protected]>
Cc: Greg Price <[email protected]>
Signed-off-by: Hannes Frederic Sowa <[email protected]>
---
As indicated by credit_entropy_bits entropy_count cannot get negative,
so I don't see any reason to include a check for entropy_count < 0
here. Do you agree?
drivers/char/random.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 0a7ac0a..cd50c4e 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -981,7 +981,7 @@ static size_t account(struct entropy_store *r, size_t nbytes, int min,
int reserved)
{
int entropy_count, orig;
- size_t ibytes;
+ size_t ibytes, nfrac;
BUG_ON(r->entropy_count > r->poolinfo->poolfracbits);
@@ -999,7 +999,11 @@ retry:
}
if (ibytes < min)
ibytes = 0;
- if ((entropy_count -= ibytes << (ENTROPY_SHIFT + 3)) < 0)
+
+ nfrac = ibytes << (ENTROPY_SHIFT + 3);
+ if (entropy_count > nfrac)
+ entropy_count -= nfrac;
+ else
entropy_count = 0;
if (cmpxchg(&r->entropy_count, orig, entropy_count) != orig)
--
1.9.3
On Wed, Jul 16, 2014 at 09:18:15PM +0200, Hannes Frederic Sowa wrote:
> The expression entropy_count -= ibytes << (ENTROPY_SHIFT + 3) could
> actually increase entropy_count if during assignment of the unsigned
> expression on the RHS (mind the -=) we reduce the value modulo
> 2^width(int) and assign it to entropy_count. Trinity found this.
>
> Reported-by: Dave Jones <[email protected]>
> Cc: Theodore Ts'o <[email protected]>
> Cc: Greg Price <[email protected]>
> Signed-off-by: Hannes Frederic Sowa <[email protected]>
> ---
> As indicated by credit_entropy_bits entropy_count cannot get negative,
> so I don't see any reason to include a check for entropy_count < 0
> here. Do you agree?
No, the check is important; after we subtract ibytes << (ENTROPY_SHIFT
+ 3) we could drive entropy_count negative, and we don't want to
trigger the WARN_ON().
I'll modify the patch to keep the check.
- Ted
Hi,
On Fri, Jul 18, 2014, at 23:25, Theodore Ts'o wrote:
> On Wed, Jul 16, 2014 at 09:18:15PM +0200, Hannes Frederic Sowa wrote:
> > The expression entropy_count -= ibytes << (ENTROPY_SHIFT + 3) could
> > actually increase entropy_count if during assignment of the unsigned
> > expression on the RHS (mind the -=) we reduce the value modulo
> > 2^width(int) and assign it to entropy_count. Trinity found this.
> >
> > Reported-by: Dave Jones <[email protected]>
> > Cc: Theodore Ts'o <[email protected]>
> > Cc: Greg Price <[email protected]>
> > Signed-off-by: Hannes Frederic Sowa <[email protected]>
> > ---
> > As indicated by credit_entropy_bits entropy_count cannot get negative,
> > so I don't see any reason to include a check for entropy_count < 0
> > here. Do you agree?
>
> No, the check is important; after we subtract ibytes << (ENTROPY_SHIFT
> + 3) we could drive entropy_count negative, and we don't want to
> trigger the WARN_ON().
If we always enter account() with r->entropy_count > 0 I think the
checks in place after this patch should suffice. The problem should only
arise in case entropy_count < 0 at the beginning of account (or in
between if we need to do a retry).
I didn't find a way how entropy_count could get smaller than 0 without
hitting a WARN_ON and a reset of r->entropy_count = 0.
Thanks,
Hannes
On Fri, Jul 18, 2014 at 05:25:04PM -0400, Theodore Ts'o wrote:
> > As indicated by credit_entropy_bits entropy_count cannot get negative,
> > so I don't see any reason to include a check for entropy_count < 0
> > here. Do you agree?
>
> No, the check is important; after we subtract ibytes << (ENTROPY_SHIFT
> + 3) we could drive entropy_count negative, and we don't want to
> trigger the WARN_ON().
>
> I'll modify the patch to keep the check.
Never mind, I took a closer look at the your patch, and I now
understand what you were asking. Since entropy_count should never
_start_ negative, simply checking to see if entropy_count > nfrac is
sufficient.
However, there's something a bit larger hiding here, which is we
shouldn't allow urandom_read to be passed a which is greater than
INT_MAX >> ENTROPY_SHIFT. Otherwise, the nfrac calcuation will
overflow, which can also result in too little entropy getting removed.
The other problem is that comparing since entropy_count is an int, and
nfrac is a size_t, this is a signed vs. unsigned comparison, which
will raise compiler warnings.
Let me know what you think of my revised patch, which should hopefully
add enough checks to be sufficiently paranoid. :-)
- Ted
From: Hannes Frederic Sowa <[email protected]>
The expression entropy_count -= ibytes << (ENTROPY_SHIFT + 3) could
actually increase entropy_count if during assignment of the unsigned
expression on the RHS (mind the -=) we reduce the value modulo
2^width(int) and assign it to entropy_count. Trinity found this.
[ Commit modified by tytso to add an additional safety check for a
negative entropy_count -- which should never happen, and to also add
an additional paranoia check to prevent overly large count values to
be passed into urandom_read(). ]
Reported-by: Dave Jones <[email protected]>
Cc: Greg Price <[email protected]>
Signed-off-by: Hannes Frederic Sowa <[email protected]>
Signed-off-by: Theodore Ts'o <[email protected]>
---
drivers/char/random.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 0a7ac0a..003f744 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -641,7 +641,7 @@ retry:
} while (unlikely(entropy_count < pool_size-2 && pnfrac));
}
- if (entropy_count < 0) {
+ if (unlikely(entropy_count < 0)) {
pr_warn("random: negative entropy/overflow: pool %s count %d\n",
r->name, entropy_count);
WARN_ON(1);
@@ -981,7 +981,7 @@ static size_t account(struct entropy_store *r, size_t nbytes, int min,
int reserved)
{
int entropy_count, orig;
- size_t ibytes;
+ size_t ibytes, nfrac;
BUG_ON(r->entropy_count > r->poolinfo->poolfracbits);
@@ -999,7 +999,17 @@ retry:
}
if (ibytes < min)
ibytes = 0;
- if ((entropy_count -= ibytes << (ENTROPY_SHIFT + 3)) < 0)
+
+ nfrac = ibytes << (ENTROPY_SHIFT + 3);
+ if (entropy_count < 0) {
+ pr_warn("random: negative entropy count: pool %s count %d\n",
+ r->name, entropy_count);
+ WARN_ON(1);
+ entropy_count = 0;
+ }
+ if ((unsigned) entropy_count > nfrac)
+ entropy_count -= nfrac;
+ else
entropy_count = 0;
if (cmpxchg(&r->entropy_count, orig, entropy_count) != orig)
@@ -1376,6 +1386,7 @@ urandom_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
"with %d bits of entropy available\n",
current->comm, nonblocking_pool.entropy_total);
+ nbytes = min_t(size_t, nbytes, INT_MAX >> ENTROPY_SHIFT);
ret = extract_entropy_user(&nonblocking_pool, buf, nbytes);
trace_urandom_read(8 * nbytes, ENTROPY_BITS(&nonblocking_pool),
--
2.0.0
Hi,
On Fr, 2014-07-18 at 18:07 -0400, Theodore Ts'o wrote:
> From: Hannes Frederic Sowa <[email protected]>
>
> The expression entropy_count -= ibytes << (ENTROPY_SHIFT + 3) could
> actually increase entropy_count if during assignment of the unsigned
> expression on the RHS (mind the -=) we reduce the value modulo
> 2^width(int) and assign it to entropy_count. Trinity found this.
>
> [ Commit modified by tytso to add an additional safety check for a
> negative entropy_count -- which should never happen, and to also add
> an additional paranoia check to prevent overly large count values to
> be passed into urandom_read(). ]
>
> Reported-by: Dave Jones <[email protected]>
> Cc: Greg Price <[email protected]>
> Signed-off-by: Hannes Frederic Sowa <[email protected]>
> Signed-off-by: Theodore Ts'o <[email protected]>
> ---
> drivers/char/random.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 0a7ac0a..003f744 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -641,7 +641,7 @@ retry:
> } while (unlikely(entropy_count < pool_size-2 && pnfrac));
> }
>
> - if (entropy_count < 0) {
> + if (unlikely(entropy_count < 0)) {
> pr_warn("random: negative entropy/overflow: pool %s count %d\n",
> r->name, entropy_count);
> WARN_ON(1);
> @@ -981,7 +981,7 @@ static size_t account(struct entropy_store *r, size_t nbytes, int min,
> int reserved)
> {
> int entropy_count, orig;
> - size_t ibytes;
> + size_t ibytes, nfrac;
>
> BUG_ON(r->entropy_count > r->poolinfo->poolfracbits);
>
> @@ -999,7 +999,17 @@ retry:
> }
> if (ibytes < min)
> ibytes = 0;
> - if ((entropy_count -= ibytes << (ENTROPY_SHIFT + 3)) < 0)
> +
> + nfrac = ibytes << (ENTROPY_SHIFT + 3);
> + if (entropy_count < 0) {
Minor nit: maybe also add an unlikely() here?
> + pr_warn("random: negative entropy count: pool %s count %d\n",
> + r->name, entropy_count);
> + WARN_ON(1);
> + entropy_count = 0;
> + }
> + if ((unsigned) entropy_count > nfrac)
(unsigned) -> (size_t)
size_t could also be (unsigned long) so the plain (unsigned) is
misleading.
(Maybe I wouldn't have done the cast at all, as we compile the kernel
with -Wno-sign-compare and we have the < 0 check right above, but I
don't have a strong opinion on that.)
> + entropy_count -= nfrac;
> + else
> entropy_count = 0;
>
> if (cmpxchg(&r->entropy_count, orig, entropy_count) != orig)
> @@ -1376,6 +1386,7 @@ urandom_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
> "with %d bits of entropy available\n",
> current->comm, nonblocking_pool.entropy_total);
>
> + nbytes = min_t(size_t, nbytes, INT_MAX >> ENTROPY_SHIFT);
Hmm, not sure, nfracs unit is 1/8 bits, so don't we have to limit nbytes
to INT_MAX >> (ENTROPY_SHIFT + 3) here?
And if we want to be even more correct here, we could switch from
INT_MAX to SIZE_MAX, as we do all nfrac calculations in the size_t
domain.
Maximum read/write size is SSIZE_MAX, so we don't need to care about
that, but if a user on a 64 bit machine requests INT_MAX bytes, we only
account/extract INT_MAX >> (ENTROPY_SHIFT + 3) bytes and cause a partial
read, though we actually could calulcate a correct nfrac for INT_MAX.
Because we don't have such large poolfragbits pools we would still
always end up with 0 while still allowing larger buffers to fill.
Hm, I just see that we should leave the INT_MAX limit just because of
the tracepoint.
Good catch,
Hannes
> ret = extract_entropy_user(&nonblocking_pool, buf, nbytes);
>
> trace_urandom_read(8 * nbytes, ENTROPY_BITS(&nonblocking_pool),
On Sat, Jul 19, 2014 at 01:35:48AM +0200, Hannes Frederic Sowa wrote:
> > + nfrac = ibytes << (ENTROPY_SHIFT + 3);
> > + if (entropy_count < 0) {
>
> Minor nit: maybe also add an unlikely() here?
Yep, done.
> > + if ((unsigned) entropy_count > nfrac)
>
> (unsigned) -> (size_t)
>
> size_t could also be (unsigned long) so the plain (unsigned) is
> misleading.
Good point, done.
> (Maybe I wouldn't have done the cast at all, as we compile the kernel
> with -Wno-sign-compare and we have the < 0 check right above, but I
> don't have a strong opinion on that.)
I also wanted to shut up other static code checkers like Coverity. :-)
> > + nbytes = min_t(size_t, nbytes, INT_MAX >> ENTROPY_SHIFT);
>
> Hmm, not sure, nfracs unit is 1/8 bits, so don't we have to limit nbytes
> to INT_MAX >> (ENTROPY_SHIFT + 3) here?
Good catch, done.
> And if we want to be even more correct here, we could switch from
> INT_MAX to SIZE_MAX, as we do all nfrac calculations in the size_t
> domain.
The main reason why I used INT_MAX was as a further safety check to
protect the:
entropy_count -= nfrac;
calculation, since nfrac is size_t and entropy_count is int.
In fact I think this online change ("nbytes = min_t(size_t, nbytes,
INT_MAX >> (ENTROPY_SHIFT + 3));") would have been enough to fix the
problem all by itself, but the other changes results in code which is
cleaner and easier to understand, and I'm a firm believer in multiple
layers of protection. :-)
Cheers,
- Ted
commit 79a8468747c5f95ed3d5ce8376a3e82e0c5857fc
Author: Hannes Frederic Sowa <[email protected]>
Date: Fri Jul 18 17:26:41 2014 -0400
random: check for increase of entropy_count because of signed conversion
The expression entropy_count -= ibytes << (ENTROPY_SHIFT + 3) could
actually increase entropy_count if during assignment of the unsigned
expression on the RHS (mind the -=) we reduce the value modulo
2^width(int) and assign it to entropy_count. Trinity found this.
[ Commit modified by tytso to add an additional safety check for a
negative entropy_count -- which should never happen, and to also add
an additional paranoia check to prevent overly large count values to
be passed into urandom_read(). ]
Reported-by: Dave Jones <[email protected]>
Signed-off-by: Hannes Frederic Sowa <[email protected]>
Signed-off-by: Theodore Ts'o <[email protected]>
Cc: [email protected]
diff --git a/drivers/char/random.c b/drivers/char/random.c
index 0a7ac0a..71529e1 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -641,7 +641,7 @@ retry:
} while (unlikely(entropy_count < pool_size-2 && pnfrac));
}
- if (entropy_count < 0) {
+ if (unlikely(entropy_count < 0)) {
pr_warn("random: negative entropy/overflow: pool %s count %d\n",
r->name, entropy_count);
WARN_ON(1);
@@ -981,7 +981,7 @@ static size_t account(struct entropy_store *r, size_t nbytes, int min,
int reserved)
{
int entropy_count, orig;
- size_t ibytes;
+ size_t ibytes, nfrac;
BUG_ON(r->entropy_count > r->poolinfo->poolfracbits);
@@ -999,7 +999,17 @@ retry:
}
if (ibytes < min)
ibytes = 0;
- if ((entropy_count -= ibytes << (ENTROPY_SHIFT + 3)) < 0)
+
+ if (unlikely(entropy_count < 0)) {
+ pr_warn("random: negative entropy count: pool %s count %d\n",
+ r->name, entropy_count);
+ WARN_ON(1);
+ entropy_count = 0;
+ }
+ nfrac = ibytes << (ENTROPY_SHIFT + 3);
+ if ((size_t) entropy_count > nfrac)
+ entropy_count -= nfrac;
+ else
entropy_count = 0;
if (cmpxchg(&r->entropy_count, orig, entropy_count) != orig)
@@ -1376,6 +1386,7 @@ urandom_read(struct file *file, char __user *buf, size_t nbytes, loff_t *ppos)
"with %d bits of entropy available\n",
current->comm, nonblocking_pool.entropy_total);
+ nbytes = min_t(size_t, nbytes, INT_MAX >> (ENTROPY_SHIFT + 3));
ret = extract_entropy_user(&nonblocking_pool, buf, nbytes);
trace_urandom_read(8 * nbytes, ENTROPY_BITS(&nonblocking_pool),
On Sa, 2014-07-19 at 01:42 -0400, Theodore Ts'o wrote:
> On Sat, Jul 19, 2014 at 01:35:48AM +0200, Hannes Frederic Sowa wrote:
> > > + nfrac = ibytes << (ENTROPY_SHIFT + 3);
> > > + if (entropy_count < 0) {
> >
> > Minor nit: maybe also add an unlikely() here?
>
> Yep, done.
>
> > > + if ((unsigned) entropy_count > nfrac)
> >
> > (unsigned) -> (size_t)
> >
> > size_t could also be (unsigned long) so the plain (unsigned) is
> > misleading.
>
> Good point, done.
>
> > (Maybe I wouldn't have done the cast at all, as we compile the kernel
> > with -Wno-sign-compare and we have the < 0 check right above, but I
> > don't have a strong opinion on that.)
>
> I also wanted to shut up other static code checkers like Coverity. :-)
>
> > > + nbytes = min_t(size_t, nbytes, INT_MAX >> ENTROPY_SHIFT);
> >
> > Hmm, not sure, nfracs unit is 1/8 bits, so don't we have to limit nbytes
> > to INT_MAX >> (ENTROPY_SHIFT + 3) here?
>
> Good catch, done.
>
> > And if we want to be even more correct here, we could switch from
> > INT_MAX to SIZE_MAX, as we do all nfrac calculations in the size_t
> > domain.
>
> The main reason why I used INT_MAX was as a further safety check to
> protect the:
>
> entropy_count -= nfrac;
>
> calculation, since nfrac is size_t and entropy_count is int.
>
> In fact I think this online change ("nbytes = min_t(size_t, nbytes,
> INT_MAX >> (ENTROPY_SHIFT + 3));") would have been enough to fix the
> problem all by itself, but the other changes results in code which is
> cleaner and easier to understand, and I'm a firm believer in multiple
> layers of protection. :-)
I see and can agree here. :)
I think the patch is good to go.
Thanks you,
Hannes