linux-kernel - Re: [PATCH v6] sbitmap: fix io hung due to race on sbitmap

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: <6acb93c3-f11b-40a4-bec0-b17fb77ad0c9@acm.org>
Date: Thu, 11 Jul 2024 09:33:20 -0700
From: Bart Van Assche <bvanassche@....org>
To: Ming Lei <ming.lei@...hat.com>, YangYang <yang.yang@...o.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Jens Axboe <axboe@...nel.dk>,
 Omar Sandoval <osandov@...com>, linux-kernel@...r.kernel.org,
 linux-block@...r.kernel.org
Subject: Re: [PATCH v6] sbitmap: fix io hung due to race on
 sbitmap_word::cleared

On 7/11/24 6:39 AM, Ming Lei wrote:
> There are only two WRITE on 'cleared':
> 
> - xchg(&map->cleared, 0) in sbitmap_deferred_clear()
> 
> - set_bit() in sbitmap_deferred_clear_bit()
> 
> xchg() supposes to provide such protection already.

Hi Ming,

The comment above 'swap_lock' in this patch is as follows:

	/**
	 * @swap_lock: Held while swapping word <-> cleared
	 */

In other words, 'swap_lock' is used to serialize *code*. Using
synchronization objects to serialize code is known as an anti-pattern,
something that shouldn't be done. Synchronization objects should be used
to serialize access to data. Hence my question whether it would be
appropriate to protect all 'cleared' changes with the newly introduced
spinlock.

Thanks,

Bart.