Re: Hardening the LVM support

[Phillip Susi]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 1/29/2013 4:41 AM, Peter Rajnoha wrote:
> Currently, the lvm module seems to recognize striped/linear, raid
> and mirror segment types. As I was looking at the code, I noticed a
> few things that would require looking at. Most notably, it's about
> LVM metadata handling where the parser used in the grub module can
> handle only a subset of what the LVM metadata parser can recognize
> and digest. If any changes are made in the LVM, this could end up
> with errors on the grub module side as a consequence and thus
> causing inability to boot such a system. The possible problems that
> might appear is about handling whitespaces, metadata field order
> processing (the grub module seems to be expecting the fields to be
> in one concrete order which might not be the case all the time),
> also there's missing checksumming (to check whether metadata are
> not corrupted in some way) or processing any new flags we might add
> and which might be usefull when processing such devices and which
> might help grub to process the metadata in a more reliable and
> effective way.
> 
> So the question is whether you are OK with reviewing this code with
> us thouroughly and then doing all the necessary changes that would
> make this solution more robust and error-prone?

I'm sure a patch would be welcome.

> Now, for starters, thinking about the metadata parser and all the
> checks needed, we would like to provide a simple library that grub
> could use/link with. Such a solution would remove the duplication
> of the work done on these parts and we will always have the proper
> code to reflect any changes done in the LVM code or any
> enhancements we might come up with and that grub can directly make
> use of through the interface we would provide (and its use won't be
> bound to grub only, it could be reused in other projects as well).

I think this might be a bit overkill.  The code to parse the metadata
isn't complex, and isn't really likely to change, so it's probably not
worth making a library for.  It may be nice though to make sure it is
just a straight copy of the code from lvm to the grub module, so that
if there are changes in the future, the same patch can be applied to grub.

> An alternative solution might be for the grub to export an
> interface for creating modules "externally" so LVM upstream can
> make use of that and provide the module based on this interface. As
> it seems there's no support for this in grub yet (the modules are
> all created directly by grub team), is there any plan for
> supporting externally built modules in the future, if possible at
> all?

The ABI is not fixed so no, it is not possible to create an external
module since any time you build a new version of the kernel, it may
break the ability to load such a module.

> The aim is simply to move the responsibility for the LVM metadata
> parsing and checking to LVM directly so LVM would maintain that and
> take full responsibility for it. At the moment, it seems the first
> suggestion would work the best (LVM team providing the interface
> for processing LVM volumes and then the lvm grub module linking
> with this interface).

Parsing the metadata and "processing" the volumes are two different
things.  LVM is going to be using all sorts of LVM only code to
manipulate device-mapper and handle things like merging snapshots.
None of this applies to grub.  Once the metadata is parsed, grub just
sets up diskfilter structures to identify the mapping between logical
and physical sectors.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJRHPbZAAoJEJrBOlT6nu75d98H/0t4Z9hClsdHXIrlzWxma94f
sy00+St3HpJLVluYhDoZVds38CKT0rB0O00652pWKYT3eQLDVPKPCissv7t6pgdq
HeZcbSNIS/6IXxmRemuikkL69PJkeZ9NCnhYEvc+X7Hc7JIAPljr0KHVe2QoB0VW
IL2xyXz/Z3sTYXJcJ8iJIMMg3yaaGBRNTr6RVo4nvYYqJ5nWBHcYNUCbp4Wia5vd
sLqotzYHcjamMNei8xj20QThXZ7hqfWNqrg+DtW2XAckxn9ATpJusU68WupfYNPA
rgOMs15n9YUgm/a+xenEpolN/wFgbLH5arhDN9okUUL6gBSbaSakBfDj008b+pc=
=YdJT
-----END PGP SIGNATURE-----