linux-kernel - Re: [RFC 3/6] md: replace GENHD_FL_UP with GENHD_FL_DISK_ADDED on is_mddev

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: <CALJn8nOWtFBnuzqPpuOH6nqikUzZGUYWEnvwCFvmrCrC5MWSmw@mail.gmail.com>
Date:   Wed, 28 Jul 2021 16:20:01 -0300
From:   "Guilherme G. Piccoli" <kernel@...ccoli.net>
To:     Luis Chamberlain <mcgrof@...nel.org>, hch@...radead.org
Cc:     axboe@...nel.dk, hare@...e.de,
        Bart Van Assche <bvanassche@....org>,
        Ming Lei <ming.lei@...hat.com>, jack@...e.cz, osandov@...com,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC 3/6] md: replace GENHD_FL_UP with GENHD_FL_DISK_ADDED on is_mddev_broken()

On Thu, Jul 15, 2021 at 5:24 PM Luis Chamberlain <mcgrof@...nel.org> wrote:
>
> The GENHD_FL_DISK_ADDED flag is what we really want, as the
> flag GENHD_FL_UP could be set on a semi-initialized device.
>
> Signed-off-by: Luis Chamberlain <mcgrof@...nel.org>
> ---
>  drivers/md/md.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 832547cf038f..80561bca1f51 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -766,7 +766,7 @@ static inline bool is_mddev_broken(struct md_rdev *rdev, const char *md_type)
>  {
>         int flags = rdev->bdev->bd_disk->flags;
>
> -       if (!(flags & GENHD_FL_UP)) {
> +       if (!(flags & GENHD_FL_DISK_ADDED)) {

Thanks for the patch Luis! And thanks Christoph for looping me in on
the last iteration.
I think specifically for md, both flags are interchangeable - if
add_disk() is not completed, I'm pretty sure we cannot have the array
properly working hence we shouldn't ever reach this check for such
device. Nevertheless, technically speaking Christoph seems correct and
we are checking here in fact if the disk was del_gendisk'ed().
My opinion is that we don't need to change this usage, if possible,
but I'm not strongly against the change if you feel it fits better.

Just double-checking - after del_gendisk(), this flag is removed anyways right?
Oh, and if possible, loop me in CC for next revisions, using this email.
Cheers,

Guilherme