[PATCH] initramfs: cleanup incomplete rootfs

[PATCH] initramfs: cleanup incomplete rootfs

[David Engraf]

Unpacking an external initrd may fail e.g. not enough memory. This leads
to an incomplete rootfs because some files might be extracted already.
Fixed by cleaning the rootfs so the kernel is not using an incomplete
rootfs.

Signed-off-by: David Engraf <david.engraf@sysgo.com>
---
 init/initramfs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/init/initramfs.c b/init/initramfs.c
index 640557788026..aa3a46cfcb4e 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -548,7 +548,6 @@ static void __init free_initrd(void)
 	initrd_end = 0;
 }
 
-#ifdef CONFIG_BLK_DEV_RAM
 #define BUF_SIZE 1024
 static void __init clean_rootfs(void)
 {
@@ -595,7 +594,6 @@ static void __init clean_rootfs(void)
 	ksys_close(fd);
 	kfree(buf);
 }
-#endif
 
 static int __init populate_rootfs(void)
 {
@@ -638,8 +636,10 @@ static int __init populate_rootfs(void)
 		printk(KERN_INFO "Unpacking initramfs...\n");
 		err = unpack_to_rootfs((char *)initrd_start,
 			initrd_end - initrd_start);
-		if (err)
+		if (err) {
 			printk(KERN_EMERG "Initramfs unpacking failed: %s\n", err);
+			clean_rootfs();
+		}
 		free_initrd();
 #endif
 	}
-- 
2.17.1

[PATCH RESEND] initramfs: cleanup incomplete rootfs

[David Engraf]

Unpacking an external initrd may fail e.g. not enough memory. This leads
to an incomplete rootfs because some files might be extracted already.
Fixed by cleaning the rootfs so the kernel is not using an incomplete
rootfs.

Signed-off-by: David Engraf <david.engraf@sysgo.com>
---
 init/initramfs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/init/initramfs.c b/init/initramfs.c
index 640557788026..aa3a46cfcb4e 100644
--- a/init/initramfs.c
+++ b/init/initramfs.c
@@ -548,7 +548,6 @@ static void __init free_initrd(void)
 	initrd_end = 0;
 }
 
-#ifdef CONFIG_BLK_DEV_RAM
 #define BUF_SIZE 1024
 static void __init clean_rootfs(void)
 {
@@ -595,7 +594,6 @@ static void __init clean_rootfs(void)
 	ksys_close(fd);
 	kfree(buf);
 }
-#endif
 
 static int __init populate_rootfs(void)
 {
@@ -638,8 +636,10 @@ static int __init populate_rootfs(void)
 		printk(KERN_INFO "Unpacking initramfs...\n");
 		err = unpack_to_rootfs((char *)initrd_start,
 			initrd_end - initrd_start);
-		if (err)
+		if (err) {
 			printk(KERN_EMERG "Initramfs unpacking failed: %s\n", err);
+			clean_rootfs();
+		}
 		free_initrd();
 #endif
 	}
-- 
2.17.1

Re: [PATCH RESEND] initramfs: cleanup incomplete rootfs

[Andy Shevchenko]

On Tue, Oct 30, 2018 at 5:22 PM David Engraf <david.engraf@sysgo.com> wrote:
>
> Unpacking an external initrd may fail e.g. not enough memory. This leads
> to an incomplete rootfs because some files might be extracted already.
> Fixed by cleaning the rootfs so the kernel is not using an incomplete
> rootfs.

This breaks my setup where I have U-boot provided more size of
initramfs than needed. This allows a bit of flexibility to increase or
decrease initramfs compressed image without taking care of bootloader.
The proper solution is to do this if we sure that we didn't get enough
memory, otherwise I can't consider the error fatal to clean up rootfs.

Andrew, what it be good idea to revert for now?

> Signed-off-by: David Engraf <david.engraf@sysgo.com>
> ---
>  init/initramfs.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/init/initramfs.c b/init/initramfs.c
> index 640557788026..aa3a46cfcb4e 100644
> --- a/init/initramfs.c
> +++ b/init/initramfs.c
> @@ -548,7 +548,6 @@ static void __init free_initrd(void)
>         initrd_end = 0;
>  }
>
> -#ifdef CONFIG_BLK_DEV_RAM
>  #define BUF_SIZE 1024
>  static void __init clean_rootfs(void)
>  {
> @@ -595,7 +594,6 @@ static void __init clean_rootfs(void)
>         ksys_close(fd);
>         kfree(buf);
>  }
> -#endif
>
>  static int __init populate_rootfs(void)
>  {
> @@ -638,8 +636,10 @@ static int __init populate_rootfs(void)
>                 printk(KERN_INFO "Unpacking initramfs...\n");
>                 err = unpack_to_rootfs((char *)initrd_start,
>                         initrd_end - initrd_start);
> -               if (err)
> +               if (err) {
>                         printk(KERN_EMERG "Initramfs unpacking failed: %s\n", err);
> +                       clean_rootfs();
> +               }
>                 free_initrd();
>  #endif
>         }
> --
> 2.17.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH RESEND] initramfs: cleanup incomplete rootfs

[Andrew Morton]

On Fri, 8 Feb 2019 21:45:21 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, Oct 30, 2018 at 5:22 PM David Engraf <david.engraf@sysgo.com> wrote:
> >
> > Unpacking an external initrd may fail e.g. not enough memory. This leads
> > to an incomplete rootfs because some files might be extracted already.
> > Fixed by cleaning the rootfs so the kernel is not using an incomplete
> > rootfs.
> 
> This breaks my setup where I have U-boot provided more size of
> initramfs than needed. This allows a bit of flexibility to increase or
> decrease initramfs compressed image without taking care of bootloader.
> The proper solution is to do this if we sure that we didn't get enough
> memory, otherwise I can't consider the error fatal to clean up rootfs.

OK, thanks.  Maybe David can suggest a fix - I'll queue up a revert
meanwhile.

I don't really understand the failure.  Why does an oversized initramfs
cause unpack_to_rootfs() to fail?

Re: [PATCH RESEND] initramfs: cleanup incomplete rootfs

[Andy Shevchenko]

On Sat, Feb 9, 2019 at 12:08 AM Andrew Morton <akpm@linux-foundation.org> wrote:
> On Fri, 8 Feb 2019 21:45:21 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Tue, Oct 30, 2018 at 5:22 PM David Engraf <david.engraf@sysgo.com> wrote:
> > >
> > > Unpacking an external initrd may fail e.g. not enough memory. This leads
> > > to an incomplete rootfs because some files might be extracted already.
> > > Fixed by cleaning the rootfs so the kernel is not using an incomplete
> > > rootfs.
> >
> > This breaks my setup where I have U-boot provided more size of
> > initramfs than needed. This allows a bit of flexibility to increase or
> > decrease initramfs compressed image without taking care of bootloader.
> > The proper solution is to do this if we sure that we didn't get enough
> > memory, otherwise I can't consider the error fatal to clean up rootfs.
>
> OK, thanks.  Maybe David can suggest a fix - I'll queue up a revert
> meanwhile.
>
> I don't really understand the failure.  Why does an oversized initramfs
> cause unpack_to_rootfs() to fail?

In my case I have got "Junk in compressed archive". I don't know (I
would check if needed) which exact condition I got  since there are
three places with this message. The file itself smaller than the size
passed through bootparam. So, when decomression is finished
(successfully!) we still have a garbarge in the memory which is not
related to archive. Message per se is okay to have, though I consider
this non-fatal.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH RESEND] initramfs: cleanup incomplete rootfs

[David Engraf]

On 09.02.19 at 11:35, Andy Shevchenko wrote:
> On Sat, Feb 9, 2019 at 12:08 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>> On Fri, 8 Feb 2019 21:45:21 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>> On Tue, Oct 30, 2018 at 5:22 PM David Engraf <david.engraf@sysgo.com> wrote:
>>>>
>>>> Unpacking an external initrd may fail e.g. not enough memory. This leads
>>>> to an incomplete rootfs because some files might be extracted already.
>>>> Fixed by cleaning the rootfs so the kernel is not using an incomplete
>>>> rootfs.
>>>
>>> This breaks my setup where I have U-boot provided more size of
>>> initramfs than needed. This allows a bit of flexibility to increase or
>>> decrease initramfs compressed image without taking care of bootloader.
>>> The proper solution is to do this if we sure that we didn't get enough
>>> memory, otherwise I can't consider the error fatal to clean up rootfs.
>>
>> OK, thanks.  Maybe David can suggest a fix - I'll queue up a revert
>> meanwhile.
>>
>> I don't really understand the failure.  Why does an oversized initramfs
>> cause unpack_to_rootfs() to fail?
> 
> In my case I have got "Junk in compressed archive". I don't know (I
> would check if needed) which exact condition I got  since there are
> three places with this message. The file itself smaller than the size
> passed through bootparam. So, when decomression is finished
> (successfully!) we still have a garbarge in the memory which is not
> related to archive. Message per se is okay to have, though I consider
> this non-fatal.

I can reproduce this special case. The unpacking decompresses the whole 
size instead of checking the archive size. I will have a look how to get 
the real archive size.

Best regards
- David

Re: [PATCH RESEND] initramfs: cleanup incomplete rootfs

[David Engraf]

On 11.02.19 at 08:56, David Engraf wrote:
> On 09.02.19 at 11:35, Andy Shevchenko wrote:
>> On Sat, Feb 9, 2019 at 12:08 AM Andrew Morton 
>> <akpm@linux-foundation.org> wrote:
>>> On Fri, 8 Feb 2019 21:45:21 +0200 Andy Shevchenko 
>>> <andy.shevchenko@gmail.com> wrote:
>>>> On Tue, Oct 30, 2018 at 5:22 PM David Engraf 
>>>> <david.engraf@sysgo.com> wrote:
>>>>>
>>>>> Unpacking an external initrd may fail e.g. not enough memory. This 
>>>>> leads
>>>>> to an incomplete rootfs because some files might be extracted already.
>>>>> Fixed by cleaning the rootfs so the kernel is not using an incomplete
>>>>> rootfs.
>>>>
>>>> This breaks my setup where I have U-boot provided more size of
>>>> initramfs than needed. This allows a bit of flexibility to increase or
>>>> decrease initramfs compressed image without taking care of bootloader.
>>>> The proper solution is to do this if we sure that we didn't get enough
>>>> memory, otherwise I can't consider the error fatal to clean up rootfs.
>>>
>>> OK, thanks.  Maybe David can suggest a fix - I'll queue up a revert
>>> meanwhile.
>>>
>>> I don't really understand the failure.  Why does an oversized initramfs
>>> cause unpack_to_rootfs() to fail?
>>
>> In my case I have got "Junk in compressed archive". I don't know (I
>> would check if needed) which exact condition I got  since there are
>> three places with this message. The file itself smaller than the size
>> passed through bootparam. So, when decomression is finished
>> (successfully!) we still have a garbarge in the memory which is not
>> related to archive. Message per se is okay to have, though I consider
>> this non-fatal.
> 
> I can reproduce this special case. The unpacking decompresses the whole 
> size instead of checking the archive size. I will have a look how to get 
> the real archive size.

I did some checks and manually increased the initramfs size but I always 
get the following kernel panic:

Kernel panic - not syncing: junk in compressed archive
---[ end Kernel panic - not syncing: junk in compressed archive

The panic was not introduced by my patch. Could you please check if you 
get a panic as well or is your rootfs just empty?

I also had a look at the decompression in unpack_to_rootfs(). This 
function already ensures unpacking only the real size of the archive. 
But it is called in a loop, thus it unpacks the first archive and then 
tries to unpack the reset of the data which are garbage in my case.
Is it intended to allow extracting multiple archives as rootfs? If not 
we could remove the loop and unpack only the first archive. Otherwise we 
could ignore errors when the first archive was extracted without errors.

Best regards
- David

Re: [PATCH RESEND] initramfs: cleanup incomplete rootfs

[Andy Shevchenko]

On Mon, Feb 11, 2019 at 10:49 AM David Engraf <david.engraf@sysgo.com> wrote:
> On 11.02.19 at 08:56, David Engraf wrote:
> > On 09.02.19 at 11:35, Andy Shevchenko wrote:
> >> On Sat, Feb 9, 2019 at 12:08 AM Andrew Morton
> >> <akpm@linux-foundation.org> wrote:
> >>> On Fri, 8 Feb 2019 21:45:21 +0200 Andy Shevchenko
> >>> <andy.shevchenko@gmail.com> wrote:
> >>>> On Tue, Oct 30, 2018 at 5:22 PM David Engraf
> >>>> <david.engraf@sysgo.com> wrote:
> >>>>>
> >>>>> Unpacking an external initrd may fail e.g. not enough memory. This
> >>>>> leads
> >>>>> to an incomplete rootfs because some files might be extracted already.
> >>>>> Fixed by cleaning the rootfs so the kernel is not using an incomplete
> >>>>> rootfs.
> >>>>
> >>>> This breaks my setup where I have U-boot provided more size of
> >>>> initramfs than needed. This allows a bit of flexibility to increase or
> >>>> decrease initramfs compressed image without taking care of bootloader.
> >>>> The proper solution is to do this if we sure that we didn't get enough
> >>>> memory, otherwise I can't consider the error fatal to clean up rootfs.
> >>>
> >>> OK, thanks.  Maybe David can suggest a fix - I'll queue up a revert
> >>> meanwhile.
> >>>
> >>> I don't really understand the failure.  Why does an oversized initramfs
> >>> cause unpack_to_rootfs() to fail?
> >>
> >> In my case I have got "Junk in compressed archive". I don't know (I
> >> would check if needed) which exact condition I got  since there are
> >> three places with this message. The file itself smaller than the size
> >> passed through bootparam. So, when decomression is finished
> >> (successfully!) we still have a garbarge in the memory which is not
> >> related to archive. Message per se is okay to have, though I consider
> >> this non-fatal.
> >
> > I can reproduce this special case. The unpacking decompresses the whole
> > size instead of checking the archive size. I will have a look how to get
> > the real archive size.
>
> I did some checks and manually increased the initramfs size but I always
> get the following kernel panic:

We need to be on the same page here.
There are two sizes of initramfs compressed archive:
 1) actual file size;
 2) what is declared by boot loader and provided via boot parameters.

In my case I have the 2) bigger than the actual file size.
Kernel decompresses the initramfs, prints an error that there is junk,
which is understandable and continues to run init, etc.

> Kernel panic - not syncing: junk in compressed archive
> ---[ end Kernel panic - not syncing: junk in compressed archive
>
> The panic was not introduced by my patch. Could you please check if you
> get a panic as well or is your rootfs just empty?

Since your patch applied I get rootfs clean followed by inability to
find working init. So, I have a panic with different reason.

> I also had a look at the decompression in unpack_to_rootfs(). This
> function already ensures unpacking only the real size of the archive.
> But it is called in a loop, thus it unpacks the first archive and then
> tries to unpack the reset of the data which are garbage in my case.

> Is it intended to allow extracting multiple archives as rootfs?

Yes. You can chain up to 64 archives IIRC.

> If not
> we could remove the loop and unpack only the first archive. Otherwise we
> could ignore errors when the first archive was extracted without errors.

Not the first one, but all the first one_s_. Means, that at least one,
when it's first(!),  is decompressed successfully.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH RESEND] initramfs: cleanup incomplete rootfs

[David Engraf]

On 11.02.19 at 12:40, Andy Shevchenko wrote:
> On Mon, Feb 11, 2019 at 10:49 AM David Engraf <david.engraf@sysgo.com> wrote:
>> On 11.02.19 at 08:56, David Engraf wrote:
>>> On 09.02.19 at 11:35, Andy Shevchenko wrote:
>>>> On Sat, Feb 9, 2019 at 12:08 AM Andrew Morton
>>>> <akpm@linux-foundation.org> wrote:
>>>>> On Fri, 8 Feb 2019 21:45:21 +0200 Andy Shevchenko
>>>>> <andy.shevchenko@gmail.com> wrote:
>>>>>> On Tue, Oct 30, 2018 at 5:22 PM David Engraf
>>>>>> <david.engraf@sysgo.com> wrote:
>>>>>>>
>>>>>>> Unpacking an external initrd may fail e.g. not enough memory. This
>>>>>>> leads
>>>>>>> to an incomplete rootfs because some files might be extracted already.
>>>>>>> Fixed by cleaning the rootfs so the kernel is not using an incomplete
>>>>>>> rootfs.
>>>>>>
>>>>>> This breaks my setup where I have U-boot provided more size of
>>>>>> initramfs than needed. This allows a bit of flexibility to increase or
>>>>>> decrease initramfs compressed image without taking care of bootloader.
>>>>>> The proper solution is to do this if we sure that we didn't get enough
>>>>>> memory, otherwise I can't consider the error fatal to clean up rootfs.
>>>>>
>>>>> OK, thanks.  Maybe David can suggest a fix - I'll queue up a revert
>>>>> meanwhile.
>>>>>
>>>>> I don't really understand the failure.  Why does an oversized initramfs
>>>>> cause unpack_to_rootfs() to fail?
>>>>
>>>> In my case I have got "Junk in compressed archive". I don't know (I
>>>> would check if needed) which exact condition I got  since there are
>>>> three places with this message. The file itself smaller than the size
>>>> passed through bootparam. So, when decomression is finished
>>>> (successfully!) we still have a garbarge in the memory which is not
>>>> related to archive. Message per se is okay to have, though I consider
>>>> this non-fatal.
>>>
>>> I can reproduce this special case. The unpacking decompresses the whole
>>> size instead of checking the archive size. I will have a look how to get
>>> the real archive size.
>>
>> I did some checks and manually increased the initramfs size but I always
>> get the following kernel panic:
> 
> We need to be on the same page here.
> There are two sizes of initramfs compressed archive:
>   1) actual file size;
>   2) what is declared by boot loader and provided via boot parameters.
> 
> In my case I have the 2) bigger than the actual file size.
> Kernel decompresses the initramfs, prints an error that there is junk,
> which is understandable and continues to run init, etc.

Ok got it. When the memory behind the actual file size is clear (0x0) 
the decompression doesn't complain and just ignores the padding. Any 
other data will be interpreted as a new archive and thus you'll see the 
error message.

Is it possible for you to fill the padding after the actual file size 
with 0x00 ?

Best regards
- David


>> Kernel panic - not syncing: junk in compressed archive
>> ---[ end Kernel panic - not syncing: junk in compressed archive
>>
>> The panic was not introduced by my patch. Could you please check if you
>> get a panic as well or is your rootfs just empty?
> 
> Since your patch applied I get rootfs clean followed by inability to
> find working init. So, I have a panic with different reason.
> 
>> I also had a look at the decompression in unpack_to_rootfs(). This
>> function already ensures unpacking only the real size of the archive.
>> But it is called in a loop, thus it unpacks the first archive and then
>> tries to unpack the reset of the data which are garbage in my case.
> 
>> Is it intended to allow extracting multiple archives as rootfs?
> 
> Yes. You can chain up to 64 archives IIRC.
> 
>> If not
>> we could remove the loop and unpack only the first archive. Otherwise we
>> could ignore errors when the first archive was extracted without errors.
> 
> Not the first one, but all the first one_s_. Means, that at least one,
> when it's first(!),  is decompressed successfully.
> 

Re: [PATCH RESEND] initramfs: cleanup incomplete rootfs

[Andy Shevchenko]

On Mon, Feb 11, 2019 at 2:40 PM David Engraf <david.engraf@sysgo.com> wrote:
> On 11.02.19 at 12:40, Andy Shevchenko wrote:
> > On Mon, Feb 11, 2019 at 10:49 AM David Engraf <david.engraf@sysgo.com> wrote:
> >> On 11.02.19 at 08:56, David Engraf wrote:
> >>> On 09.02.19 at 11:35, Andy Shevchenko wrote:
> >>>> On Sat, Feb 9, 2019 at 12:08 AM Andrew Morton
> >>>> <akpm@linux-foundation.org> wrote:
> >>>>> On Fri, 8 Feb 2019 21:45:21 +0200 Andy Shevchenko
> >>>>> <andy.shevchenko@gmail.com> wrote:
> >>>>>> On Tue, Oct 30, 2018 at 5:22 PM David Engraf
> >>>>>> <david.engraf@sysgo.com> wrote:

> >>>> In my case I have got "Junk in compressed archive". I don't know (I
> >>>> would check if needed) which exact condition I got  since there are
> >>>> three places with this message. The file itself smaller than the size
> >>>> passed through bootparam. So, when decomression is finished
> >>>> (successfully!) we still have a garbarge in the memory which is not
> >>>> related to archive. Message per se is okay to have, though I consider
> >>>> this non-fatal.
> >>>
> >>> I can reproduce this special case. The unpacking decompresses the whole
> >>> size instead of checking the archive size. I will have a look how to get
> >>> the real archive size.
> >>
> >> I did some checks and manually increased the initramfs size but I always
> >> get the following kernel panic:
> >
> > We need to be on the same page here.
> > There are two sizes of initramfs compressed archive:
> >   1) actual file size;
> >   2) what is declared by boot loader and provided via boot parameters.
> >
> > In my case I have the 2) bigger than the actual file size.
> > Kernel decompresses the initramfs, prints an error that there is junk,
> > which is understandable and continues to run init, etc.
>
> Ok got it. When the memory behind the actual file size is clear (0x0)
> the decompression doesn't complain and just ignores the padding. Any
> other data will be interpreted as a new archive and thus you'll see the
> error message.

Correct.

> Is it possible for you to fill the padding after the actual file size
> with 0x00 ?

Not sure. This is boot loader realm. Even if I patch U-Boot, not every
boot loader will guarantee this.
So, it's fragile to rely on data being 0x00 after actual archive.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH RESEND] initramfs: cleanup incomplete rootfs

[David Engraf]

On 12.02.19 at 11:43, Andy Shevchenko wrote:
> On Mon, Feb 11, 2019 at 2:40 PM David Engraf <david.engraf@sysgo.com> wrote:
>> On 11.02.19 at 12:40, Andy Shevchenko wrote:
>>> On Mon, Feb 11, 2019 at 10:49 AM David Engraf <david.engraf@sysgo.com> wrote:
>>>> On 11.02.19 at 08:56, David Engraf wrote:
>>>>> On 09.02.19 at 11:35, Andy Shevchenko wrote:
>>>>>> On Sat, Feb 9, 2019 at 12:08 AM Andrew Morton
>>>>>> <akpm@linux-foundation.org> wrote:
>>>>>>> On Fri, 8 Feb 2019 21:45:21 +0200 Andy Shevchenko
>>>>>>> <andy.shevchenko@gmail.com> wrote:
>>>>>>>> On Tue, Oct 30, 2018 at 5:22 PM David Engraf
>>>>>>>> <david.engraf@sysgo.com> wrote:
> 
>>>>>> In my case I have got "Junk in compressed archive". I don't know (I
>>>>>> would check if needed) which exact condition I got  since there are
>>>>>> three places with this message. The file itself smaller than the size
>>>>>> passed through bootparam. So, when decomression is finished
>>>>>> (successfully!) we still have a garbarge in the memory which is not
>>>>>> related to archive. Message per se is okay to have, though I consider
>>>>>> this non-fatal.
>>>>>
>>>>> I can reproduce this special case. The unpacking decompresses the whole
>>>>> size instead of checking the archive size. I will have a look how to get
>>>>> the real archive size.
>>>>
>>>> I did some checks and manually increased the initramfs size but I always
>>>> get the following kernel panic:
>>>
>>> We need to be on the same page here.
>>> There are two sizes of initramfs compressed archive:
>>>    1) actual file size;
>>>    2) what is declared by boot loader and provided via boot parameters.
>>>
>>> In my case I have the 2) bigger than the actual file size.
>>> Kernel decompresses the initramfs, prints an error that there is junk,
>>> which is understandable and continues to run init, etc.
>>
>> Ok got it. When the memory behind the actual file size is clear (0x0)
>> the decompression doesn't complain and just ignores the padding. Any
>> other data will be interpreted as a new archive and thus you'll see the
>> error message.
> 
> Correct.
> 
>> Is it possible for you to fill the padding after the actual file size
>> with 0x00 ?
> 
> Not sure. This is boot loader realm. Even if I patch U-Boot, not every
> boot loader will guarantee this.
> So, it's fragile to rely on data being 0x00 after actual archive.

The problem is that the kernel expects another archive if there are data 
left. If these data do not contain a valid magic the kernel prints an 
error message which is correct.

I could make this error not critical and keep the rootfs, but it's still 
an error and unexpected. You're using a modified bootloader which 
reports a size larger than the file itself. Other bootloader will use 
the file size and report the correct size to the kernel. So this 
workaround is required by your setup only.

@Andrew: What do you think about that? Shall I create a workaround for 
the special case?

Best regards
- David

Re: [PATCH RESEND] initramfs: cleanup incomplete rootfs

[Andy Shevchenko]

On Tue, Feb 12, 2019 at 2:12 PM David Engraf <david.engraf@sysgo.com> wrote:
> On 12.02.19 at 11:43, Andy Shevchenko wrote:
> > On Mon, Feb 11, 2019 at 2:40 PM David Engraf <david.engraf@sysgo.com> wrote:
> >> On 11.02.19 at 12:40, Andy Shevchenko wrote:

> >> Ok got it. When the memory behind the actual file size is clear (0x0)
> >> the decompression doesn't complain and just ignores the padding. Any
> >> other data will be interpreted as a new archive and thus you'll see the
> >> error message.
> >
> > Correct.
> >
> >> Is it possible for you to fill the padding after the actual file size
> >> with 0x00 ?
> >
> > Not sure. This is boot loader realm. Even if I patch U-Boot, not every
> > boot loader will guarantee this.
> > So, it's fragile to rely on data being 0x00 after actual archive.
>
> The problem is that the kernel expects another archive if there are data
> left. If these data do not contain a valid magic the kernel prints an
> error message which is correct.

Agree.

> I could make this error not critical and keep the rootfs, but it's still
> an error and unexpected.

I would rather call it a warning and continue.

Perhaps something like

static void warning(const char *msg)
{
  ...print warning...
  return without assigning return value.
}

> You're using a modified bootloader which
> reports a size larger than the file itself.

This is not true. The size supplied by whatever user input through it
command line or configuration.

> Other bootloader will use
> the file size and report the correct size to the kernel.

This follows and thus not true either. It depends on boot loader completely.

> So this
> workaround is required by your setup only.

So this is not true.

> @Andrew: What do you think about that? Shall I create a workaround for
> the special case?
--
With Best Regards,
Andy Shevchenko

Re: [PATCH RESEND] initramfs: cleanup incomplete rootfs

[Andy Shevchenko]

On Tue, Feb 12, 2019 at 3:50 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, Feb 12, 2019 at 2:12 PM David Engraf <david.engraf@sysgo.com> wrote:
> > On 12.02.19 at 11:43, Andy Shevchenko wrote:
> > > On Mon, Feb 11, 2019 at 2:40 PM David Engraf <david.engraf@sysgo.com> wrote:
> > >> On 11.02.19 at 12:40, Andy Shevchenko wrote:
>
> > >> Ok got it. When the memory behind the actual file size is clear (0x0)
> > >> the decompression doesn't complain and just ignores the padding. Any
> > >> other data will be interpreted as a new archive and thus you'll see the
> > >> error message.
> > >
> > > Correct.
> > >
> > >> Is it possible for you to fill the padding after the actual file size
> > >> with 0x00 ?
> > >
> > > Not sure. This is boot loader realm. Even if I patch U-Boot, not every
> > > boot loader will guarantee this.
> > > So, it's fragile to rely on data being 0x00 after actual archive.
> >
> > The problem is that the kernel expects another archive if there are data
> > left. If these data do not contain a valid magic the kernel prints an
> > error message which is correct.
>
> Agree.

JFYI: I had tested your another patch and it prints me the following

[   20.672873] Initramfs unpacking failed: invalid magic at start of
compressed archive

Nevertheless, I can use the system (since revert of this patch).

> > I could make this error not critical and keep the rootfs, but it's still
> > an error and unexpected.
>
> I would rather call it a warning and continue.
>
> Perhaps something like
>
> static void warning(const char *msg)
> {
>   ...print warning...
>   return without assigning return value.
> }


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH RESEND] initramfs: cleanup incomplete rootfs

[Andrew Morton]

On Sat, 9 Feb 2019 12:35:03 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sat, Feb 9, 2019 at 12:08 AM Andrew Morton <akpm@linux-foundation.org> wrote:
> > On Fri, 8 Feb 2019 21:45:21 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > On Tue, Oct 30, 2018 at 5:22 PM David Engraf <david.engraf@sysgo.com> wrote:
> > > >
> > > > Unpacking an external initrd may fail e.g. not enough memory. This leads
> > > > to an incomplete rootfs because some files might be extracted already.
> > > > Fixed by cleaning the rootfs so the kernel is not using an incomplete
> > > > rootfs.
> > >
> > > This breaks my setup where I have U-boot provided more size of
> > > initramfs than needed. This allows a bit of flexibility to increase or
> > > decrease initramfs compressed image without taking care of bootloader.
> > > The proper solution is to do this if we sure that we didn't get enough
> > > memory, otherwise I can't consider the error fatal to clean up rootfs.
> >
> > OK, thanks.  Maybe David can suggest a fix - I'll queue up a revert
> > meanwhile.
> >
> > I don't really understand the failure.  Why does an oversized initramfs
> > cause unpack_to_rootfs() to fail?
> 
> In my case I have got "Junk in compressed archive". I don't know (I
> would check if needed) which exact condition I got  since there are
> three places with this message.

Well that's a plain irritating screwup right there.  Could someone
please cook up a patch to give us three distinct (and hopefully more
informative) error messages?

Re: [PATCH RESEND] initramfs: cleanup incomplete rootfs

[David Engraf]

On 12.02.19 at 01:56, Andrew Morton wrote:
> On Sat, 9 Feb 2019 12:35:03 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
>> On Sat, Feb 9, 2019 at 12:08 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>>> On Fri, 8 Feb 2019 21:45:21 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>>> On Tue, Oct 30, 2018 at 5:22 PM David Engraf <david.engraf@sysgo.com> wrote:
>>>>>
>>>>> Unpacking an external initrd may fail e.g. not enough memory. This leads
>>>>> to an incomplete rootfs because some files might be extracted already.
>>>>> Fixed by cleaning the rootfs so the kernel is not using an incomplete
>>>>> rootfs.
>>>>
>>>> This breaks my setup where I have U-boot provided more size of
>>>> initramfs than needed. This allows a bit of flexibility to increase or
>>>> decrease initramfs compressed image without taking care of bootloader.
>>>> The proper solution is to do this if we sure that we didn't get enough
>>>> memory, otherwise I can't consider the error fatal to clean up rootfs.
>>>
>>> OK, thanks.  Maybe David can suggest a fix - I'll queue up a revert
>>> meanwhile.
>>>
>>> I don't really understand the failure.  Why does an oversized initramfs
>>> cause unpack_to_rootfs() to fail?
>>
>> In my case I have got "Junk in compressed archive". I don't know (I
>> would check if needed) which exact condition I got  since there are
>> three places with this message.
> 
> Well that's a plain irritating screwup right there.  Could someone
> please cook up a patch to give us three distinct (and hopefully more
> informative) error messages?

Done. BTW "invalid magic at start of compressed archive" is the error we 
get with the patch.

Best regards
- David