linux-kernel - Re: [PATCH v10 06/17] mm: slub: introduce metadata_access_enable()/metadata_access

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: <CAPAsAGxPOC7j9Z=xjtobcdMEgGJB0L+drO1m4TWBgkNNfZWmSw@mail.gmail.com>
Date:	Sat, 31 Jan 2015 03:11:55 +0400
From:	Andrey Ryabinin <ryabinin.a.a@...il.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Andrey Ryabinin <a.ryabinin@...sung.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Dmitry Vyukov <dvyukov@...gle.com>,
	Konstantin Serebryany <kcc@...gle.com>,
	Dmitry Chernenkov <dmitryc@...gle.com>,
	Andrey Konovalov <adech.fo@...il.com>,
	Yuri Gribov <tetra2005@...il.com>,
	Konstantin Khlebnikov <koct9i@...il.com>,
	Sasha Levin <sasha.levin@...cle.com>,
	Christoph Lameter <cl@...ux.com>,
	Joonsoo Kim <iamjoonsoo.kim@....com>,
	Dave Hansen <dave.hansen@...el.com>,
	Andi Kleen <andi@...stfloor.org>,
	"x86@...nel.org" <x86@...nel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	Pekka Enberg <penberg@...nel.org>,
	David Rientjes <rientjes@...gle.com>
Subject: Re: [PATCH v10 06/17] mm: slub: introduce metadata_access_enable()/metadata_access_disable()

2015-01-31 0:42 GMT+03:00 Andrew Morton <akpm@...ux-foundation.org>:
> On Fri, 30 Jan 2015 20:05:13 +0300 Andrey Ryabinin <a.ryabinin@...sung.com> wrote:
>
>> >> --- a/mm/slub.c
>> >> +++ b/mm/slub.c
>> >> @@ -467,13 +467,23 @@ static int slub_debug;
>> >>  static char *slub_debug_slabs;
>> >>  static int disable_higher_order_debug;
>> >>
>> >> +static inline void metadata_access_enable(void)
>> >> +{
>> >> +}
>> >> +
>> >> +static inline void metadata_access_disable(void)
>> >> +{
>> >> +}
>> >
>> > Some code comments here would be useful.  What they do, why they exist,
>> > etc.  The next patch fills them in with
>> > kasan_disable_local/kasan_enable_local but that doesn't help the reader
>> > to understand what's going on.  The fact that
>> > kasan_disable_local/kasan_enable_local are also undocumented doesn't
>> > help.
>> >
>>
>> Ok, How about this?
>>
>> /*
>>  * This hooks separate payload access from metadata access.
>>  * Useful for memory checkers that have to know when slub
>>  * accesses metadata.
>>  */
>
> "These hooks".
>
> I still don't understand :( Maybe I'm having a more-stupid-than-usual
> day.

I think it's me being stupid today ;) I'll try to explain better.

> How can a function "separate access"?  What does this mean?  More
> details, please.  I think I've only once seen a comment which had too
> much info!
>

slub could access memory marked by kasan as inaccessible (object's metadata).
Kasan shouldn't print report in that case because this access is valid.
Disabling instrumentation of slub.c code is not enough to achieve this
because slub passes pointer to object's metadata into memchr_inv().

We can't disable instrumentation for memchr_inv() because this is quite
generic function.

So metadata_access_enable/metadata_access_disable wrap some
places in slub.c where access to object's metadata starts/end.
And kasan_disable_local/kasan_enable_local just disable/enable
error reporting in this places.
--
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/