]> git.proxmox.com Git - mirror_ubuntu-kernels.git/commitdiff
idr: fix invalid ptr dereference on item delete
authorMatthew Wilcox <mawilcox@microsoft.com>
Fri, 25 May 2018 21:47:24 +0000 (14:47 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Sat, 26 May 2018 01:12:10 +0000 (18:12 -0700)
If the radix tree underlying the IDR happens to be full and we attempt
to remove an id which is larger than any id in the IDR, we will call
__radix_tree_delete() with an uninitialised 'slot' pointer, at which
point anything could happen.  This was easiest to hit with a single
entry at id 0 and attempting to remove a non-0 id, but it could have
happened with 64 entries and attempting to remove an id >= 64.

Roman said:

  The syzcaller test boils down to opening /dev/kvm, creating an
  eventfd, and calling a couple of KVM ioctls. None of this requires
  superuser. And the result is dereferencing an uninitialized pointer
  which is likely a crash. The specific path caught by syzbot is via
  KVM_HYPERV_EVENTD ioctl which is new in 4.17. But I guess there are
  other user-triggerable paths, so cc:stable is probably justified.

Matthew added:

  We have around 250 calls to idr_remove() in the kernel today. Many of
  them pass an ID which is embedded in the object they're removing, so
  they're safe. Picking a few likely candidates:

  drivers/firewire/core-cdev.c looks unsafe; the ID comes from an ioctl.
  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c is similar
  drivers/atm/nicstar.c could be taken down by a handcrafted packet

Link: http://lkml.kernel.org/r/20180518175025.GD6361@bombadil.infradead.org
Fixes: 0a835c4f090a ("Reimplement IDR and IDA using the radix tree")
Reported-by: <syzbot+35666cba7f0a337e2e79@syzkaller.appspotmail.com>
Debugged-by: Roman Kagan <rkagan@virtuozzo.com>
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
lib/radix-tree.c
tools/testing/radix-tree/idr-test.c

index 43e0cbedc3a0ce1adf0d7807b623c9f8b60f3e53..a9e41aed6de4bfb063af9d16d41942a041a23fdd 100644 (file)
@@ -2034,10 +2034,12 @@ void *radix_tree_delete_item(struct radix_tree_root *root,
                             unsigned long index, void *item)
 {
        struct radix_tree_node *node = NULL;
-       void __rcu **slot;
+       void __rcu **slot = NULL;
        void *entry;
 
        entry = __radix_tree_lookup(root, index, &node, &slot);
+       if (!slot)
+               return NULL;
        if (!entry && (!is_idr(root) || node_tag_get(root, node, IDR_FREE,
                                                get_slot_offset(node, slot))))
                return NULL;
index 6c645eb77d4218fd102968fb2a24b19350e3edd9..ee820fcc29b0d3c201cbd63e56daa2392e89bcbf 100644 (file)
@@ -252,6 +252,13 @@ void idr_checks(void)
        idr_remove(&idr, 3);
        idr_remove(&idr, 0);
 
+       assert(idr_alloc(&idr, DUMMY_PTR, 0, 0, GFP_KERNEL) == 0);
+       idr_remove(&idr, 1);
+       for (i = 1; i < RADIX_TREE_MAP_SIZE; i++)
+               assert(idr_alloc(&idr, DUMMY_PTR, 0, 0, GFP_KERNEL) == i);
+       idr_remove(&idr, 1 << 30);
+       idr_destroy(&idr);
+
        for (i = INT_MAX - 3UL; i < INT_MAX + 1UL; i++) {
                struct item *item = item_create(i, 0);
                assert(idr_alloc(&idr, item, i, i + 10, GFP_KERNEL) == i);