Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Theodore Ts'o]

On Mon, Oct 30, 2017 at 08:39:56AM +0100, Greg KH wrote:
> 
> No "Reported-by:"?

Good point, fixed in my tree.

- Ted

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Theodore Ts'o]

On Mon, Oct 30, 2017 at 08:39:56AM +0100, Greg KH wrote:
> 
> No "Reported-by:"?

Good point, fixed in my tree.

- Ted

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Greg KH]

On Sun, Oct 29, 2017 at 02:25:29PM -0400, Theodore Ts'o wrote:
> On Sat, Oct 28, 2017 at 11:22:00AM +0800, Chen Feng wrote:
> > 
> > I checked the ioctl. What's the purpose of RNDADDTOENTCNT ioctl to
> > userspace?
> 
> It's a legacy ioctl which is probably not used anywhere; it's been
> replaced by RNDADDENTROPY.  It previously allows root to bump the
> entropy estimate, but the right way to do this by rngd is to
> atomically add entropy to the pool land and bump the entropy estimate
> at the same time.
> 
> The UBSAN is harmless.  The ioctl requires root, and the entropy_total
> field, which is involved in the UBSAN, is only used in the first few
> seconds of boot, to determine when the entropy pool has been
> initialized.  In general on desktop and servers this happens before
> userspace has a chance to run.
> 
> In any case, here's a fix for this.
> 
>   - Ted
> 
> commit 6f7034d0c52e21f30002b95126b6b98e4618dc57
> Author: Theodore Ts'o 
> Date:   Sun Oct 29 14:17:26 2017 -0400
> 
> random: use a tighter cap in credit_entropy_bits_safe()
> 
> This fixes a harmless UBSAN where root could potentially end up
> causing an overflow while bumping the entropy_total field (which is
> ignored once the entropy pool has been initialized, and this generally
> is completed during the boot sequence).
> 
> This is marginal for the stable kernel series, but it's a really
> trivial patch, and it UBSAN warning that might cause security folks to
> get overly excited for no reason.
> 
> Signed-off-by: Theodore Ts'o 
> Cc: sta...@vger.kernel.org

No "Reported-by:"?

thanks,

greg k-h

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Greg KH]

On Sun, Oct 29, 2017 at 02:25:29PM -0400, Theodore Ts'o wrote:
> On Sat, Oct 28, 2017 at 11:22:00AM +0800, Chen Feng wrote:
> > 
> > I checked the ioctl. What's the purpose of RNDADDTOENTCNT ioctl to
> > userspace?
> 
> It's a legacy ioctl which is probably not used anywhere; it's been
> replaced by RNDADDENTROPY.  It previously allows root to bump the
> entropy estimate, but the right way to do this by rngd is to
> atomically add entropy to the pool land and bump the entropy estimate
> at the same time.
> 
> The UBSAN is harmless.  The ioctl requires root, and the entropy_total
> field, which is involved in the UBSAN, is only used in the first few
> seconds of boot, to determine when the entropy pool has been
> initialized.  In general on desktop and servers this happens before
> userspace has a chance to run.
> 
> In any case, here's a fix for this.
> 
>   - Ted
> 
> commit 6f7034d0c52e21f30002b95126b6b98e4618dc57
> Author: Theodore Ts'o 
> Date:   Sun Oct 29 14:17:26 2017 -0400
> 
> random: use a tighter cap in credit_entropy_bits_safe()
> 
> This fixes a harmless UBSAN where root could potentially end up
> causing an overflow while bumping the entropy_total field (which is
> ignored once the entropy pool has been initialized, and this generally
> is completed during the boot sequence).
> 
> This is marginal for the stable kernel series, but it's a really
> trivial patch, and it UBSAN warning that might cause security folks to
> get overly excited for no reason.
> 
> Signed-off-by: Theodore Ts'o 
> Cc: sta...@vger.kernel.org

No "Reported-by:"?

thanks,

greg k-h

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Theodore Ts'o]

On Sat, Oct 28, 2017 at 11:22:00AM +0800, Chen Feng wrote:
> 
> I checked the ioctl. What's the purpose of RNDADDTOENTCNT ioctl to
> userspace?

It's a legacy ioctl which is probably not used anywhere; it's been
replaced by RNDADDENTROPY.  It previously allows root to bump the
entropy estimate, but the right way to do this by rngd is to
atomically add entropy to the pool land and bump the entropy estimate
at the same time.

The UBSAN is harmless.  The ioctl requires root, and the entropy_total
field, which is involved in the UBSAN, is only used in the first few
seconds of boot, to determine when the entropy pool has been
initialized.  In general on desktop and servers this happens before
userspace has a chance to run.

In any case, here's a fix for this.

- Ted

commit 6f7034d0c52e21f30002b95126b6b98e4618dc57
Author: Theodore Ts'o 
Date:   Sun Oct 29 14:17:26 2017 -0400

random: use a tighter cap in credit_entropy_bits_safe()

This fixes a harmless UBSAN where root could potentially end up
causing an overflow while bumping the entropy_total field (which is
ignored once the entropy pool has been initialized, and this generally
is completed during the boot sequence).

This is marginal for the stable kernel series, but it's a really
trivial patch, and it UBSAN warning that might cause security folks to
get overly excited for no reason.

Signed-off-by: Theodore Ts'o 
Cc: sta...@vger.kernel.org

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 8ad92707e45f..ae8a2f829890 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -733,7 +733,7 @@ static void credit_entropy_bits(struct entropy_store *r, 
int nbits)
 
 static int credit_entropy_bits_safe(struct entropy_store *r, int nbits)
 {
-   const int nbits_max = (int)(~0U >> (ENTROPY_SHIFT + 1));
+   const int nbits_max = r->poolinfo->poolwords * 32;
 
if (nbits < 0)
return -EINVAL;

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Theodore Ts'o]

On Sat, Oct 28, 2017 at 11:22:00AM +0800, Chen Feng wrote:
> 
> I checked the ioctl. What's the purpose of RNDADDTOENTCNT ioctl to
> userspace?

It's a legacy ioctl which is probably not used anywhere; it's been
replaced by RNDADDENTROPY.  It previously allows root to bump the
entropy estimate, but the right way to do this by rngd is to
atomically add entropy to the pool land and bump the entropy estimate
at the same time.

The UBSAN is harmless.  The ioctl requires root, and the entropy_total
field, which is involved in the UBSAN, is only used in the first few
seconds of boot, to determine when the entropy pool has been
initialized.  In general on desktop and servers this happens before
userspace has a chance to run.

In any case, here's a fix for this.

- Ted

commit 6f7034d0c52e21f30002b95126b6b98e4618dc57
Author: Theodore Ts'o 
Date:   Sun Oct 29 14:17:26 2017 -0400

random: use a tighter cap in credit_entropy_bits_safe()

This fixes a harmless UBSAN where root could potentially end up
causing an overflow while bumping the entropy_total field (which is
ignored once the entropy pool has been initialized, and this generally
is completed during the boot sequence).

This is marginal for the stable kernel series, but it's a really
trivial patch, and it UBSAN warning that might cause security folks to
get overly excited for no reason.

Signed-off-by: Theodore Ts'o 
Cc: sta...@vger.kernel.org

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 8ad92707e45f..ae8a2f829890 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -733,7 +733,7 @@ static void credit_entropy_bits(struct entropy_store *r, 
int nbits)
 
 static int credit_entropy_bits_safe(struct entropy_store *r, int nbits)
 {
-   const int nbits_max = (int)(~0U >> (ENTROPY_SHIFT + 1));
+   const int nbits_max = r->poolinfo->poolwords * 32;
 
if (nbits < 0)
return -EINVAL;

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Chen Feng]

Hi ted,

On 2017/10/26 23:04, Theodore Ts'o wrote:
> On Thu, Oct 26, 2017 at 04:25:15PM +0800, Chen Feng wrote:
>>
>>
>> On 2017/10/25 16:49, Theodore Ts'o wrote:
>>> Other people who have sent me fuzzer test reproducers are able to
>>> reproduce syzkaller logs into a simple C program.  Can you explain to
>>> me what the heck:
>>>
 r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 
 0x0, 0x0)
>>>
>>> means?
>>
>> Take a look at this:
>>
>> https://github.com/google/syzkaller/blob/master/sys/linux/random.txt
> 
> Sorry, this *still* looks like gobbledygook.
> 
> What ioctls are you executing, and with what arguments?
> 
> *Please*, give me a C program I can compile.

I checked the ioctl. What's the purpose of RNDADDTOENTCNT ioctl to userspace?

We need to checked the user-input at credit_entropy_bits_safe.

+   if (INT_MAX - nbits < r->entropy_total)
+   return -EINVAL;
+


The test-code below:

void *random_ioctl_test(void *args)
{
int fd = -1;
int ret = -1;
int test_arg = 0x7fff;

fd = open("dev/urandom", 0x0, 0x0);
if (fd < 0) {
printf("open dev/urandom failed!\n");
return NULL;
}

ret = ioctl(fd, 0x40045201, _arg);

printf("random_ioctl ret=%d\n", ret);
close(fd);
return NULL;
}

int main(int argc, char *argv[])
{
int ret, i;
pthread_t thread[100];

for (i = 0; i < 100; i++) {
ret = pthread_create([i], NULL, random_ioctl_test, );
if (ret) {
printf("create thread %d fail with ret=%d\n", i, ret);
return -1;
}
}

for (i = 0; i < 100; i++) {
pthread_join(thread[i], NULL);
}
return 0;
}


> 
>-Ted
> 
> .
> 

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Chen Feng]

Hi ted,

On 2017/10/26 23:04, Theodore Ts'o wrote:
> On Thu, Oct 26, 2017 at 04:25:15PM +0800, Chen Feng wrote:
>>
>>
>> On 2017/10/25 16:49, Theodore Ts'o wrote:
>>> Other people who have sent me fuzzer test reproducers are able to
>>> reproduce syzkaller logs into a simple C program.  Can you explain to
>>> me what the heck:
>>>
 r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 
 0x0, 0x0)
>>>
>>> means?
>>
>> Take a look at this:
>>
>> https://github.com/google/syzkaller/blob/master/sys/linux/random.txt
> 
> Sorry, this *still* looks like gobbledygook.
> 
> What ioctls are you executing, and with what arguments?
> 
> *Please*, give me a C program I can compile.

I checked the ioctl. What's the purpose of RNDADDTOENTCNT ioctl to userspace?

We need to checked the user-input at credit_entropy_bits_safe.

+   if (INT_MAX - nbits < r->entropy_total)
+   return -EINVAL;
+


The test-code below:

void *random_ioctl_test(void *args)
{
int fd = -1;
int ret = -1;
int test_arg = 0x7fff;

fd = open("dev/urandom", 0x0, 0x0);
if (fd < 0) {
printf("open dev/urandom failed!\n");
return NULL;
}

ret = ioctl(fd, 0x40045201, _arg);

printf("random_ioctl ret=%d\n", ret);
close(fd);
return NULL;
}

int main(int argc, char *argv[])
{
int ret, i;
pthread_t thread[100];

for (i = 0; i < 100; i++) {
ret = pthread_create([i], NULL, random_ioctl_test, );
if (ret) {
printf("create thread %d fail with ret=%d\n", i, ret);
return -1;
}
}

for (i = 0; i < 100; i++) {
pthread_join(thread[i], NULL);
}
return 0;
}


> 
>-Ted
> 
> .
> 

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Theodore Ts'o]

On Thu, Oct 26, 2017 at 04:25:15PM +0800, Chen Feng wrote:
> 
> 
> On 2017/10/25 16:49, Theodore Ts'o wrote:
> > Other people who have sent me fuzzer test reproducers are able to
> > reproduce syzkaller logs into a simple C program.  Can you explain to
> > me what the heck:
> > 
> >> r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 
> >> 0x0, 0x0)
> > 
> > means?
> 
> Take a look at this:
> 
> https://github.com/google/syzkaller/blob/master/sys/linux/random.txt

Sorry, this *still* looks like gobbledygook.

What ioctls are you executing, and with what arguments?

*Please*, give me a C program I can compile.

 -Ted

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Theodore Ts'o]

On Thu, Oct 26, 2017 at 04:25:15PM +0800, Chen Feng wrote:
> 
> 
> On 2017/10/25 16:49, Theodore Ts'o wrote:
> > Other people who have sent me fuzzer test reproducers are able to
> > reproduce syzkaller logs into a simple C program.  Can you explain to
> > me what the heck:
> > 
> >> r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 
> >> 0x0, 0x0)
> > 
> > means?
> 
> Take a look at this:
> 
> https://github.com/google/syzkaller/blob/master/sys/linux/random.txt

Sorry, this *still* looks like gobbledygook.

What ioctls are you executing, and with what arguments?

*Please*, give me a C program I can compile.

 -Ted

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Chen Feng]

On 2017/10/25 16:49, Theodore Ts'o wrote:
> Other people who have sent me fuzzer test reproducers are able to
> reproduce syzkaller logs into a simple C program.  Can you explain to
> me what the heck:
> 
>> r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 
>> 0x0, 0x0)
> 
> means?

Take a look at this:

https://github.com/google/syzkaller/blob/master/sys/linux/random.txt

> 
>   - Ted
> 
> 

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Chen Feng]

On 2017/10/25 16:49, Theodore Ts'o wrote:
> Other people who have sent me fuzzer test reproducers are able to
> reproduce syzkaller logs into a simple C program.  Can you explain to
> me what the heck:
> 
>> r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 
>> 0x0, 0x0)
> 
> means?

Take a look at this:

https://github.com/google/syzkaller/blob/master/sys/linux/random.txt

> 
>   - Ted
> 
> 

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Theodore Ts'o]

Other people who have sent me fuzzer test reproducers are able to
reproduce syzkaller logs into a simple C program.  Can you explain to
me what the heck:

> r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 
> 0x0, 0x0)

means?

- Ted

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Theodore Ts'o]

Other people who have sent me fuzzer test reproducers are able to
reproduce syzkaller logs into a simple C program.  Can you explain to
me what the heck:

> r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 
> 0x0, 0x0)

means?

- Ted

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Chen Feng]

On 2017/10/25 14:56, Greg KH wrote:
> On Wed, Oct 25, 2017 at 02:30:56PM +0800, Chen Feng wrote:
>> Hi Ted,
>>
>> On 2017/10/24 18:25, Theodore Ts'o wrote:
>>> On Tue, Oct 24, 2017 at 11:09:27AM +0200, Greg KH wrote:
 On Tue, Oct 24, 2017 at 03:44:17PM +0800, Chen Feng wrote:
> [pid:11940,cpu6,syz-executor][flp_ioctl]cmd[0x1]
> Restart is not permit
> =
> UBSAN: Undefined behaviour in
> kernel/linux-4.4/drivers/char/random.c:676:19
> signed integer overflow:
> 2147483645 + 268435455 cannot be represented in type 'int'
> CPU: 4 PID: 11941 Comm: syz-executor Not tainted 4.4.76+ #2

 Does this also happen on 4.14-rc6?
>>>
>>> No.  It was fixed in 4.8, by commit 86a574de4590: "random: strengthen
>>> input validation for RNDADDTOENTCNT".
>>
>>
>> I see my kernel has already merged this patch. So I don't think this patch
>> can resolve the issue above.
> 
> Do you have a reproducer for this issue that we can use to test?
> 

It's hard to reproduce, we found this on stress test with syzkaller test.

r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 0x0)
syz_open_dev$random(&(0x7f005000-0xc)="2f6465762f72616e646f6d00", 0x0, 
0x10100)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
UBSAN: Undefined behaviour in kernel/linux-4.4/drivers/char/random.c:676:19
[] random_ioctl+0x338/0x384


git log --oneline drivers/char/random.c

3991576 random: properly align get_random_int_hash
5f87c47 Merge branch 'linux-linaro-lsk-v4.4' into linux-linaro-lsk-v4.4-android
f48dd2d random: add interrupt callback to VMBus IRQ handler
529025b random: print a warning for the first ten uninitialized random users
f41fc0b random: initialize the non-blocking pool via 
add_hwgenerator_randomness()
fdd8543 Merge branch 'linux-linaro-lsk-v4.4' into linux-linaro-lsk-v4.4-android
93f84c8 random: strengthen input validation for RNDADDTOENTCNT
06bfe14 FROMLIST: drivers: char: random: add get_random_long()
c271950 random: Remove kernel blocking API
205a525 random: Add callback API for random pool readiness
16b369a random: Blocking API for accessing nonblocking_pool
1d9de44 random: Wake up all getrandom(2) callers when pool is ready
19acc77 random: Fix fast_mix() function


> thanks,
> 
> greg k-h
> 
> .
> 

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Chen Feng]

On 2017/10/25 14:56, Greg KH wrote:
> On Wed, Oct 25, 2017 at 02:30:56PM +0800, Chen Feng wrote:
>> Hi Ted,
>>
>> On 2017/10/24 18:25, Theodore Ts'o wrote:
>>> On Tue, Oct 24, 2017 at 11:09:27AM +0200, Greg KH wrote:
 On Tue, Oct 24, 2017 at 03:44:17PM +0800, Chen Feng wrote:
> [pid:11940,cpu6,syz-executor][flp_ioctl]cmd[0x1]
> Restart is not permit
> =
> UBSAN: Undefined behaviour in
> kernel/linux-4.4/drivers/char/random.c:676:19
> signed integer overflow:
> 2147483645 + 268435455 cannot be represented in type 'int'
> CPU: 4 PID: 11941 Comm: syz-executor Not tainted 4.4.76+ #2

 Does this also happen on 4.14-rc6?
>>>
>>> No.  It was fixed in 4.8, by commit 86a574de4590: "random: strengthen
>>> input validation for RNDADDTOENTCNT".
>>
>>
>> I see my kernel has already merged this patch. So I don't think this patch
>> can resolve the issue above.
> 
> Do you have a reproducer for this issue that we can use to test?
> 

It's hard to reproduce, we found this on stress test with syzkaller test.

r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 0x0)
syz_open_dev$random(&(0x7f005000-0xc)="2f6465762f72616e646f6d00", 0x0, 
0x10100)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
r3 = syz_open_dev$urandom(&(0x7f00a000)="2f6465762f7572616e646f6d00", 0x0, 
0x0)
UBSAN: Undefined behaviour in kernel/linux-4.4/drivers/char/random.c:676:19
[] random_ioctl+0x338/0x384


git log --oneline drivers/char/random.c

3991576 random: properly align get_random_int_hash
5f87c47 Merge branch 'linux-linaro-lsk-v4.4' into linux-linaro-lsk-v4.4-android
f48dd2d random: add interrupt callback to VMBus IRQ handler
529025b random: print a warning for the first ten uninitialized random users
f41fc0b random: initialize the non-blocking pool via 
add_hwgenerator_randomness()
fdd8543 Merge branch 'linux-linaro-lsk-v4.4' into linux-linaro-lsk-v4.4-android
93f84c8 random: strengthen input validation for RNDADDTOENTCNT
06bfe14 FROMLIST: drivers: char: random: add get_random_long()
c271950 random: Remove kernel blocking API
205a525 random: Add callback API for random pool readiness
16b369a random: Blocking API for accessing nonblocking_pool
1d9de44 random: Wake up all getrandom(2) callers when pool is ready
19acc77 random: Fix fast_mix() function


> thanks,
> 
> greg k-h
> 
> .
> 

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Greg KH]

On Wed, Oct 25, 2017 at 02:30:56PM +0800, Chen Feng wrote:
> Hi Ted,
> 
> On 2017/10/24 18:25, Theodore Ts'o wrote:
> > On Tue, Oct 24, 2017 at 11:09:27AM +0200, Greg KH wrote:
> >> On Tue, Oct 24, 2017 at 03:44:17PM +0800, Chen Feng wrote:
> >>> [pid:11940,cpu6,syz-executor][flp_ioctl]cmd[0x1]
> >>> Restart is not permit
> >>> =
> >>> UBSAN: Undefined behaviour in
> >>> kernel/linux-4.4/drivers/char/random.c:676:19
> >>> signed integer overflow:
> >>> 2147483645 + 268435455 cannot be represented in type 'int'
> >>> CPU: 4 PID: 11941 Comm: syz-executor Not tainted 4.4.76+ #2
> >>
> >> Does this also happen on 4.14-rc6?
> > 
> > No.  It was fixed in 4.8, by commit 86a574de4590: "random: strengthen
> > input validation for RNDADDTOENTCNT".
> 
> 
> I see my kernel has already merged this patch. So I don't think this patch
> can resolve the issue above.

Do you have a reproducer for this issue that we can use to test?

thanks,

greg k-h

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Greg KH]

On Wed, Oct 25, 2017 at 02:30:56PM +0800, Chen Feng wrote:
> Hi Ted,
> 
> On 2017/10/24 18:25, Theodore Ts'o wrote:
> > On Tue, Oct 24, 2017 at 11:09:27AM +0200, Greg KH wrote:
> >> On Tue, Oct 24, 2017 at 03:44:17PM +0800, Chen Feng wrote:
> >>> [pid:11940,cpu6,syz-executor][flp_ioctl]cmd[0x1]
> >>> Restart is not permit
> >>> =
> >>> UBSAN: Undefined behaviour in
> >>> kernel/linux-4.4/drivers/char/random.c:676:19
> >>> signed integer overflow:
> >>> 2147483645 + 268435455 cannot be represented in type 'int'
> >>> CPU: 4 PID: 11941 Comm: syz-executor Not tainted 4.4.76+ #2
> >>
> >> Does this also happen on 4.14-rc6?
> > 
> > No.  It was fixed in 4.8, by commit 86a574de4590: "random: strengthen
> > input validation for RNDADDTOENTCNT".
> 
> 
> I see my kernel has already merged this patch. So I don't think this patch
> can resolve the issue above.

Do you have a reproducer for this issue that we can use to test?

thanks,

greg k-h

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Chen Feng]

Hi Ted,

On 2017/10/24 18:25, Theodore Ts'o wrote:
> On Tue, Oct 24, 2017 at 11:09:27AM +0200, Greg KH wrote:
>> On Tue, Oct 24, 2017 at 03:44:17PM +0800, Chen Feng wrote:
>>> [pid:11940,cpu6,syz-executor][flp_ioctl]cmd[0x1]
>>> Restart is not permit
>>> =
>>> UBSAN: Undefined behaviour in
>>> kernel/linux-4.4/drivers/char/random.c:676:19
>>> signed integer overflow:
>>> 2147483645 + 268435455 cannot be represented in type 'int'
>>> CPU: 4 PID: 11941 Comm: syz-executor Not tainted 4.4.76+ #2
>>
>> Does this also happen on 4.14-rc6?
> 
> No.  It was fixed in 4.8, by commit 86a574de4590: "random: strengthen
> input validation for RNDADDTOENTCNT".


I see my kernel has already merged this patch. So I don't think this patch
can resolve the issue above.

-feng
> 
>   - Ted
> 
> .
> 

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Chen Feng]

Hi Ted,

On 2017/10/24 18:25, Theodore Ts'o wrote:
> On Tue, Oct 24, 2017 at 11:09:27AM +0200, Greg KH wrote:
>> On Tue, Oct 24, 2017 at 03:44:17PM +0800, Chen Feng wrote:
>>> [pid:11940,cpu6,syz-executor][flp_ioctl]cmd[0x1]
>>> Restart is not permit
>>> =
>>> UBSAN: Undefined behaviour in
>>> kernel/linux-4.4/drivers/char/random.c:676:19
>>> signed integer overflow:
>>> 2147483645 + 268435455 cannot be represented in type 'int'
>>> CPU: 4 PID: 11941 Comm: syz-executor Not tainted 4.4.76+ #2
>>
>> Does this also happen on 4.14-rc6?
> 
> No.  It was fixed in 4.8, by commit 86a574de4590: "random: strengthen
> input validation for RNDADDTOENTCNT".


I see my kernel has already merged this patch. So I don't think this patch
can resolve the issue above.

-feng
> 
>   - Ted
> 
> .
> 

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Theodore Ts'o]

On Tue, Oct 24, 2017 at 11:09:27AM +0200, Greg KH wrote:
> On Tue, Oct 24, 2017 at 03:44:17PM +0800, Chen Feng wrote:
> > [pid:11940,cpu6,syz-executor][flp_ioctl]cmd[0x1]
> > Restart is not permit
> > =
> > UBSAN: Undefined behaviour in
> > kernel/linux-4.4/drivers/char/random.c:676:19
> > signed integer overflow:
> > 2147483645 + 268435455 cannot be represented in type 'int'
> > CPU: 4 PID: 11941 Comm: syz-executor Not tainted 4.4.76+ #2
> 
> Does this also happen on 4.14-rc6?

No.  It was fixed in 4.8, by commit 86a574de4590: "random: strengthen
input validation for RNDADDTOENTCNT".

- Ted

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Theodore Ts'o]

On Tue, Oct 24, 2017 at 11:09:27AM +0200, Greg KH wrote:
> On Tue, Oct 24, 2017 at 03:44:17PM +0800, Chen Feng wrote:
> > [pid:11940,cpu6,syz-executor][flp_ioctl]cmd[0x1]
> > Restart is not permit
> > =
> > UBSAN: Undefined behaviour in
> > kernel/linux-4.4/drivers/char/random.c:676:19
> > signed integer overflow:
> > 2147483645 + 268435455 cannot be represented in type 'int'
> > CPU: 4 PID: 11941 Comm: syz-executor Not tainted 4.4.76+ #2
> 
> Does this also happen on 4.14-rc6?

No.  It was fixed in 4.8, by commit 86a574de4590: "random: strengthen
input validation for RNDADDTOENTCNT".

- Ted

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Greg KH]

On Tue, Oct 24, 2017 at 05:24:01PM +0800, Chen Feng wrote:
> 
> 
> On 2017/10/24 17:09, Greg KH wrote:
> > On Tue, Oct 24, 2017 at 03:44:17PM +0800, Chen Feng wrote:
> >> [pid:11940,cpu6,syz-executor][flp_ioctl]cmd[0x1]
> >> Restart is not permit
> >> =
> >> UBSAN: Undefined behaviour in
> >> kernel/linux-4.4/drivers/char/random.c:676:19
> >> signed integer overflow:
> >> 2147483645 + 268435455 cannot be represented in type 'int'
> >> CPU: 4 PID: 11941 Comm: syz-executor Not tainted 4.4.76+ #2
> > 
> > Does this also happen on 4.14-rc6?
> 
> No, mainline also has this issue.
> > 
> >> TGID: 11928 Comm: syz-executor
> >> Hardware name: hi3660 (DT)
> >> Call trace:
> >> [] dump_backtrace+0x0/0x314
> >> [] show_stack+0x1c/0x24
> >> [] dump_stack+0xdc/0x130
> >> [] ubsan_epilogue+0x18/0x6c
> >> [] handle_overflow+0x180/0x1d4
> >> [] __ubsan_handle_add_overflow+0x2c/0x34
> >> [] credit_entropy_bits+0x358/0x9a8
> >> [] random_ioctl+0x338/0x384
> >> [] do_vfs_ioctl+0x60c/0xa4c
> >> [] SyS_ioctl+0x9c/0xc0
> >> [] el0_svc_naked+0x24/0x28
> >> =
> >>
> >> Signed-off-by: Chen Feng 
> >> Signed-off-by: Yukun Zhao 
> >> ---
> >>  drivers/char/random.c | 5 +
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/char/random.c b/drivers/char/random.c
> >> index 1ef2640..6f2bd6a 100644
> >> --- a/drivers/char/random.c
> >> +++ b/drivers/char/random.c
> >> @@ -699,6 +699,11 @@ static void credit_entropy_bits(struct entropy_store 
> >> *r, int nbits)
> >>if (cmpxchg(>entropy_count, orig, entropy_count) != orig)
> >>goto retry;
> >>  
> >> +  if (INT_MAX - nbits < r->entropy_total) {
> >> +  WARN_ON(1);
> > 
> > Why WARN_ON()?  What is that going to help with?
> Actually, I am not familiar with the random module
> 
> This patch is RFC to see if some one has better idea.

Well, not spamming the kernel log for something that userspace can
trigger is a good start to modifying your patch :)

thanks,

greg k-h

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Greg KH]

On Tue, Oct 24, 2017 at 05:24:01PM +0800, Chen Feng wrote:
> 
> 
> On 2017/10/24 17:09, Greg KH wrote:
> > On Tue, Oct 24, 2017 at 03:44:17PM +0800, Chen Feng wrote:
> >> [pid:11940,cpu6,syz-executor][flp_ioctl]cmd[0x1]
> >> Restart is not permit
> >> =
> >> UBSAN: Undefined behaviour in
> >> kernel/linux-4.4/drivers/char/random.c:676:19
> >> signed integer overflow:
> >> 2147483645 + 268435455 cannot be represented in type 'int'
> >> CPU: 4 PID: 11941 Comm: syz-executor Not tainted 4.4.76+ #2
> > 
> > Does this also happen on 4.14-rc6?
> 
> No, mainline also has this issue.
> > 
> >> TGID: 11928 Comm: syz-executor
> >> Hardware name: hi3660 (DT)
> >> Call trace:
> >> [] dump_backtrace+0x0/0x314
> >> [] show_stack+0x1c/0x24
> >> [] dump_stack+0xdc/0x130
> >> [] ubsan_epilogue+0x18/0x6c
> >> [] handle_overflow+0x180/0x1d4
> >> [] __ubsan_handle_add_overflow+0x2c/0x34
> >> [] credit_entropy_bits+0x358/0x9a8
> >> [] random_ioctl+0x338/0x384
> >> [] do_vfs_ioctl+0x60c/0xa4c
> >> [] SyS_ioctl+0x9c/0xc0
> >> [] el0_svc_naked+0x24/0x28
> >> =
> >>
> >> Signed-off-by: Chen Feng 
> >> Signed-off-by: Yukun Zhao 
> >> ---
> >>  drivers/char/random.c | 5 +
> >>  1 file changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/char/random.c b/drivers/char/random.c
> >> index 1ef2640..6f2bd6a 100644
> >> --- a/drivers/char/random.c
> >> +++ b/drivers/char/random.c
> >> @@ -699,6 +699,11 @@ static void credit_entropy_bits(struct entropy_store 
> >> *r, int nbits)
> >>if (cmpxchg(>entropy_count, orig, entropy_count) != orig)
> >>goto retry;
> >>  
> >> +  if (INT_MAX - nbits < r->entropy_total) {
> >> +  WARN_ON(1);
> > 
> > Why WARN_ON()?  What is that going to help with?
> Actually, I am not familiar with the random module
> 
> This patch is RFC to see if some one has better idea.

Well, not spamming the kernel log for something that userspace can
trigger is a good start to modifying your patch :)

thanks,

greg k-h

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Chen Feng]

On 2017/10/24 17:09, Greg KH wrote:
> On Tue, Oct 24, 2017 at 03:44:17PM +0800, Chen Feng wrote:
>> [pid:11940,cpu6,syz-executor][flp_ioctl]cmd[0x1]
>> Restart is not permit
>> =
>> UBSAN: Undefined behaviour in
>> kernel/linux-4.4/drivers/char/random.c:676:19
>> signed integer overflow:
>> 2147483645 + 268435455 cannot be represented in type 'int'
>> CPU: 4 PID: 11941 Comm: syz-executor Not tainted 4.4.76+ #2
> 
> Does this also happen on 4.14-rc6?

No, mainline also has this issue.
> 
>> TGID: 11928 Comm: syz-executor
>> Hardware name: hi3660 (DT)
>> Call trace:
>> [] dump_backtrace+0x0/0x314
>> [] show_stack+0x1c/0x24
>> [] dump_stack+0xdc/0x130
>> [] ubsan_epilogue+0x18/0x6c
>> [] handle_overflow+0x180/0x1d4
>> [] __ubsan_handle_add_overflow+0x2c/0x34
>> [] credit_entropy_bits+0x358/0x9a8
>> [] random_ioctl+0x338/0x384
>> [] do_vfs_ioctl+0x60c/0xa4c
>> [] SyS_ioctl+0x9c/0xc0
>> [] el0_svc_naked+0x24/0x28
>> =
>>
>> Signed-off-by: Chen Feng 
>> Signed-off-by: Yukun Zhao 
>> ---
>>  drivers/char/random.c | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/char/random.c b/drivers/char/random.c
>> index 1ef2640..6f2bd6a 100644
>> --- a/drivers/char/random.c
>> +++ b/drivers/char/random.c
>> @@ -699,6 +699,11 @@ static void credit_entropy_bits(struct entropy_store 
>> *r, int nbits)
>>  if (cmpxchg(>entropy_count, orig, entropy_count) != orig)
>>  goto retry;
>>  
>> +if (INT_MAX - nbits < r->entropy_total) {
>> +WARN_ON(1);
> 
> Why WARN_ON()?  What is that going to help with?
Actually, I am not familiar with the random module

This patch is RFC to see if some one has better idea.

-feng

> 
> thanks,
> 
> greg k-h
> 
> .
> 

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Chen Feng]

On 2017/10/24 17:09, Greg KH wrote:
> On Tue, Oct 24, 2017 at 03:44:17PM +0800, Chen Feng wrote:
>> [pid:11940,cpu6,syz-executor][flp_ioctl]cmd[0x1]
>> Restart is not permit
>> =
>> UBSAN: Undefined behaviour in
>> kernel/linux-4.4/drivers/char/random.c:676:19
>> signed integer overflow:
>> 2147483645 + 268435455 cannot be represented in type 'int'
>> CPU: 4 PID: 11941 Comm: syz-executor Not tainted 4.4.76+ #2
> 
> Does this also happen on 4.14-rc6?

No, mainline also has this issue.
> 
>> TGID: 11928 Comm: syz-executor
>> Hardware name: hi3660 (DT)
>> Call trace:
>> [] dump_backtrace+0x0/0x314
>> [] show_stack+0x1c/0x24
>> [] dump_stack+0xdc/0x130
>> [] ubsan_epilogue+0x18/0x6c
>> [] handle_overflow+0x180/0x1d4
>> [] __ubsan_handle_add_overflow+0x2c/0x34
>> [] credit_entropy_bits+0x358/0x9a8
>> [] random_ioctl+0x338/0x384
>> [] do_vfs_ioctl+0x60c/0xa4c
>> [] SyS_ioctl+0x9c/0xc0
>> [] el0_svc_naked+0x24/0x28
>> =
>>
>> Signed-off-by: Chen Feng 
>> Signed-off-by: Yukun Zhao 
>> ---
>>  drivers/char/random.c | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/char/random.c b/drivers/char/random.c
>> index 1ef2640..6f2bd6a 100644
>> --- a/drivers/char/random.c
>> +++ b/drivers/char/random.c
>> @@ -699,6 +699,11 @@ static void credit_entropy_bits(struct entropy_store 
>> *r, int nbits)
>>  if (cmpxchg(>entropy_count, orig, entropy_count) != orig)
>>  goto retry;
>>  
>> +if (INT_MAX - nbits < r->entropy_total) {
>> +WARN_ON(1);
> 
> Why WARN_ON()?  What is that going to help with?
Actually, I am not familiar with the random module

This patch is RFC to see if some one has better idea.

-feng

> 
> thanks,
> 
> greg k-h
> 
> .
> 

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Greg KH]

On Tue, Oct 24, 2017 at 03:44:17PM +0800, Chen Feng wrote:
> [pid:11940,cpu6,syz-executor][flp_ioctl]cmd[0x1]
> Restart is not permit
> =
> UBSAN: Undefined behaviour in
> kernel/linux-4.4/drivers/char/random.c:676:19
> signed integer overflow:
> 2147483645 + 268435455 cannot be represented in type 'int'
> CPU: 4 PID: 11941 Comm: syz-executor Not tainted 4.4.76+ #2

Does this also happen on 4.14-rc6?

> TGID: 11928 Comm: syz-executor
> Hardware name: hi3660 (DT)
> Call trace:
> [] dump_backtrace+0x0/0x314
> [] show_stack+0x1c/0x24
> [] dump_stack+0xdc/0x130
> [] ubsan_epilogue+0x18/0x6c
> [] handle_overflow+0x180/0x1d4
> [] __ubsan_handle_add_overflow+0x2c/0x34
> [] credit_entropy_bits+0x358/0x9a8
> [] random_ioctl+0x338/0x384
> [] do_vfs_ioctl+0x60c/0xa4c
> [] SyS_ioctl+0x9c/0xc0
> [] el0_svc_naked+0x24/0x28
> =
> 
> Signed-off-by: Chen Feng 
> Signed-off-by: Yukun Zhao 
> ---
>  drivers/char/random.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 1ef2640..6f2bd6a 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -699,6 +699,11 @@ static void credit_entropy_bits(struct entropy_store *r, 
> int nbits)
>   if (cmpxchg(>entropy_count, orig, entropy_count) != orig)
>   goto retry;
>  
> + if (INT_MAX - nbits < r->entropy_total) {
> + WARN_ON(1);

Why WARN_ON()?  What is that going to help with?

thanks,

greg k-h

Re: [PATCH RFC] random: fix syzkaller fuzzer test int overflow

[Greg KH]

On Tue, Oct 24, 2017 at 03:44:17PM +0800, Chen Feng wrote:
> [pid:11940,cpu6,syz-executor][flp_ioctl]cmd[0x1]
> Restart is not permit
> =
> UBSAN: Undefined behaviour in
> kernel/linux-4.4/drivers/char/random.c:676:19
> signed integer overflow:
> 2147483645 + 268435455 cannot be represented in type 'int'
> CPU: 4 PID: 11941 Comm: syz-executor Not tainted 4.4.76+ #2

Does this also happen on 4.14-rc6?

> TGID: 11928 Comm: syz-executor
> Hardware name: hi3660 (DT)
> Call trace:
> [] dump_backtrace+0x0/0x314
> [] show_stack+0x1c/0x24
> [] dump_stack+0xdc/0x130
> [] ubsan_epilogue+0x18/0x6c
> [] handle_overflow+0x180/0x1d4
> [] __ubsan_handle_add_overflow+0x2c/0x34
> [] credit_entropy_bits+0x358/0x9a8
> [] random_ioctl+0x338/0x384
> [] do_vfs_ioctl+0x60c/0xa4c
> [] SyS_ioctl+0x9c/0xc0
> [] el0_svc_naked+0x24/0x28
> =
> 
> Signed-off-by: Chen Feng 
> Signed-off-by: Yukun Zhao 
> ---
>  drivers/char/random.c | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 1ef2640..6f2bd6a 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -699,6 +699,11 @@ static void credit_entropy_bits(struct entropy_store *r, 
> int nbits)
>   if (cmpxchg(>entropy_count, orig, entropy_count) != orig)
>   goto retry;
>  
> + if (INT_MAX - nbits < r->entropy_total) {
> + WARN_ON(1);

Why WARN_ON()?  What is that going to help with?

thanks,

greg k-h