lkml.org 
[lkml]   [2018]   [May]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 022/496] idr: fix invalid ptr dereference on item delete
    Date
    4.14-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Matthew Wilcox <mawilcox@microsoft.com>

    commit 7a4deea1aa8bddfed4ef1b35fc2b6732563d8ad5 upstream.

    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>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    lib/radix-tree.c | 4 +++-
    tools/testing/radix-tree/idr-test.c | 7 +++++++
    2 files changed, 10 insertions(+), 1 deletion(-)

    --- a/lib/radix-tree.c
    +++ b/lib/radix-tree.c
    @@ -2037,10 +2037,12 @@ void *radix_tree_delete_item(struct radi
    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;
    --- a/tools/testing/radix-tree/idr-test.c
    +++ b/tools/testing/radix-tree/idr-test.c
    @@ -202,6 +202,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);

    \
     
     \ /
      Last update: 2018-05-28 12:42    [W:4.035 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site