linux-kernel - Re: [RFC PATCH 1/4] init: move block device helpers from init/do

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: <Y3j+Pzy1JpqG8Yd8@makrotopia.org>
Date:   Sat, 19 Nov 2022 16:03:11 +0000
From:   Daniel Golle <daniel@...rotopia.org>
To:     Christoph Hellwig <hch@...radead.org>
Cc:     Richard Weinberger <richard@....at>,
        Matthew Wilcox <willy@...radead.org>,
        Jens Axboe <axboe@...nel.dk>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Chaitanya Kulkarni <kch@...dia.com>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>,
        linux-block@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 1/4] init: move block device helpers from
 init/do_mounts.c

Hi Christoph,

On Wed, Nov 16, 2022 at 09:55:55PM -0800, Christoph Hellwig wrote:
> On Thu, Nov 17, 2022 at 12:42:43AM +0000, Daniel Golle wrote:
> > In init/do_mounts.c there are helper functions devt_from_partuuid,
> > devt_from_partlabel and devt_from_devname. In order to make these
> > functions available to other users, move them to block/bdev.c.
> 
> Yes, they should be moved and I have an old patch to do this a bit
> smarter.  But that doesn't mean anyone else should call them.

I have followed your advise and implemented a "real" block driver
stacking on top of an existing block device to expose uImage.FIT
filesystem subimages as block devices. It mangles any bio submitted to
it and re-submits the bio with changed bi_bdev and sector offset added
to bi_iter.bi_sector for all list elements.

That works, but has slightly less utility value than the partition
parser approach as in this way I cannot easily populate the PARTNAME
uevent which can later help userspace to identify a device by the FIT
subimage name -- I'd have to either invent a new bus_type or
device_type, both seem inappropriate and have unwanted side effects.
Or am I missing something and there is a way to use add_uevent_var()
for a disk_type device?

In exchange, instead of using the PARTNAME= uevent var, I can now
freely name the device node by setting the disk_name string in
struct gendisk. While this doesn't offer the same amount of freedom, it
is sufficient for our use-case.

However, I don't see a way to avoid using (or duplicating)
devt_from_devname() to select the lower device somehow without having
to probe and parse *all* block devices present (which is, from what I
understood, what you want to avoid, and I agree that it is more safe to
not do that...)

Can you or anyone give some advise on how this should be done?

As block partitions are not present in device tree, using a cmdline
argument (with similar semantics to 'root=') specifying the lower block
device seems unavoidable.

Also note that due to libfdt not exporting symbols, the driver
currently cannot be built as a module. Hence, I would either need to
export most of libfdt API or only allow the driver to be built-into the
kernel. I don't see a problem with having it always built-in.

Yet another (imho not terrible) problem is removal of the lower device.
Many of the supported SBC use a micro SD card to boot, which can be
removed by the user while the system is running (which is generally not
a good idea, but anyway). For partitions this is handled automatically
by blk_drop_partitions() called directly from genhd.c.
I'm currently playing with doing something similar using the bus device
removal notification, but it doesn't seem to work for all cases, e.g.
mmcblk device do not seem to have the ->bus pointer populated at all
(ie. disk_to_dev(disk)->bus == NULL for mmcblk devices).

Any ideas regarding that?

Thank you for your advise!

Daniel