linux-kernel - Re: [PATCH] kthread: add kthread_mod_pending_delayed

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: <YCp288w57eYnjPl0@alley>
Date:   Mon, 15 Feb 2021 14:28:19 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Yiwei Zhang <zzyiwei@...roid.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Felix Kuehling <Felix.Kuehling@....com>,
        Jens Axboe <axboe@...nel.dk>,
        "J. Bruce Fields" <bfields@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Frederic Weisbecker <frederic@...nel.org>,
        Marcelo Tosatti <mtosatti@...hat.com>,
        Ilias Stamatis <stamatis.iliass@...il.com>,
        Rob Clark <robdclark@...omium.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Liang Chen <cl@...k-chips.com>, linux-kernel@...r.kernel.org,
        kernel-team@...roid.com
Subject: Re: [PATCH] kthread: add kthread_mod_pending_delayed_work api

On Sun 2021-02-14 00:06:11, Yiwei Zhang wrote:
> The existing kthread_mod_delayed_work api will queue a new work if
> failing to cancel the current work due to no longer being pending.
> However, there's a case that the same work can be enqueued from both
> an async request and a delayed work, and a racing could happen if the
> async request comes right after the timeout delayed work gets
> scheduled,

By other words, you want to modify the delayed work only when
it is still waiting in the queue. You do not want to queue new
work when it has not been already queued. Do I get it correctly?

Could you please provide a patch where the new API is used?

> because the clean up work may not be safe to run twice.

This looks like a bad design of the code. There is likely
another race that might break it. You should ask the following
questions:

Why anyone tries to modify the clean up work when it has been already
queued? There should be only one location/caller that triggers the clean up.

Could anyone queue any work to the workqueue after the clean up
work was queued? The cleanup work should be the last queued one.
The workqueue user must inform all other users that the queue
is being destroyed and nobody is allowed to queue any work
any longer.

Best Regards,
Petr