linux-kernel - Re: [PATCH v2 (REPOST)] rapidio/tsi721: Replace flush_scheduled_work() with flush

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] [day] [month] [year] [list]

Message-ID: <ce9a2543-84eb-a7e3-c6f2-636d378d9d68@I-love.SAKURA.ne.jp>
Date:   Fri, 16 Sep 2022 23:21:42 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To:     Matt Porter <mporter@...nel.crashing.org>,
        Alexandre Bounine <alex.bou9@...il.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v2 (REPOST)] rapidio/tsi721: Replace
 flush_scheduled_work() with flush_work().

Matt and Alexandre, what can I do on this patch?

On 2022/08/15 18:47, Tetsuo Handa wrote:
> On 2022/08/15 9:14, Andrew Morton wrote:
>> On Thu, 28 Jul 2022 10:02:13 +0900 Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp> wrote:
>>
>>> Since "struct tsi721_device" is per a device struct, I assume that
>>> tsi721_remove() needs to wait for only two works associated with that
>>> device. Therefore, wait for only these works using flush_work().
>>
>> The changelog provides no reason for making this change.  Correctness?
>> Efficiency?
> 
> For reducing locking dependency chains that might lead to deadlock.
> 
> The reason was explained in commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue()
> using a macro"). For example, after the fact it turned out that a blind conversion patch
> fixed a deadlock problem at https://syzkaller.appspot.com/bug?extid=2d6ac90723742279e101
> which was hidden due to use of lockdep_set_novalidate_class() call.
> 
> Many of flush_scheduled_work() callers have been removed by v6.0-rc1, but not all
> patches were verbose (because the reason was already explained).
> 
>   a1124c84d467 power: supply: ab8500: Remove flush_scheduled_work() call.
>   162b05524ed3 rtc: Replace flush_scheduled_work() with flush_work().
>   31a1e4a5c104 platform/surface: avoid flush_scheduled_work() usage
>   90c3ca3f247d scsi: mpt3sas: Remove flush_scheduled_work() call
>   62ebaf2f9261 ath6kl: avoid flush_scheduled_work() usage
>   76faa32077b0 iio: light: tsl2563: Replace flush_scheduled_work() with cancel_delayed_work_sync().
>   4bbdc208a5ff staging: olpc_dcon: Replace flush_scheduled_work() with flush_work().
>   c4f135d64382 workqueue: Wrap flush_workqueue() using a macro
>   9cf62d91e4b7 RDMA/mlx4: Avoid flush_scheduled_work() usage
>   549f39a58acf IB/isert: Avoid flush_scheduled_work() usage
>   1b3ce51dde36 Input: psmouse-smbus - avoid flush_scheduled_work() usage
>   0a2f4b5785ca crypto: atmel - Avoid flush_scheduled_work() usage
>   eeff214dbfcb wfx: avoid flush_workqueue(system_highpri_wq) usage
>   0b8d7622ab18 aoe: Avoid flush_scheduled_work() usage
>   cc271ab86606 wwan_hwsim: Avoid flush_scheduled_work() usage
>   ff815a89398d RDMA/core: Avoid flush_workqueue(system_unbound_wq) usage
> 
> There are 7 callers remaining as of v6.0-rc1, and I'm pinging them for heads up.
> 
>   drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c
>   drivers/gpu/drm/i915/display/intel_display.c
>   drivers/gpu/drm/i915/gt/selftest_execlists.c
>   drivers/md/dm.c
>   drivers/message/fusion/mptscsih.c
>   drivers/rapidio/devices/tsi721.c
>   drivers/scsi/qla2xxx/qla_target.c
> 
> If you want to update patch description, you can insert the following.
> 
> ----------------------------------------
> 
> Like commit c4f135d643823a86 ("workqueue: Wrap flush_workqueue() using a
> macro") says, flush_scheduled_work() is dangerous and will be forbidden.
> We are on the way for removing all flush_scheduled_work() callers from
> the kernel, and this patch is for removing flush_scheduled_work() call
>  from tsi721 driver.
>