Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-26 Thread Mike Snitzer
On Fri, Feb 26 2016 at  2:59pm -0500,
Kees Cook  wrote:

> On Fri, Feb 26, 2016 at 11:21 AM, Mike Snitzer  wrote:
> > On Fri, Feb 26 2016 at  1:52pm -0500,
> > Kees Cook  wrote:
> >
> >> On Fri, Feb 26, 2016 at 8:53 AM, Mike Snitzer  wrote:
> >> > On Mon, Feb 22 2016 at  1:55pm -0500,
> >> > Kees Cook  wrote:
> >> >
> >> >> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon  
> >> >> 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.
> >
> > Not sure what "verified boot block size" means but...
> 
> Chrome OS uses coreboot as its boot firmware and a coreboot module
> known as "depthcharge" for doing the crypto-verification and booting
> of the Chrome OS system. This is the static root of trust Chrome OS
> extends from its read-only boot firmware through to the kernel it
> loads and the dm-verity partition it mounts as the read-only root
> filesystem. To keep the boot speed fast and the kernel size small,
> there is no initramfs.
> 
> > Sorry I really don't buy that using a custom initramfs would be the
> > source of slow boot.  initramfs is _not_ this hugely inefficient
> > mechanism you'd have us believe.
> 
> I didn't say it was hugely inefficient, but for Chrome OS it's not
> needed, and was a measurable source of boot time. We just got rid of
> it since it was redundant. I can't change that design decision; I'm
> just here to help bring the dm= boot support upstream. :)
> 
> > And if that is the justification for this early boot dm= support then
> > the Chrome OS project/team will have to continue to carry the hack
> > locally.  It has no place upstream.  But I'm open to revisiting this if
> > it can be implemented in a very cheap way.
> 
> Yeah, I'm open to whatever suggestions you have.
> 
> >> > 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:
> >> >[,  ]
> >> > (inventing a special token to denote , 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.
> >
> > I was hoping to avoid any extra hacks but yes... seems you'd need a new
> > API to issue the equivalent of a DM ioctl programatically.  Hopefully
> > it'd be quite a small wrapper.
> 
> Seems like it shouldn't be too bad.

OK, I'm waiting on you to give it a shot.  I'll do my best to help.

Thanks,
Mike


Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-26 Thread Mike Snitzer
On Fri, Feb 26 2016 at  2:59pm -0500,
Kees Cook  wrote:

> On Fri, Feb 26, 2016 at 11:21 AM, Mike Snitzer  wrote:
> > On Fri, Feb 26 2016 at  1:52pm -0500,
> > Kees Cook  wrote:
> >
> >> On Fri, Feb 26, 2016 at 8:53 AM, Mike Snitzer  wrote:
> >> > On Mon, Feb 22 2016 at  1:55pm -0500,
> >> > Kees Cook  wrote:
> >> >
> >> >> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon  
> >> >> 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.
> >
> > Not sure what "verified boot block size" means but...
> 
> Chrome OS uses coreboot as its boot firmware and a coreboot module
> known as "depthcharge" for doing the crypto-verification and booting
> of the Chrome OS system. This is the static root of trust Chrome OS
> extends from its read-only boot firmware through to the kernel it
> loads and the dm-verity partition it mounts as the read-only root
> filesystem. To keep the boot speed fast and the kernel size small,
> there is no initramfs.
> 
> > Sorry I really don't buy that using a custom initramfs would be the
> > source of slow boot.  initramfs is _not_ this hugely inefficient
> > mechanism you'd have us believe.
> 
> I didn't say it was hugely inefficient, but for Chrome OS it's not
> needed, and was a measurable source of boot time. We just got rid of
> it since it was redundant. I can't change that design decision; I'm
> just here to help bring the dm= boot support upstream. :)
> 
> > And if that is the justification for this early boot dm= support then
> > the Chrome OS project/team will have to continue to carry the hack
> > locally.  It has no place upstream.  But I'm open to revisiting this if
> > it can be implemented in a very cheap way.
> 
> Yeah, I'm open to whatever suggestions you have.
> 
> >> > 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:
> >> >[,  ]
> >> > (inventing a special token to denote , 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.
> >
> > I was hoping to avoid any extra hacks but yes... seems you'd need a new
> > API to issue the equivalent of a DM ioctl programatically.  Hopefully
> > it'd be quite a small wrapper.
> 
> Seems like it shouldn't be too bad.

OK, I'm waiting on you to give it a shot.  I'll do my best to help.

Thanks,
Mike


Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-26 Thread Kees Cook
On Fri, Feb 26, 2016 at 11:21 AM, Mike Snitzer  wrote:
> On Fri, Feb 26 2016 at  1:52pm -0500,
> Kees Cook  wrote:
>
>> On Fri, Feb 26, 2016 at 8:53 AM, Mike Snitzer  wrote:
>> > On Mon, Feb 22 2016 at  1:55pm -0500,
>> > Kees Cook  wrote:
>> >
>> >> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon  
>> >> 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.
>
> Not sure what "verified boot block size" means but...

Chrome OS uses coreboot as its boot firmware and a coreboot module
known as "depthcharge" for doing the crypto-verification and booting
of the Chrome OS system. This is the static root of trust Chrome OS
extends from its read-only boot firmware through to the kernel it
loads and the dm-verity partition it mounts as the read-only root
filesystem. To keep the boot speed fast and the kernel size small,
there is no initramfs.

> Sorry I really don't buy that using a custom initramfs would be the
> source of slow boot.  initramfs is _not_ this hugely inefficient
> mechanism you'd have us believe.

I didn't say it was hugely inefficient, but for Chrome OS it's not
needed, and was a measurable source of boot time. We just got rid of
it since it was redundant. I can't change that design decision; I'm
just here to help bring the dm= boot support upstream. :)

> And if that is the justification for this early boot dm= support then
> the Chrome OS project/team will have to continue to carry the hack
> locally.  It has no place upstream.  But I'm open to revisiting this if
> it can be implemented in a very cheap way.

Yeah, I'm open to whatever suggestions you have.

>> > 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:
>> >[,  ]
>> > (inventing a special token to denote , 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.
>
> I was hoping to avoid any extra hacks but yes... seems you'd need a new
> API to issue the equivalent of a DM ioctl programatically.  Hopefully
> it'd be quite a small wrapper.

Seems like it shouldn't be too bad.

>> > 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.
>
> Ah yes, microoptimization associated with no udev or normal Linux boot
> comes full circle and limits the use of existing standard interfaces.

Chrome OS 

Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-26 Thread Kees Cook
On Fri, Feb 26, 2016 at 11:21 AM, Mike Snitzer  wrote:
> On Fri, Feb 26 2016 at  1:52pm -0500,
> Kees Cook  wrote:
>
>> On Fri, Feb 26, 2016 at 8:53 AM, Mike Snitzer  wrote:
>> > On Mon, Feb 22 2016 at  1:55pm -0500,
>> > Kees Cook  wrote:
>> >
>> >> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon  
>> >> 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.
>
> Not sure what "verified boot block size" means but...

Chrome OS uses coreboot as its boot firmware and a coreboot module
known as "depthcharge" for doing the crypto-verification and booting
of the Chrome OS system. This is the static root of trust Chrome OS
extends from its read-only boot firmware through to the kernel it
loads and the dm-verity partition it mounts as the read-only root
filesystem. To keep the boot speed fast and the kernel size small,
there is no initramfs.

> Sorry I really don't buy that using a custom initramfs would be the
> source of slow boot.  initramfs is _not_ this hugely inefficient
> mechanism you'd have us believe.

I didn't say it was hugely inefficient, but for Chrome OS it's not
needed, and was a measurable source of boot time. We just got rid of
it since it was redundant. I can't change that design decision; I'm
just here to help bring the dm= boot support upstream. :)

> And if that is the justification for this early boot dm= support then
> the Chrome OS project/team will have to continue to carry the hack
> locally.  It has no place upstream.  But I'm open to revisiting this if
> it can be implemented in a very cheap way.

Yeah, I'm open to whatever suggestions you have.

>> > 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:
>> >[,  ]
>> > (inventing a special token to denote , 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.
>
> I was hoping to avoid any extra hacks but yes... seems you'd need a new
> API to issue the equivalent of a DM ioctl programatically.  Hopefully
> it'd be quite a small wrapper.

Seems like it shouldn't be too bad.

>> > 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.
>
> Ah yes, microoptimization associated with no udev or normal Linux boot
> comes full circle and limits the use of existing standard interfaces.

Chrome OS is hardly the only Linux device without an initramfs, but
yeah, it does cause us headaches from time to 

Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-26 Thread Mike Snitzer
On Fri, Feb 26 2016 at  1:52pm -0500,
Kees Cook  wrote:

> On Fri, Feb 26, 2016 at 8:53 AM, Mike Snitzer  wrote:
> > On Mon, Feb 22 2016 at  1:55pm -0500,
> > Kees Cook  wrote:
> >
> >> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon  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.

Not sure what "verified boot block size" means but...

Sorry I really don't buy that using a custom initramfs would be the
source of slow boot.  initramfs is _not_ this hugely inefficient
mechanism you'd have us believe.

And if that is the justification for this early boot dm= support then
the Chrome OS project/team will have to continue to carry the hack
locally.  It has no place upstream.  But I'm open to revisiting this if
it can be implemented in a very cheap way.

> > 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:
> >[,  ]
> > (inventing a special token to denote , 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.

I was hoping to avoid any extra hacks but yes... seems you'd need a new
API to issue the equivalent of a DM ioctl programatically.  Hopefully
it'd be quite a small wrapper.

> > 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.

Ah yes, microoptimization associated with no udev or normal Linux boot
comes full circle and limits the use of existing standard interfaces.


Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-26 Thread Mike Snitzer
On Fri, Feb 26 2016 at  1:52pm -0500,
Kees Cook  wrote:

> On Fri, Feb 26, 2016 at 8:53 AM, Mike Snitzer  wrote:
> > On Mon, Feb 22 2016 at  1:55pm -0500,
> > Kees Cook  wrote:
> >
> >> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon  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.

Not sure what "verified boot block size" means but...

Sorry I really don't buy that using a custom initramfs would be the
source of slow boot.  initramfs is _not_ this hugely inefficient
mechanism you'd have us believe.

And if that is the justification for this early boot dm= support then
the Chrome OS project/team will have to continue to carry the hack
locally.  It has no place upstream.  But I'm open to revisiting this if
it can be implemented in a very cheap way.

> > 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:
> >[,  ]
> > (inventing a special token to denote , 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.

I was hoping to avoid any extra hacks but yes... seems you'd need a new
API to issue the equivalent of a DM ioctl programatically.  Hopefully
it'd be quite a small wrapper.

> > 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.

Ah yes, microoptimization associated with no udev or normal Linux boot
comes full circle and limits the use of existing standard interfaces.


Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-26 Thread Kees Cook
On Fri, Feb 26, 2016 at 8:53 AM, Mike Snitzer  wrote:
> On Mon, Feb 22 2016 at  1:55pm -0500,
> Kees Cook  wrote:
>
>> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon  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:
>[,  ]
> (inventing a special token to denote , 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


Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-26 Thread Kees Cook
On Fri, Feb 26, 2016 at 8:53 AM, Mike Snitzer  wrote:
> On Mon, Feb 22 2016 at  1:55pm -0500,
> Kees Cook  wrote:
>
>> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon  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:
>[,  ]
> (inventing a special token to denote , 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


Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-26 Thread Mike Snitzer
On Mon, Feb 22 2016 at  1:55pm -0500,
Kees Cook  wrote:

> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon  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?

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:
   [,  ]
(inventing a special token to denote , to support tables with
multiple entries, rather than relying on commas and counts, etc)

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.

Mike


Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-26 Thread Mike Snitzer
On Mon, Feb 22 2016 at  1:55pm -0500,
Kees Cook  wrote:

> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon  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?

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:
   [,  ]
(inventing a special token to denote , to support tables with
multiple entries, rather than relying on commas and counts, etc)

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.

Mike


Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-22 Thread Kees Cook
On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon  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!

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security


Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-22 Thread Kees Cook
On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon  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!

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security


Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-21 Thread Alasdair G Kergon
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?

Alasdair



Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-21 Thread Alasdair G Kergon
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?

Alasdair



[PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-20 Thread Kees Cook
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/

-Kees



[PATCH v5 0/3] init: add support to directly boot to a mapped device

2016-02-20 Thread Kees Cook
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/

-Kees