Re: [RFC] kmemcheck: trap uses of uninitialized memory (v2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Vegard Nossum wrote:
General description: kmemcheck will trap every read and write to memory that was allocated dynamically (ie. with kmalloc()). If a memory address is read that has
not previously been written to, a message is printed to the kernel log.

<snip>
diff --git a/arch/x86/kernel/kmemcheck_32.c b/arch/x86/kernel/kmemcheck_32.c
new file mode 100644
index 0000000..9d065b9
--- /dev/null
+++ b/arch/x86/kernel/kmemcheck_32.c
@@ -0,0 +1,290 @@
+#include <linux/kernel.h>
+#include <linux/kallsyms.h>
+#include <linux/mm.h>
+#include <asm/cacheflush.h>
+#include <asm/kmemcheck.h>
+#include <asm/tlbflush.h>
+
+static int
Not 'static bool'?
+page_is_tracked(struct page *page)
+{
+    struct page *head;
+
+    if (!page)
+        return 0;
+    head = compound_head(page);
+    if (!head)
+        return 0;
+    if (!(head->flags & (1 << PG_slab)))
+        return 0;
+    if (!head->slab)
+        return 0;
+    if (head->slab->flags & __GFP_NOTRACK)
+        return 0;
+    return 1;
Why not returning 'false' and 'true'?
+}
+
+static void
+show_addr(uint32_t addr)
+{
+    struct page *page;
+    pte_t *pte;
+
+    if (!addr)
+        return;
+    page = virt_to_page(addr);
+    if (!page_is_tracked(page))
+        return;
+
+    pte = lookup_address(addr);
+    change_page_attr(page, 1, __pgprot(pte->pte_low | _PAGE_VISIBLE));
+    __flush_tlb_one(addr);
+}
+
+static void
+hide_addr(uint32_t addr)
+{
+    struct page *page;
+    pte_t *pte;
+
+    if (!addr)
+        return;
+    page = virt_to_page(addr);
+    if (!page_is_tracked(page))
+        return;
+    pte = lookup_address(addr);
+    change_page_attr(page, 1, __pgprot(pte->pte_low & ~_PAGE_VISIBLE));
+    __flush_tlb_one(addr);
+}
+
+DEFINE_PER_CPU(uint32_t, kmemcheck_read_addr);
+DEFINE_PER_CPU(uint32_t, kmemcheck_write_addr);
+
+void
+kmemcheck_show(struct pt_regs *regs)
+{
+    show_addr(__get_cpu_var(kmemcheck_read_addr));
+    show_addr(__get_cpu_var(kmemcheck_write_addr));
+    regs->eflags |= TF_MASK;
+}
+
+void
+kmemcheck_hide(struct pt_regs *regs)
+{
+    hide_addr(__get_cpu_var(kmemcheck_read_addr));
+    hide_addr(__get_cpu_var(kmemcheck_write_addr));
+    regs->eflags &= ~TF_MASK;
+}
+
+void
+kmemcheck_hide_pages(struct page *p, unsigned int n)
+{
+    unsigned int i;
+
+    for(i = 0; i < n; ++i) {
+        unsigned long address = (unsigned long) page_address(&p[i]);
+        pte_t *pte = lookup_address(address);
+
+        change_page_attr(&p[i], 1,
+            __pgprot(pte->pte_low & ~_PAGE_VISIBLE));
+        __flush_tlb_one(address);
+    }
+}
+
+static int
'static bool'?
+opcode_is_prefix(uint8_t b)
+{
+    return
+        /* Group 1 */
+        b == 0xf0 || b == 0xf2 || b == 0xf3
+        /* Group 2 */
+        || b == 0x2e || b == 0x36 || b == 0x3e || b == 0x26
+        || b == 0x64 || b == 0x65 || b == 0x2e || b == 0x3e
+        /* Group 3 */
+        || b == 0x66
+        /* Group 4 */
+        || b == 0x67;
+}
+
+/* This is a VERY crude opcode decoder. We only need to find the size of the + * load/store that caused our #PF and this should work for all the opcodes + * that we care about. Moreover, the ones who invented this instruction set
+ * should be shot. */
+static unsigned int
+opcode_get_size(const uint8_t *opcode)
Are we not using 'u8' in the kernel?
+{
+    const uint8_t *i;
and here. Also, I find the name 'i' a bit confusing in this context, since it is not really a counter. What about 'cur', 'prefix_op' or something of the sort?
+
+    /* Default operand size */
+    int operand_size_override = 32;
+
+    /* prefixes */
+    for (i = opcode; opcode_is_prefix(*i); ++i) {
+        if (*i == 0x66)
+            operand_size_override = 16;
+    }
+
+    /* escape opcode */
+    if (*i == 0x0f) {
+        ++i;
+
+        if(*i == 0xb6)
Please do, as the previous, 'if ()'. A good checker for these kind of things is the 'scripts/checkpatch.pl'...
+            return operand_size_override >> 1;
+        if(*i == 0xb7)
+            return 16;
+    }
+
+    return (*i & 1) ? operand_size_override : 8;
+}
+
+static uint8_t
and here...
+opcode_get_primary(const uint8_t *opcode)
and here..
+{
+    const uint8_t *i;
and here. Also, I find the name 'i' a bit confusing in this context, since it is not really a counter. What about 'cur', 'prim_op', 'prefix_op' or something of the sort?
+
+    /* skip prefixes */
+    for (i = opcode; opcode_is_prefix(*i); ++i);
+    return *i;
+}
+
+static void *address_get_shadow_slab(unsigned long address,
+    struct page *head)
+{
+    if (!head->slab)
+        return NULL;
+
+    if (head->slab->flags & __GFP_NOTRACK)
+        return NULL;
+
+    return (void *) address + (PAGE_SIZE << head->slab->order);
+}
+
+static void *address_get_shadow(unsigned long address)
+{
+    struct page *page = virt_to_page(address);
+    struct page *head = compound_head(page);
+
+    if (!head)
+        return NULL;
+
+    if (head->flags & (1 << PG_slab))
+        return address_get_shadow_slab(address, head);
+
+    return NULL;
+}
+
+static int
'static bool'?
+test(void *shadow, unsigned int size)
+{
+    switch (size) {
+    case 8:
+        return *(uint8_t *) shadow == 0xff;
+    case 16:
+        return *(uint16_t *) shadow == 0xffff;
+    case 32:
+        return *(uint32_t *) shadow == 0xffffffff;
+    }
+
+    BUG();
+    return 0;
'return false;'?
+}
+
<snip>

cu
Richard Knutsson

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux