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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <159722125596.33733.17725649536425524344@swboyd.mtv.corp.google.com>
Date:   Wed, 12 Aug 2020 01:34:15 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     Felix.Kuehling@....com, Qianli Zhao <zhaoqianligood@...il.com>,
        akpm@...ux-foundation.org, axboe@...nel.dk, tglx@...utronix.de
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 Qianli Zhao (2020-08-11 22:14:14)
> @@ -115,7 +125,7 @@ struct kthread_delayed_work {
>         }
>  
>  #define KTHREAD_WORK_INIT(work, fn)    {                               \
> -       .node = LIST_HEAD_INIT((work).node),                            \
> +       .node = { .next = KWORK_ENTRY_STATIC },                         \
>         .func = (fn),                                                   \
>         }
>  
> diff --git a/include/linux/poison.h b/include/linux/poison.h
> index df34330..2e6a370 100644
> --- a/include/linux/poison.h
> +++ b/include/linux/poison.h
> @@ -86,4 +86,7 @@
>  /********** security/ **********/
>  #define KEY_DESTROY            0xbd
>  
> +/********** 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.

> +
>  #endif
> diff --git a/kernel/kthread.c b/kernel/kthread.c
> index 132f84a..ca00bd2 100644
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -698,6 +786,7 @@ int kthread_worker_fn(void *worker_ptr)
>                 work = list_first_entry(&worker->work_list,
>                                         struct kthread_work, node);
>                 list_del_init(&work->node);
> +               debug_kwork_deactivate(work);

Shouldn't this come before the list operation so that any sort of fix
can be made before possibly corrupting a list?

>         }
>         worker->current_work = work;
>         raw_spin_unlock_irq(&worker->lock);
> @@ -835,8 +924,11 @@ static void kthread_insert_work(struct kthread_worker *worker,
>  
>         list_add_tail(&work->node, pos);
>         work->worker = worker;
> -       if (!worker->current_work && likely(worker->task))
> +
> +       if (!worker->current_work && likely(worker->task)) {
> +               debug_kwork_activate(work);
>                 wake_up_process(worker->task);
> +       }
>  }
>  
>  /**
> @@ -1054,6 +1146,7 @@ static bool __kthread_cancel_work(struct kthread_work *work, bool is_dwork,
>          */
>         if (!list_empty(&work->node)) {
>                 list_del_init(&work->node);
> +               debug_kwork_deactivate(work);

Same comment.

>                 return true;
>         }