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

Date:   Mon, 15 Aug 2022 18:47:16 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     Matt Porter <mporter@...nel.crashing.org>,
        Alexandre Bounine <alex.bou9@...il.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 (REPOST)] rapidio/tsi721: Replace
 flush_scheduled_work() with flush_work().

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.