LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Taras Kondratiuk <firstname.lastname@example.org>
To: Rob Landley <email@example.com>,
Victor Kamensky <firstname.lastname@example.org>,
Cc: Al Viro <email@example.com>, Arnd Bergmann <firstname.lastname@example.org>,
Mimi Zohar <email@example.com>,
Jonathan Corbet <firstname.lastname@example.org>,
James McMechan <email@example.com>,
Subject: Re: [PATCH v3 01/15] Documentation: add newcx initramfs format description
Date: Sat, 17 Feb 2018 02:04:00 -0800 [thread overview]
Message-ID: <151886184029.6069.5504703113024901667@takondra-t460s> (raw)
Quoting firstname.lastname@example.org (2018-02-16 16:00:36)
> On February 16, 2018 1:47:35 PM PST, Victor Kamensky <email@example.com> wrote:
> >On Fri, 16 Feb 2018, Rob Landley wrote:
> >> On 02/16/2018 02:59 PM, H. Peter Anvin wrote:
> >>> On 02/16/18 12:33, Taras Kondratiuk wrote:
> >>>> Many of the Linux security/integrity features are dependent on file
> >>>> metadata, stored as extended attributes (xattrs), for making
> >>>> These features need to be initialized during initcall and enabled
> >>>> early as possible for complete security coverage.
> >>>> Initramfs (tmpfs) supports xattrs, but newc CPIO archive format
> >does not
> >>>> support including them into the archive.
> >>>> This patch describes "extended" newc format (newcx) that is based
> >>>> newc and has following changes:
> >>>> - extended attributes support
> >>>> - increased size of filesize to support files >4GB
> >>>> - increased mtime field size to have 64 bits of seconds and added a
> >>>> field for nanoseconds
> >>>> - removed unused checksum field
> >>> If you are going to implement a new, non-backwards-compatible
> >>> you shouldn't replicate the mistakes of the current format.
> >> So rather than make minimal changes to the existing format and
> >continue to
> >> support the existing format (sharing as much code as possible), you
> >> gratuitous aesthetic changes?
> >>> 1. The use of ASCII-encoded fixed-length numbers is an idiotic
> >>> from an era before there were any portable way of dealing with
> >>> with prespecified endianness.
> >> It lets encoders and decoders easily share code with the existing
> >cpio format,
> >> which we still intend to be able to read and write.
> >>> If you are going to use ASCII, make them
> >>> delimited so that they don't have fixed limits, or just use binary.
> >> When it's gzipped this accomplishes what? (Other than being
> >> different from the previous iteration?)
> >>> The cpio header isn't fixed size, so that argument goes away, in
> >>> the only way to determine the end of the header is to scan forward.
> >>> 2. Alignment sensitivity! Because there is no header length
> >>> information, the above scan tells you where the header ends, but
> >>> is padding before the data, and the size of that padding is only
> >>> by alignment.
> >> Again, these are minimal changes to the existing cpio format. You're
> >> about _cpio_, and that the new stuff isn't _different_ enough from
> >>> 3. Inband encoding of EOF: if you actually have a filename
> >>> you have problems.
> >> Been there, done that:
> >> http://lkml.iu.edu/hypermail/linux/kernel/1801.3/01791.html
> >>> But first, before you define a whole new format for which no tools
> >>> (you will have to work with the maintainers of the GNU tools to add
> >>> support)
> >> No, he's been working with the maintainer of toybox to add support
> >(for about a
> >> year now), which gets him the Android command line. And the kernel
> >has its own
> >> built-in tool to generate cpio images anyway.
> >> Why would anyone care what the GNU project thinks?
> >In our internal use of this patch series we do use gnu cpio
> >to create initramfs.cpio.
> >And reference to gnu cpio patch that supports newcx format is
> >posted in description for this serieis:
> >Whether GNU cpio maintainers will accept it is different matter.
> >We will try, but we need to start somewhere and agree on
> >new format first.
> >>> you should see how complex it would be to support the POSIX
> >>> tar/pax format,
> >> That argument was had (at length) when initramfs went in over a
> >decade ago.
> >> There are links in
> >Documentation/filesystems/ramfs-rootfs-initramfs.txt to the
> >> mailing list entries about it.
> >>> which already has all the features you are seeking, and
> >>> by now is well-supported.
> >> So... tar wasn't well-supported 15 years ago? (Hasn't the kernel
> >source always
> >> been distributed via tarball back since 0.0.1?)
> >> You're suggesting having a whole second codepath that shares no code
> >with the
> >> existing cpio extractor. Are you suggesting abandoning support for
> >the existing
> >> initramfs.cpio.gz file format?
> >> Rob
> Introducing new, incompatible data formats is an inherently *very* costly operation; unfortunately many engineers don't seem to have a good grip of just *how* expensive it is (see "silly embedded nonsense hacks", "too little, too soon".)
> Cpio itself is a great horror show of just how bad this gets: a bunch of minor tweaks without finding underlying design bugs resulting in a ton of mutually incompatible formats. "They are almost the same" doesn't help: they are still incompatible.
> Introducing a new incompatible data format without strong justification is engineering malpractice. Doing it under the non-justification of expedience ("oh, we can share most of the code") is aggravated engineering malpractice.
> It is entirely possible that the modern posix tar/pax format is too complex to be practical in this case – that would be justifying a new format. But then you are taking the fundamental cost of breakage, and then the new format definitely should not be replicating known defects of another format and without at least some thought about how to avoid it in the future.
I do understand a cost of adding a new format and I'd be very happy not
to do it if there is a better option. I did consider using tar/pax, but
looks like it was already discussed in 2001 between you and Al Viro 
and tar was rejected.
My main tar concerns:
- ustar+pax header is *huge*. E.g. directory entry in archive: pax 1536
bytes vs cpio <200 bytes. Overall compressed initramfs size increase
is not significant though.
- pax is not a strict format. E.g. xattrs may be stored under different
names: SHCILY.xattr (GNU tar, star) vs LIBARCHIVE.xattr (libarchive).
I'm not sure which option is better. Adding tar to the kernel or adding
new cpio format into several tools (GNU cpio, libarchive, busybox,
toybox) will result in approximately the same amount of code.
It would be nice to get Al Viro's thoughts on this.
next prev parent reply other threads:[~2018-02-17 10:04 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-16 20:33 [PATCH v3 00/15] extend initramfs archive format to support xattrs Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 01/15] Documentation: add newcx initramfs format description Taras Kondratiuk
2018-02-16 20:59 ` H. Peter Anvin
2018-02-16 21:25 ` Rob Landley
2018-02-16 21:47 ` Victor Kamensky
2018-02-17 0:00 ` hpa
2018-02-17 10:04 ` Taras Kondratiuk [this message]
2018-02-17 17:32 ` Rob Landley
2018-02-18 0:15 ` Mimi Zohar
2018-02-18 0:24 ` hpa
2018-02-18 0:26 ` hpa
2018-02-18 0:47 ` Mimi Zohar
2018-02-16 20:33 ` [PATCH v3 02/15] initramfs: replace states with function pointers Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 03/15] initramfs: store file name in name_buf Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 04/15] initramfs: remove unnecessary symlinks processing shortcut Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 05/15] initramfs: move files creation into separate state Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 06/15] initramfs: separate reading cpio method from header Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 07/15] initramfs: split header layout information from parsing function Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 08/15] initramfs: add newcx format Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 09/15] initramfs: set extended attributes Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 10/15] gen_init_cpio: move header formatting into function Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 11/15] gen_init_cpio: add newcx format Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 12/15] gen_init_cpio: set extended attributes for " Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 12/14] gen_initramfs_list.sh: add -x option to enable " Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 13/15] " Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 13/14] selinux: allow setxattr on rootfs so initramfs code can set them Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 14/15] " Taras Kondratiuk
2018-02-20 19:01 ` Stephen Smalley
2018-03-11 3:07 ` Victor Kamensky
2018-03-20 16:33 ` [Non-DoD Source] " Stephen Smalley
2018-02-16 20:33 ` [PATCH v3 14/14] selinux: delay sid population for rootfs till init is complete Taras Kondratiuk
2018-02-16 20:33 ` [PATCH v3 15/15] " Taras Kondratiuk
2018-02-20 18:56 ` Stephen Smalley
2018-03-07 16:51 ` Rob Landley
2018-03-07 17:26 ` Victor Kamensky
2018-03-11 3:08 ` Victor Kamensky
2018-03-20 16:30 ` [Non-DoD Source] " Stephen Smalley
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--subject='Re: [PATCH v3 01/15] Documentation: add newcx initramfs format description' \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).