linux-kernel - Re: [PATCH 07/13] kmemcheck: add the kmemcheck core

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <19f34abd0807171623j15e2e940rac790567f5874a87@mail.gmail.com>
Date:	Fri, 18 Jul 2008 01:23:41 +0200
From:	"Vegard Nossum" <vegard.nossum@...il.com>
To:	"Randy Dunlap" <rdunlap@...otime.net>
Cc:	"Andrew Morton" <akpm@...ux-foundation.org>,
	"Ingo Molnar" <mingo@...e.hu>,
	"Pekka Enberg" <penberg@...helsinki.fi>,
	"Peter Zijlstra" <a.p.zijlstra@...llo.nl>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 07/13] kmemcheck: add the kmemcheck core

On Thu, Jul 17, 2008 at 11:49 PM, Randy Dunlap <rdunlap@...otime.net> wrote:
> On Wed, 16 Jul 2008 02:23:29 +0200 Vegard Nossum wrote:
>
>> >From fcd8f514a8962ea22aafb831b9f22a2ea1a13870 Mon Sep 17 00:00:00 2001
>
> eh?

Uhm, probably relative to my previous patch? About the date, I have no
idea. All my "git format-patch" output looks like this.

>> +++ b/arch/x86/mm/kmemcheck/error.h
>> @@ -0,0 +1,15 @@
>> +#ifndef ARCH__X86__MM__KMEMCHECK__ERROR_H
>> +#define ARCH__X86__MM__KMEMCHECK__ERROR_H
>
> We don't usually use double __ here.

But it's such a nice convention! How can we differentiate between
mm/types.h and mm_types.h!

>> +void *kmemcheck_memset(void *s, int c, size_t n)
>> +{
>> +     unsigned long addr;
>> +     unsigned long start_page, start_offset;
>> +     unsigned long end_page, end_offset;
>> +     unsigned long i;
>> +
>> +     if (!n)
>> +             return s;
>> +
>> +     if (!slab_is_available()) {
>> +             __memset(s, c, n);
>> +             return s;
>> +     }
>> +
>> +     addr = (unsigned long) s;
>> +
>> +     start_page = addr & PAGE_MASK;
>> +     end_page = (addr + n) & PAGE_MASK;
>> +
>> +     if (start_page == end_page) {
>> +             /* The entire area is within the same page. Good, we only
>> +              * need one memset(). */
>> +             memset_one_page(s, c, n);
>> +             return s;
>> +     }
>> +
>> +     start_offset = addr & ~PAGE_MASK;
>> +     end_offset = (addr + n) & ~PAGE_MASK;
>> +
>> +     /* Clear the head, body, and tail of the memory area. */
>> +     if (start_offset < PAGE_SIZE)
>> +             memset_one_page(s, c, PAGE_SIZE - start_offset);
>> +     for (i = start_page + PAGE_SIZE; i < end_page; i += PAGE_SIZE)
>> +             memset_one_page((void *) i, c, PAGE_SIZE);
>> +     if (end_offset > 0)
>> +             memset_one_page((void *) end_page, c, end_offset);
>> +
>> +     return s;
>> +}
>> +
>> +EXPORT_SYMBOL(kmemcheck_memset);
>
> We would prefer to have kernel-doc on exported functions...

Oops. I will not add a kernel-doc, but I will add an explanation.

This function is a drop-in replacement for memset, and the reason for
the EXPORT is that the real memset() in fact is just a macro that
calls this function. And memset is needed in modules. This function
should not actually have any users beside the memset() in x86 headers.

Thanks for mini-review! (Does this mean the rest was okay? :-D)


Vegard

-- 
"The animistic metaphor of the bug that maliciously sneaked in while
the programmer was not looking is intellectually dishonest as it
disguises that the error is the programmer's own creation."
	-- E. W. Dijkstra, EWD1036
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/