lkml.org 
[lkml]   [2018]   [Jul]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] radix-tree: avoid NULL dereference
Date
When idr_alloc() is called for the first time on an IDR (which has no
nodes in its radix tree), we end up with calculate_count() calling
get_slot_offset() with a NULL node, leading to a NULL pointer
dereference caught by UBSAN:

================================================================================
UBSAN: Undefined behaviour in lib/radix-tree.c:123:14
member access within null pointer of type 'const struct radix_tree_node'
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.18.0-rc3+ #14
Hardware name: linux,dummy-virt (DT)
Call trace:
dump_backtrace+0x0/0x458
show_stack+0x20/0x30
dump_stack+0x18c/0x248
ubsan_epilogue+0x18/0x94
handle_null_ptr_deref+0x1d4/0x228
__ubsan_handle_type_mismatch_v1+0x188/0x1e0
__radix_tree_replace+0x29c/0x2a0
radix_tree_iter_replace+0x58/0x88
idr_alloc_u32+0x240/0x340
idr_alloc+0xe8/0x188
worker_pool_assign_id+0x74/0x128
workqueue_init_early+0x588/0xae4
start_kernel+0x54c/0xb9c
================================================================================

The result of the load is passed into node_tag_get(), which ignores the
value when node is NULL. Typically, the compiler inlines both
get_slot_offset() and node_tag_get() into calculate_count(), optimizing
away the NULL-pointer dereference, and hence this doesn't typically
result in a boot failure.

We can't rely on the compiler always doing this, and must avoid
dereferencing fields from node when it is potentially NULL.

To do so, this patch folds the generation of offset into tag_get(), such
that this only happens when node is not NULL. Callers are updated to
pass the relevant slot, rather than the offset derived from it.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Matthew Wilcox <mawilcox@microsoft.com>
Cc: valdis.kletnieks@vt.edu
---
lib/radix-tree.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

I beleive this is what Valdis hit [1] back in March. I spotted this while
booting an arm64 machine.

Mark.

[1] https://lkml.kernel.org/r/22378.1520883323@turing-police.cc.vt.edu

diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index a9e41aed6de4..4c6f409582cb 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -1137,10 +1137,10 @@ static void replace_slot(void __rcu **slot, void *item,

static bool node_tag_get(const struct radix_tree_root *root,
const struct radix_tree_node *node,
- unsigned int tag, unsigned int offset)
+ unsigned int tag, void __rcu **slot)
{
if (node)
- return tag_get(node, tag, offset);
+ return tag_get(node, tag, get_slot_offset(node, slot));
return root_tag_get(root, tag);
}

@@ -1156,8 +1156,7 @@ static int calculate_count(struct radix_tree_root *root,
void *item, void *old)
{
if (is_idr(root)) {
- unsigned offset = get_slot_offset(node, slot);
- bool free = node_tag_get(root, node, IDR_FREE, offset);
+ bool free = node_tag_get(root, node, IDR_FREE, slot);
if (!free)
return 0;
if (!old)
@@ -2041,7 +2040,7 @@ void *radix_tree_delete_item(struct radix_tree_root *root,
if (!slot)
return NULL;
if (!entry && (!is_idr(root) || node_tag_get(root, node, IDR_FREE,
- get_slot_offset(node, slot))))
+ slot)))
return NULL;

if (item && entry != item)
--
2.11.0
\
 
 \ /
  Last update: 2018-07-06 15:42    [W:0.053 / U:0.520 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site