linux-kernel - Re: let md auto-detect 128+ raid members, fix potential race condition

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: <orac6qerr4.fsf@free.oliva.athome.lsd.ic.unicamp.br>
Date:	Sun, 30 Jul 2006 17:56:31 -0300
From:	Alexandre Oliva <aoliva@...hat.com>
To:	Andrew Morton <akpm@...l.org>
Cc:	linux-kernel@...r.kernel.org, Neil Brown <neilb@...e.de>
Subject: Re: let md auto-detect 128+ raid members, fix potential race condition

On Jul 30, 2006, Andrew Morton <akpm@...l.org> wrote:

> On Sun, 30 Jul 2006 03:56:21 -0300
> Alexandre Oliva <aoliva@...hat.com> wrote:

>> -void md_autodetect_dev(dev_t dev);
>> +int md_register_autodetect_dev(dev_t dev);

> Put it in a header file, please.

AFAICT it really isn't supposed to be used elsewhere.  I suppose I
could add it to either blkdev.h, fs.h or raid/md.h, since it's more of
glue code between two modules than something that belongs to one
specific module.  E.g., if it goes in raid/md.h, where it feels the
most appropriate, then fs/parititions/check.c has to include it, which
doesn't sound right.  OTOH, if it goes in blkdev.h or fs.h, then a lot
of code ends up seeing the declaration that shouldn't be available.
Thoughts?

Maybe we could replace this with some register/unregister notifier
interface, such that add_partitions() could then notify multiple
watchers when a new partition is configured.  This would remove the
backwards dependency here, but I feel it should be done in a separate
patch.  I don't mind if they're integrated at once, but I don't feel
that changing two unrelated issues at once is a good approach.

>> +	if (!ldev)
>> +	  return -1;

> whitespace broke.

Oops, thanks, fixed below.

>> #ifdef CONFIG_BLK_DEV_MD
>> -		if (state->parts[p].flags)
>> -			md_autodetect_dev(bdev->bd_dev+p);
>> +		if (state->parts[p].flags
>> +		    && md_register_autodetect_dev(bdev->bd_dev+p))
>> +			printk(KERN_ERR "md: out of memory registering %s%d\n",
>> +			       disk->disk_name, p);
>> #endif

> What happens if CONFIG_BLK_DEV_MD=m?

AFAIK then you'd get a link failure.  One more reason to go with the
notifier approach, I guess.  It wouldn't quite enable md to
auto-detect from partitions set up before the module was loaded, but
it would at least remove this presumed link error.

Another approach would be to split the autodetect stuff out of md.c
into a separate file that goes in the main kernel image (if
CONFIG_MD=y, it's never m) even if CONFIG_BLK_DEV_MD=m.  Would this be
a desirable arrangement?

View attachment "raid-detected-list.patch" of type "text/x-patch" (3393 bytes)

-- 
Alexandre Oliva         http://www.lsd.ic.unicamp.br/~oliva/
Secretary for FSF Latin America        http://www.fsfla.org/
Red Hat Compiler Engineer   aoliva@...dhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@...d.ic.unicamp.br, gnu.org}