random: strengthen input validation for RNDADDTOENTCNT
authorTheodore Ts'o <tytso@mit.edu>
Sun, 3 Jul 2016 21:01:26 +0000 (17:01 -0400)
committerTheodore Ts'o <tytso@mit.edu>
Sun, 3 Jul 2016 21:09:33 +0000 (17:09 -0400)
Don't allow RNDADDTOENTCNT or RNDADDENTROPY to accept a negative
entropy value.  It doesn't make any sense to subtract from the entropy
counter, and it can trigger a warning:

random: negative entropy/overflow: pool input count -40000
------------[ cut here ]------------
WARNING: CPU: 3 PID: 6828 at drivers/char/random.c:670[<      none
 >] credit_entropy_bits+0x21e/0xad0 drivers/char/random.c:670
Modules linked in:
CPU: 3 PID: 6828 Comm: a.out Not tainted 4.7.0-rc4+ #4
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 ffffffff880b58e0 ffff88005dd9fcb0 ffffffff82cc838f ffffffff87158b40
 fffffbfff1016b1c 0000000000000000 0000000000000000 ffffffff87158b40
 ffffffff83283dae 0000000000000009 ffff88005dd9fcf8 ffffffff8136d27f
Call Trace:
 [<     inline     >] __dump_stack lib/dump_stack.c:15
 [<ffffffff82cc838f>] dump_stack+0x12e/0x18f lib/dump_stack.c:51
 [<ffffffff8136d27f>] __warn+0x19f/0x1e0 kernel/panic.c:516
 [<ffffffff8136d48c>] warn_slowpath_null+0x2c/0x40 kernel/panic.c:551
 [<ffffffff83283dae>] credit_entropy_bits+0x21e/0xad0 drivers/char/random.c:670
 [<     inline     >] credit_entropy_bits_safe drivers/char/random.c:734
 [<ffffffff8328785d>] random_ioctl+0x21d/0x250 drivers/char/random.c:1546
 [<     inline     >] vfs_ioctl fs/ioctl.c:43
 [<ffffffff8185316c>] do_vfs_ioctl+0x18c/0xff0 fs/ioctl.c:674
 [<     inline     >] SYSC_ioctl fs/ioctl.c:689
 [<ffffffff8185405f>] SyS_ioctl+0x8f/0xc0 fs/ioctl.c:680
 [<ffffffff86a995c0>] entry_SYSCALL_64_fastpath+0x23/0xc1
arch/x86/entry/entry_64.S:207
---[ end trace 5d4902b2ba842f1f ]---

This was triggered using the test program:

// autogenerated by syzkaller (http://github.com/google/syzkaller)

int main() {
        int fd = open("/dev/random", O_RDWR);
        int val = -5000;
        ioctl(fd, RNDADDTOENTCNT, &val);
        return 0;
}

It's harmless in that (a) only root can trigger it, and (b) after
complaining the code never does let the entropy count go negative, but
it's better to simply not allow this userspace from passing in a
negative entropy value altogether.

Google-Bug-Id: #29575089
Reported-By: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
drivers/char/random.c

index 783dee11cdc97626d0b38a2534b128bb3db98885..8d0af74f656987f2d1be560e336962e2d7badf20 100644 (file)
@@ -738,15 +738,18 @@ retry:
        }
 }
 
-static void credit_entropy_bits_safe(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));
 
+       if (nbits < 0)
+               return -EINVAL;
+
        /* Cap the value to avoid overflows */
        nbits = min(nbits,  nbits_max);
-       nbits = max(nbits, -nbits_max);
 
        credit_entropy_bits(r, nbits);
+       return 0;
 }
 
 /*********************************************************************
@@ -1823,8 +1826,7 @@ static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
                        return -EPERM;
                if (get_user(ent_count, p))
                        return -EFAULT;
-               credit_entropy_bits_safe(&input_pool, ent_count);
-               return 0;
+               return credit_entropy_bits_safe(&input_pool, ent_count);
        case RNDADDENTROPY:
                if (!capable(CAP_SYS_ADMIN))
                        return -EPERM;
@@ -1838,8 +1840,7 @@ static long random_ioctl(struct file *f, unsigned int cmd, unsigned long arg)
                                    size);
                if (retval < 0)
                        return retval;
-               credit_entropy_bits_safe(&input_pool, ent_count);
-               return 0;
+               return credit_entropy_bits_safe(&input_pool, ent_count);
        case RNDZAPENTCNT:
        case RNDCLEARPOOL:
                /*