linux-kernel - Re: [RFC V2] kthread: add object debug support

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 for Android: free password hash cracker in your pocket

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

Message-ID: <159748064613.2270147.9288160745183605164@swboyd.mtv.corp.google.com>
Date:   Sat, 15 Aug 2020 01:37:26 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     Felix.Kuehling@....com, Qianli Zhao <zhaoqianligood@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        akpm@...ux-foundation.org, axboe@...nel.dk
Cc:     john.stultz@...aro.org, ben.dooks@...ethink.co.uk,
        bfields@...hat.com, cl@...k-chips.com,
        linux-kernel@...r.kernel.org, zhaoqianli@...omi.com
Subject: Re: [RFC V2] kthread: add object debug support

Quoting Thomas Gleixner (2020-08-12 03:27:03)
> Stephen,
> 
> Stephen Boyd <sboyd@...nel.org> writes:
> > Quoting Qianli Zhao (2020-08-11 22:14:14)
> >> +/********** kernel/kthread **********/
> >> +#define KWORK_ENTRY_STATIC     ((void *) 0x600 + POISON_POINTER_DELTA)
> >
> > Is this related to the debugobjects change here? It looks like another
> > version of list poison.
> 
> Yes, it is. We use these poison entries to mark statically allocated
> objects. debug objects does not know about statically initialized
> objects up to the point where they are used (activated).
> 
> That means the object state lookup will fail which causes debugobjects
> to complain about using an uninitialized object. But in case of static
> initialized ones that's a false positive. So we mark these objects in
> their list head (or some other appropriate place) with a poison value
> and in case of a failed lookup debug object does:
> 
>         if (descr->is_static_object && descr->is_static_object(addr)) {
>                 /* track this static object */
>                 debug_object_init(addr, descr);
>                 debug_object_activate(addr, descr);
>         }        
> 
> The object specific is_static_object() callback will then check for the
> magic list poison value being present:

Thanks! I missed this function below.

> 
> > +static bool kwork_is_static_object(void *addr)
> > +{
> > +     struct kthread_work *kwork = addr;
> > +
> > +     return (kwork->node.prev == NULL &&
> > +             kwork->node.next == KWORK_ENTRY_STATIC);
> > +}
> 
> and if so the debug object core fixes its internal state by creating a
> tracking object and then activating it.
> 
> It's not a perfect "yes this is statically initialized" check but good
> enough. If you go and do:
> 
>    work = kzalloc(sizeof(*work);
>    work->node.next = KWORK_ENTRY_STATIC;
> 
>    kthread_insert_work(worker, work);
> 
> or any other variant of insanity which makes the check claim that this
> is statically initialized then you rightfully can keep the pieces :)

Makes sense. Maybe this "technique" should be documented in
Documentation/core-api/debug-objects.rst? I can cook up a patch to add
is_static_object() to the Fixup functions section.