ucsb-seclab / dr_checker

DR.CHECKER : A Soundy Vulnerability Detection Tool for Linux Kernel Drivers
BSD 2-Clause "Simplified" License
329 stars 71 forks source link

How to add arm32 support? #26

Closed leiwen83 closed 5 years ago

leiwen83 commented 5 years ago

I see ARCH64 is supported in the docker. How could I check arm32 kernel? LLVM complain for "does not support '-march=armv7-a". Do I need to reconfig the LLVM? Do you have recommend steps to do the configuration?

Machiry commented 5 years ago

You should provide -n 1 flag to the run_all.py script. Can you give me more details on which kernel you are trying to compile and how?

leiwen83 commented 5 years ago

The kernel I am try to compile comes from rockchip but not mainlined yet. After change to "-n 1", things goes fine now.

There are some other error warning during helper_scripts runs: clang-3.8: error: the clang compiler does not support '-Wa,-mfpu=softvfp+vfp' clang-3.8: error: the clang compiler does not support '-Wa,-mfpu=softvfp+vfp'

clang-3.8: error: the clang compiler does not support '-Wa,-march=armv7-a+sec'

:40:47: error: too many positional arguments str8w r0, r3, r4, r5, r6, r7, r8, r9, ip, , abort=19f /dockershare/guo4/kernel/out/arch/arm/lib/io-readsb.S:118:3: error: invalid instruction strgeb r3, [r1], #1 Those error seems not affect the final result comes out, I am not sure whether it should be fixed or not. There is additional problem for me, when I try to visualize the result following the guide, I could see one web page comes out, but there is no following more details showing even after waiting for very long time.
Machiry commented 5 years ago

The compilation errors for .S files are expected. They should not affect the analysis. For the visualization, if you have followed all the instruction as listed in: https://github.com/ucsb-seclab/dr_checker/blob/speedy/docs/docker.md#visualizing-the-results ? Then could you check if the path given to RESULTS_DIR contains any JSON files?

It would be much easier to debug, if you can share the rockchip kernel you are using.

leiwen83 commented 5 years ago

The rockchip kernel is current unavaible to the public... While it use similar gpu driver as mtk online: https://android.googlesource.com/kernel/mediatek/+/android-mtk-3.18/drivers/misc/mediatek/gpu/gpu_mali/mali_utgard/mali/mali/linux/mali_kernel_sysfs.c?autodive=0%2F%2F%2F%2F%2F%2F%2F

In this file, drcheck report "Trying to write to a user pointer." "warn_data": { "at": " store i8 0, i8 %arrayidx, align 1, !dbg !92353", "at_file": "/drivers/gpu/arm/mali400/mali/linux/mali_kernel_sysfs.c", "at_func": "power_always_on_write", "at_line": 492, "by": "TaintedPointerDereferenceChecker says:", "inst_trace": [ { "instr": " %val = alloca i32, align 4", "instr_func": "power_always_on_write", "instr_loc": -1 }, { "instr": " store i32 %cnt, i32 %_min1, align 4, !dbg !92306", "instr_file": "/drivers/gpu/arm/mali400/mali/linux/mali_kernel_sysfs.c", "instr_func": "power_always_on_write", "instr_loc": 488 }, { "instr": " %2 = load i32, i32 %_min1, align 4, !dbg !92308", "instr_file": "drivers/gpu/arm/mali400/mali/linux/mali_kernel_sysfs.c", "instr_func": "power_always_on_write", "instr_loc": 488 }, { "instr": " %cond = phi i32 [ %2, %cond.true ], [ %3, %cond.false ], !dbg !92312", "instr_file": "//drivers/gpu/arm/mali400/mali/linux/mali_kernel_sysfs.c", "instr_func": "power_always_on_write", "instr_loc": 488 }, { "instr": " %arrayidx = getelementptr [32 x i8], [32 x i8] %buf, i32 0, i32 %cond, !dbg !92352", "instr_file": "/drivers/gpu/arm/mali400/mali/linux/mali_kernel_sysfs.c", "instr_func": "power_always_on_write", "instr_loc": 492 } ], "warn_str": "Trying to write to a user pointer." },

But I don;t see any possible for the user pointer write .. As the cnt is limited with buf's space already...

static ssize_t power_always_on_write(struct file filp, const char __user ubuf, size_t cnt, loff_t *ppos) { unsigned long val; int ret; char buf[32];

cnt = min(cnt, sizeof(buf) - 1);
if (copy_from_user(buf, ubuf, cnt)) {
    return -EFAULT;
}
buf[cnt] = '\0';

ret = kstrtoul(buf, 10, &val);
if (0 != ret) {
    return ret;
Machiry commented 5 years ago

It is because, cnt could potentially come from user space and you are using it as index into buf, which could a potential arbitrary memory write.

Note that, DR.CHECKER as of now does not support untainting and it is flow-sensitive i.e., merge all the values across different paths.

In short, DR.CHECKER does not know that cnt is vetted before.