[PATCH] Chained CPIOs writing to the same file bug

[PATCH] Chained CPIOs writing to the same file bug

[Michael Neuling]

You can chain CPIOs together for the initramfs, but if two CPIOs write
to the same file, we don't clear the first before writing the second.
If the first is larger than the second, we end up with a mash of the
two.  Trivial patch below to fix this.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
 init/initramfs.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletion(-)

Index: linux-2.6-linus/init/initramfs.c
===================================================================
--- linux-2.6-linus.orig/init/initramfs.c
+++ linux-2.6-linus/init/initramfs.c
@@ -250,7 +250,8 @@ static int __init do_name(void)
 		return 0;
 	if (S_ISREG(mode)) {
 		if (maybe_link() >= 0) {
-			wfd = sys_open(collected, O_WRONLY|O_CREAT, mode);
+			wfd = sys_open(collected, O_WRONLY|O_CREAT|O_TRUNC,
+				       mode);
 			if (wfd >= 0) {
 				sys_fchown(wfd, uid, gid);
 				sys_fchmod(wfd, mode);

[PATCH] initramfs: multiple CPIO unpacking fix

[Michael Neuling]

The following patch unlinks (deletes) files, symlinks, FIFOs, devices
etc before writing them when extracting CPIOs.  It doesn't delete
directories.  This stops weird behaviour like:
 1) writing through symlinks created in earlier CPIOs. eg foo->bar in
    the first CPIO.  Having foo as a non link in a subsequent CPIO,
    results in bar being written and foo remaining as a symlink.  
 2) if the first version of file foo is larger than foo in a
    subsequent CPIO, we end up with a mix of the two.  ie. neither
    the first or second version of /foo.
 3) special files like devices, fifo etc can't be overwritten in
    subsequent CPIOS.

With this patch, the kernel will more closely replicate
  for i in *.cpio; do cpio --extract --unconditional < $i ; done

This patch doesn't break hardlinks like my previous attempt.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
 initramfs.c |    3 +++
 1 files changed, 3 insertions(+)

Index: linux-2.6.15/init/initramfs.c
===================================================================
--- linux-2.6.15.orig/init/initramfs.c
+++ linux-2.6.15/init/initramfs.c
@@ -249,6 +249,7 @@ static int __init do_name(void)
 	if (dry_run)
 		return 0;
 	if (S_ISREG(mode)) {
+		sys_unlink(collected);
 		if (maybe_link() >= 0) {
 			wfd = sys_open(collected, O_WRONLY|O_CREAT, mode);
 			if (wfd >= 0) {
@@ -263,6 +264,7 @@ static int __init do_name(void)
 		sys_chmod(collected, mode);
 	} else if (S_ISBLK(mode) || S_ISCHR(mode) ||
 		   S_ISFIFO(mode) || S_ISSOCK(mode)) {
+		sys_unlink(collected);
 		if (maybe_link() == 0) {
 			sys_mknod(collected, mode, rdev);
 			sys_chown(collected, uid, gid);
@@ -291,6 +293,7 @@ static int __init do_copy(void)
 static int __init do_symlink(void)
 {
 	collected[N_ALIGN(name_len) + body_len] = '\0';
+	sys_unlink(collected);
 	sys_symlink(collected + N_ALIGN(name_len), collected);
 	sys_lchown(collected, uid, gid);
 	state = SkipIt;

Re: [klibc] [PATCH] initramfs: multiple CPIO unpacking fix

[Jeff Bailey]

[-- Attachment #1: Type: text/plain, Size: 1170 bytes --]

Le mercredi 22 février 2006 à 10:45 +1100, Michael Neuling a écrit :
> The following patch unlinks (deletes) files, symlinks, FIFOs, devices
> etc before writing them when extracting CPIOs.  It doesn't delete
> directories.  This stops weird behaviour like:
>  1) writing through symlinks created in earlier CPIOs. eg foo->bar in
>     the first CPIO.  Having foo as a non link in a subsequent CPIO,
>     results in bar being written and foo remaining as a symlink.  

I've tended to think of this as a feature, actually.  In Ubuntu, for
instance, we might have 2.6.15-8 and 2.6.15-9 which represent different
ABIs from security updates or other changes.  If I have a module that is
intended to be compatible with both, I might setup /lib/modules/generic
to be a symlink to /lib/modules/2.6.15-9/ and unpack the modules after
the symlink is expected to be there.

(I don't think we use this feature right now, but I had tested it and
noted it before.  It's very convenient, since it's the exact same
behaviour that dpkg itself has)

Tks,
Jeff Bailey

* Canonical Ltd * Ubuntu Service and Support * +1 514 691 7221 *

Linux for Human Beings.

[-- Attachment #2: Ceci est une partie de message numériquement signée --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

Re: [klibc] [PATCH] initramfs: multiple CPIO unpacking fix

[H. Peter Anvin]

Jeff Bailey wrote:
> 
> I've tended to think of this as a feature, actually.  In Ubuntu, for
> instance, we might have 2.6.15-8 and 2.6.15-9 which represent different
> ABIs from security updates or other changes.  If I have a module that is
> intended to be compatible with both, I might setup /lib/modules/generic
> to be a symlink to /lib/modules/2.6.15-9/ and unpack the modules after
> the symlink is expected to be there.
> 

This is pretty broken for a bunch of other reasons, though.  In 
particular, it prevents the very useful behaviour of providing a symlink 
in entry A that can be overridden by a file in entry B.

> (I don't think we use this feature right now, but I had tested it and
> noted it before.  It's very convenient, since it's the exact same
> behaviour that dpkg itself has)

I would personally consider that a bug in dpkg :-/

	-hpa

[PATCH] initramfs: CPIO unpacking fix

[Michael Neuling]

Unlink files, symlinks, FIFOs, devices etc. (except directories) before
writing them when extracting CPIOs.  This stops weird behaviour like:
 1) writing through symlinks created in earlier CPIOs. eg foo->bar in
    the first CPIO.  Having foo as a non-link in a subsequent CPIO,
    results in bar being written and foo remaining as a symlink.  
 2) if the first version of file foo is larger than foo in a
    subsequent CPIO, we end up with a mix of the two.  ie. neither
    the first or second version of /foo.
 3) special files like devices, fifo etc. can't be overwritten in
    subsequent CPIOS.

With this, the kernel will more closely replicate
  for i in *.cpio; do cpio --extract --unconditional < $i ; done

This is a change but it's regarded as fixing broken functionality.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
This is a retransmission.

 init/initramfs.c |    3 +++
 1 files changed, 3 insertions(+)

Index: linux-2.6.15/init/initramfs.c
===================================================================
--- linux-2.6.15.orig/init/initramfs.c
+++ linux-2.6.15/init/initramfs.c
@@ -249,6 +249,7 @@ static int __init do_name(void)
 	if (dry_run)
 		return 0;
 	if (S_ISREG(mode)) {
+		sys_unlink(collected);
 		if (maybe_link() >= 0) {
 			wfd = sys_open(collected, O_WRONLY|O_CREAT, mode);
 			if (wfd >= 0) {
@@ -263,6 +264,7 @@ static int __init do_name(void)
 		sys_chmod(collected, mode);
 	} else if (S_ISBLK(mode) || S_ISCHR(mode) ||
 		   S_ISFIFO(mode) || S_ISSOCK(mode)) {
+		sys_unlink(collected);
 		if (maybe_link() == 0) {
 			sys_mknod(collected, mode, rdev);
 			sys_chown(collected, uid, gid);
@@ -291,6 +293,7 @@ static int __init do_copy(void)
 static int __init do_symlink(void)
 {
 	collected[N_ALIGN(name_len) + body_len] = '\0';
+	sys_unlink(collected);
 	sys_symlink(collected + N_ALIGN(name_len), collected);
 	sys_lchown(collected, uid, gid);
 	state = SkipIt;

Re: [PATCH] initramfs: CPIO unpacking fix

[Jeff Garzik]

Michael Neuling wrote:
> Unlink files, symlinks, FIFOs, devices etc. (except directories) before
> writing them when extracting CPIOs.  This stops weird behaviour like:
>  1) writing through symlinks created in earlier CPIOs. eg foo->bar in
>     the first CPIO.  Having foo as a non-link in a subsequent CPIO,
>     results in bar being written and foo remaining as a symlink.  
>  2) if the first version of file foo is larger than foo in a
>     subsequent CPIO, we end up with a mix of the two.  ie. neither
>     the first or second version of /foo.
>  3) special files like devices, fifo etc. can't be overwritten in
>     subsequent CPIOS.
> 
> With this, the kernel will more closely replicate
>   for i in *.cpio; do cpio --extract --unconditional < $i ; done
> 
> This is a change but it's regarded as fixing broken functionality.
> 
> Signed-off-by: Michael Neuling <mikey@neuling.org>

For the kernel, I would regard that as needless code...  Coding for a 
chain of CPIO archives overwriting each other seems like overengineering.

	Jeff

Re: [klibc] Re: [PATCH] initramfs: CPIO unpacking fix

[H. Peter Anvin]

Jeff Garzik wrote:
> 
> For the kernel, I would regard that as needless code...  Coding for a 
> chain of CPIO archives overwriting each other seems like overengineering.
> 

No, it's actually significant.  The ability to compose initramfs 
contents from multiple sources is one of the major improvements over initrd.

For example, people has asked that kinit should be able to be called 
from user-provided (initrd-loaded) initramfs code.  The easiest way to 
do that is to have the in-kernel module have:

	/init -> /kinit
	/kinit

... and allow /init to be overwritten.

	-hpa

Re: [PATCH] initramfs: CPIO unpacking fix

[Rob Landley]

On Wednesday 22 March 2006 2:14 am, Jeff Garzik wrote:
> Michael Neuling wrote:
> > Unlink files, symlinks, FIFOs, devices etc. (except directories) before
> > writing them when extracting CPIOs.  This stops weird behaviour like:
> >  1) writing through symlinks created in earlier CPIOs. eg foo->bar in
> >     the first CPIO.  Having foo as a non-link in a subsequent CPIO,
> >     results in bar being written and foo remaining as a symlink.
> >  2) if the first version of file foo is larger than foo in a
> >     subsequent CPIO, we end up with a mix of the two.  ie. neither
> >     the first or second version of /foo.
> >  3) special files like devices, fifo etc. can't be overwritten in
> >     subsequent CPIOS.
> >
> > With this, the kernel will more closely replicate
> >   for i in *.cpio; do cpio --extract --unconditional < $i ; done
> >
> > This is a change but it's regarded as fixing broken functionality.
> >
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
>
> For the kernel, I would regard that as needless code...  Coding for a
> chain of CPIO archives overwriting each other seems like overengineering.

There's an obvious use case:

First initramfs.cpio.gz built into the kernel, second initramfs.cpio.gz 
supplied as an external file via the initrd mechanism.  Both get extracted 
into the same rootfs, and I believe external one will overwrite the internal 
one if files conflict.

And yes, there are people out there who want to deploy the same binary kernel 
image across a product line (or at least put each new one through 3 months of 
testing).  And others who want to be able to twiddle the rootfs contents 
without rebuilding the kernel from source each time.  (And of course anybody 
who needs to supply binary firmware to a statically linked device driver like 
ipw2200 is probably pretty happy about the ability to keep it in a separate 
file from the kernel, for license reasons.  Or should be, anyway.)

I'm actually fiddling with a script to let people do this for 
vmlinux->bzImage.  Objcopy with a new init.ramfs section and then go through 
the song and dance to make a bzImage.  Replacing the initramfs _in_ a 
bzimage?  Not fun.  Turning vmlinux into each of the other binary packaging 
types for other platforms?  Also not fun.  (I don't even have a complete list 
of what they all _are_ yet.  Working on it...)

> 	Jeff

Rob
-- 
Never bet against the cheap plastic solution.

Re: [PATCH] initramfs: CPIO unpacking fix

[Jeff Garzik]

Rob Landley wrote:
> First initramfs.cpio.gz built into the kernel, second initramfs.cpio.gz 
> supplied as an external file via the initrd mechanism.  Both get extracted 
> into the same rootfs, and I believe external one will overwrite the internal 
> one if files conflict.

Based on this and HPA's response, I definitely stand corrected.

	Jeff

[PATCH] initramfs: CPIO unpacking fix

[H. Peter Anvin]

Unlink files, symlinks, FIFOs, devices etc. (except directories) before
writing them when extracting CPIOs.  This stops weird behaviour like:
  1) writing through symlinks created in earlier CPIOs. eg foo->bar in
     the first CPIO.  Having foo as a non-link in a subsequent CPIO,
     results in bar being written and foo remaining as a symlink.
  2) if the first version of file foo is larger than foo in a
     subsequent CPIO, we end up with a mix of the two.  ie. neither
     the first or second version of /foo.
  3) special files like devices, fifo etc. can't be overwritten in
     subsequent CPIOS.

With this, the kernel will more closely replicate
   for i in *.cpio; do cpio --extract --unconditional < $i ; done

This is a change but it's regarded as fixing broken functionality.

Signed-off-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: H. Peter Anvin <hpa@zytor.com>

  init/initramfs.c |    3 +++
  1 files changed, 3 insertions(+)

Index: linux-2.6.15/init/initramfs.c
===================================================================
--- linux-2.6.15.orig/init/initramfs.c
+++ linux-2.6.15/init/initramfs.c
@@ -249,6 +249,7 @@ static int __init do_name(void)
  	if (dry_run)
  		return 0;
  	if (S_ISREG(mode)) {
+		sys_unlink(collected);
  		if (maybe_link() >= 0) {
  			wfd = sys_open(collected, O_WRONLY|O_CREAT, mode);
  			if (wfd >= 0) {
@@ -263,6 +264,7 @@ static int __init do_name(void)
  		sys_chmod(collected, mode);
  	} else if (S_ISBLK(mode) || S_ISCHR(mode) ||
  		   S_ISFIFO(mode) || S_ISSOCK(mode)) {
+		sys_unlink(collected);
  		if (maybe_link() == 0) {
  			sys_mknod(collected, mode, rdev);
  			sys_chown(collected, uid, gid);
@@ -291,6 +293,7 @@ static int __init do_copy(void)
  static int __init do_symlink(void)
  {
  	collected[N_ALIGN(name_len) + body_len] = '\0';
+	sys_unlink(collected);
  	sys_symlink(collected + N_ALIGN(name_len), collected);
  	sys_lchown(collected, uid, gid);
  	state = SkipIt;