On Fri, Feb 26, 2016 at 8:53 AM, Mike Snitzer <snitzer@xxxxxxxxxx> wrote: > On Mon, Feb 22 2016 at 1:55pm -0500, > Kees Cook <keescook@xxxxxxxxxxxx> wrote: > >> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon <agk@xxxxxxxxxx> wrote: >> > On Sat, Feb 20, 2016 at 10:13:49AM -0800, Kees Cook wrote: >> >> This is a resurrection of a patch series from a few years back, first >> >> brought to the dm maintainers in 2010. It creates a way to define dm >> >> devices on the kernel command line for systems that do not use an >> >> initramfs, or otherwise need a dm running before init starts. >> >> >> >> This has been used by Chrome OS for several years, and now by Brillo >> >> (and likely Android soon). >> >> >> >> The last version was v4: >> >> https://patchwork.kernel.org/patch/104860/ >> >> https://patchwork.kernel.org/patch/104861/ >> > >> > Inconsistencies in the terminology here can be sorted out during review, >> > and I see that you've taken on board some of my review comments from >> > 2010, but what are your responses to the rest of them? >> >> Ah, sorry, the threads I could find were incomplete, so I wasn't able >> to find those comments that were made to Will's 2010 submission. In >> some of the cleanups I did I was very confused about "target" vs >> "table", and tried to fix that. Regardless, I'm open to fixing >> whatever is needed. :) >> >> Thanks for looking at this again! > > This work isn't going to fly as is. I appreciate the effort and the > goal (without understanding _why_) but: you're open-coding, duplicating > and/or reinventing way too much in do_mounts_dm.c > > 1) You first need to answer: _why_ is using a proper initramfs not > viable? A very simple initramfs that issues dmsetup commands, etc, > isn't so daunting is it? Why is it so important for the kernel to > natively provide a dmsetup interface? Chrome, Android, etc cannot use > initramfs? That is correct: Chrome OS does not (and won't) use an initramfs. This is mainly for reasons of boot speed, verified boot block size, and maybe some other things I don't remember. > 2) If you are able to adequately justify the need for dm=: > I'd much rather the dm= kernel commandline be a simple series of > comma-delimited dmsetup-like commands. > > You'd handle each command with extremely basic parsing: > <dm_ioctl_cmd> <args> [, <dm_ioctl_cmd> <args>] > (inventing a special token to denote <newline>, to support tables with > multiple entries, rather than relying on commas and counts, etc) Sure, changing the syntax is fine by me. We'd need to plumb access to the ioctl interface, though. > and you'd then have do_mounts_dm.c open /dev/mapper/control directly and > issue proper DM ioctls rather than adding all your shim code. This last > bit of opening /dev/mapper/control from init needs more research -- not > sure if doing such a thing from kernel is viable/safe/acceptable. Well, there's no /dev and no init since our dm is the root device (dm-verity). We need everything up and running before we mount the root filesystem, very similar to do_mount_md.c's purpose. -Kees -- Kees Cook Chrome OS & Brillo Security -- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
