linux-kernel - Re: [PATCH] mtd: avoid registering reboot notifier twice

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, 2 Feb 2015 01:20:13 -0800
From:	Brian Norris <computersforpeace@...il.com>
To:	Ricard Wanderlof <ricard.wanderlof@...s.com>
Cc:	Niklas Cassel <nks@...wful.org>,
	"linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
	"dwmw2@...radead.org" <dwmw2@...radead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mtd: avoid registering reboot notifier twice

On Mon, Feb 02, 2015 at 10:01:51AM +0100, Ricard Wanderlof wrote:
> 
> > On 02/02/2015 12:07 AM, Brian Norris wrote:
> > >
> > > No driver should be calling mtd_device_parse_register() multiple times
> > > on the same mtd_info. Under what context do you see this? What driver?
> > arch/cris/arch-v32/drivers/axisflashmap.c
> 
> Looking at it another way, why should it not be allowed, if the only 
> thing stopping it from working is Niklas' fix below?

It severely breaks the way that normal MTD partition parsers work, for
one. cmdlineparts could never be sanely applied here, for instance.

> If there are multiple 
> ways a driver can acquire partitioning information, it is handy to be able 
> to do mtd_device_parse_register() several times, rather than collecting 
> all the partitioning information in one place and doing the call there, 
> which I suppose would be the alternative.

It may be handy, but it breaks the abstraction that is established in
most every other case, that partitions are determined by exactly one
source: a single list from platform data, device tree, the kernel
command line, or an on-flash parser (e.g., RedBoot, bcm47xxpart, etc.).

> I've noted that mtd/nand/diskonchip.c does it too:
> 
>         mtd_device_register(mtd, NULL, 0);
>         if (!no_autopart)
>                 mtd_device_register(mtd, parts, numparts);
> 
> but this driver might be essentially deprecated, I don't know.

Yeah, I just noticed that one too.

TBH, I'm not sure the reboot notifier patch places that code in the best
place anyway. There really is no central initialization code for the
'master' mtd_info struct, as this initialization is handled ad-hoc in
every single MTD driver. It just happens that the partition
parsing/registration function fit nicely as the one place that (almost)
all MTD master devices got registered. But this just exposed yet another
instance where my assumptions are challenged.

There's so much legacy crap in MTD that sometimes I wonder why I even
bother...

Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/