linux-kernel - Re: [PATCH v3 2/3] block: introduce new field bd_flags in block

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: <ZV25nGGMYQuyclK6@fedora>
Date:   Wed, 22 Nov 2023 16:19:40 +0800
From:   Ming Lei <ming.lei@...hat.com>
To:     Christoph Hellwig <hch@...radead.org>
Cc:     Yu Kuai <yukuai1@...weicloud.com>, axboe@...nel.dk,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        yukuai3@...wei.com, yi.zhang@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH v3 2/3] block: introduce new field bd_flags in
 block_device

On Tue, Nov 21, 2023 at 11:53:17PM -0800, Christoph Hellwig wrote:
> On Wed, Nov 22, 2023 at 03:45:24PM +0800, Ming Lei wrote:
> > All the existed 'bool' flags are not atomic RW, so I think it isn't
> > necessary to define 'bd_flags' as 'unsigned long' for replacing them.
> 
> So because the old code wasn't correct we'll never bother?  The new
> flag and the new placement certainly make this more critical as well.

Can you explain why the old code was wrong?

1) ->bd_read_only and ->bd_make_it_fail

- set from userspace interface(ioctl or sysfs)
- check in IO code path

so changing it into atomic bit doesn't make difference from user
viewpoint.

2) ->bd_write_holder

disk->open_mutex is held for read & write this flag

3) ->bd_has_submit_bio

This flag is setup as oneshot before adding disk, and check in FS io code
path.

Of course, defining it as 'unsigned long' can extend its future usage, but
it depends on the atomic requirement of each flag itself.


Thanks, 
Ming