NULL pointer dereference in patch modules with multiple objects #494
Description
A patch module will crash due to null pointer dereference in cases where it contains patches to multiple objects (example: a patch module built from patches changing vmlinux and ext4)
This happens in patch_find_object_by_name() in kmod/patch/livepatch-patch-hook.c. While we're processing each function in __kpatch_funcs, if we've already created a vmlinux patch_object and we come across a function from another object (say, ext4) then we'll end up dereferencing a null pointer when we try to strcmp object->name and the passed in name argument. Here are the offending lines from patch_find_object_by_name():
static struct patch_object *patch_find_object_by_name(const char *name) {
[ snip ]
list_for_each_entry(object, &patch_objects, list) {
if ((!strcmp(name, "vmlinux") && !object->name) ||
!strcmp(object->name, name)) // Crash here
return object;
}
So order here was important to trigger the bug. Say no patch_objects have been allocated yet. We process a vmlinux function first and a vmlinux patch_object is alloc'd and added to &patch_objects. Then we come across an ext4 function next and when we enter list_for_each_entry, name obviously doesn't match vmlinux so that case fails, but object->name will be NULL because object->name is not set for the vmlinux patch_object (see patch_alloc_new_object), and strcmp crashes. Only module patch_objects get name fields. Had we processed the kpatch funcs the other order around, this bug wouldn't have triggered since the ext4 object would have been allocated first and that does have a name field.
I think the best fix is to just have the vmlinux patch object to have its object->name field filled in, that way we don't even risk having a null pointer to begin with.
Stacktrace:
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [<ffffffff812c81e8>] strcmp+0x18/0x40
[snip]
Call Trace:
[<ffffffffa032f0af>] patch_init+0xaf/0x1000 [kpatch_multiple_objnames]
[<ffffffff81002128>] do_one_initcall+0xb8/0x200
[<ffffffff8119f1d2>] ? __vunmap+0xa2/0x100
[<ffffffff811bc916>] ? kfree+0x176/0x180
[<ffffffff8157536b>] do_init_module+0x60/0x1cb
[snip...]
Sample patch to reproduce bug:
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 2faaa29..3c61be8 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2574,6 +2574,7 @@ static void *s_next(struct seq_file *m, void *p, loff_t *pos)
{
struct vmap_area *va = p, *next;
+ pr_info("s_next");
++*pos;
next = list_entry(va->list.next, typeof(*va), list);
if (&next->list != &vmap_area_list)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ca9d4a2..d803b46 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2480,6 +2480,7 @@ static ssize_t lifetime_write_kbytes_show(struct ext4_attr *a,
struct ext4_sb_info *sbi, char *buf)
{
struct super_block *sb = sbi->s_buddy_cache->i_sb;
+ pr_info("ext4: lifetime_write_kbytes_show");
if (!sb->s_bdev->bd_part)
return snprintf(buf, PAGE_SIZE, "0\n");