[PATCH] initramfs: print error and shell out for unsupported content

[PATCH] initramfs: print error and shell out for unsupported content

[Alexander Holler]

The initramfs generation is broken for file and directory names which contain
colons or spaces. Print an error and don't try to continue.

Tests:

cd linux
make defconfig
echo 'CONFIG_BLK_DEV_INITRD=y' >> .config
echo 'CONFIG_INITRAMFS_ROOT_UID=0' >>.config
echo 'CONFIG_INITRAMFS_ROOT_GID=0' >>.config
echo 'CONFIG_INITRAMFS_COMPRESSION_NONE=y' >>.config
echo 'CONFIG_INITRAMFS_SOURCE="/tmp/bugroot"' >>.config

Problem with colons:

mkdir -p /tmp/bugroot/a:b
make -j4 bzImage # no error
make bzImage # try again, oops

Problem with spaces:

mkdir -p /tmp/bugroot/a\ b
make -j4 bzImage # no error
zcat usr/initramfs_data.cpio.gz | cpio --extract --list # oops, no content

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 scripts/gen_initramfs_list.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/scripts/gen_initramfs_list.sh b/scripts/gen_initramfs_list.sh
index 17fa901..aacf366 100644
--- a/scripts/gen_initramfs_list.sh
+++ b/scripts/gen_initramfs_list.sh
@@ -171,6 +171,17 @@ dir_filelist() {
 	${dep_list}header "$1"
 
 	srcdir=$(echo "$1" | sed -e 's://*:/:g')
+
+	# Files and directories with spaces and colons are unsupported.
+	local unsupported=$(find "${srcdir}" -regex '.*\(:\| \).*')
+	if [ ! -z "${unsupported}" ]; then
+		printf "ERROR: Unable to handle files/directories with " >&2
+		printf "unsupported characters.\nPlease use other ways " >&2
+		printf "generate a cpio-archive.\nUnsupported files and " >&2
+		printf "directories are:\n$unsupported\n" >&2
+		exit 1
+	fi
+
 	dirlist=$(find "${srcdir}" -printf "%p %m %U %G\n")
 
 	# If $dirlist is only one line, then the directory is empty
-- 
1.8.3.1

Re: [PATCH] initramfs: print error and shell out for unsupported content

[Levente Kurusa]

Hi,

2014-03-20 22:35 GMT+01:00 Alexander Holler <holler@ahsoftware.de>:
> The initramfs generation is broken for file and directory names which contain
> colons or spaces. Print an error and don't try to continue.
>
> Tests:
>
> cd linux
> make defconfig
> echo 'CONFIG_BLK_DEV_INITRD=y' >> .config
> echo 'CONFIG_INITRAMFS_ROOT_UID=0' >>.config
> echo 'CONFIG_INITRAMFS_ROOT_GID=0' >>.config
> echo 'CONFIG_INITRAMFS_COMPRESSION_NONE=y' >>.config
> echo 'CONFIG_INITRAMFS_SOURCE="/tmp/bugroot"' >>.config
>
> Problem with colons:
>
> mkdir -p /tmp/bugroot/a:b
> make -j4 bzImage # no error
> make bzImage # try again, oops
>
> Problem with spaces:
>
> mkdir -p /tmp/bugroot/a\ b
> make -j4 bzImage # no error
> zcat usr/initramfs_data.cpio.gz | cpio --extract --list # oops, no content
>
> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
> ---
>  scripts/gen_initramfs_list.sh | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/scripts/gen_initramfs_list.sh b/scripts/gen_initramfs_list.sh
> index 17fa901..aacf366 100644
> --- a/scripts/gen_initramfs_list.sh
> +++ b/scripts/gen_initramfs_list.sh
> @@ -171,6 +171,17 @@ dir_filelist() {
>         ${dep_list}header "$1"
>
>         srcdir=$(echo "$1" | sed -e 's://*:/:g')
> +
> +       # Files and directories with spaces and colons are unsupported.
> +       local unsupported=$(find "${srcdir}" -regex '.*\(:\| \).*')
> +       if [ ! -z "${unsupported}" ]; then
> +               printf "ERROR: Unable to handle files/directories with " >&2
> +               printf "unsupported characters.\nPlease use other ways " >&2

'Please use other ways to...'
(i.e. missing "to")

> +               printf "generate a cpio-archive.\nUnsupported files and " >&2
> +               printf "directories are:\n$unsupported\n" >&2
> +               exit 1
> +       fi
> +

I think it would be worthy to tell the user what kind of characters
are unsupported. For instance, tell them that 'spaces' are unsupported.


>         dirlist=$(find "${srcdir}" -printf "%p %m %U %G\n")
>
>         # If $dirlist is only one line, then the directory is empty
> [...]

--
Regards,
Levente Kurusa

[PATCH v2] initramfs: print error and shell out for unsupported content

[Alexander Holler]

The initramfs generation is broken for file and directory names which contain
colons or spaces. Print an error and don't try to continue.

Tests:

cd linux
make defconfig
echo 'CONFIG_BLK_DEV_INITRD=y' >> .config
echo 'CONFIG_INITRAMFS_ROOT_UID=0' >>.config
echo 'CONFIG_INITRAMFS_ROOT_GID=0' >>.config
echo 'CONFIG_INITRAMFS_COMPRESSION_NONE=y' >>.config
echo 'CONFIG_INITRAMFS_SOURCE="/tmp/bugroot"' >>.config

Problem with colons:

mkdir -p /tmp/bugroot/a:b
make -j4 bzImage # no error
make bzImage # try again, oops

Problem with spaces:

mkdir -p /tmp/bugroot/a\ b
make -j4 bzImage # no error
zcat usr/initramfs_data.cpio.gz | cpio --extract --list # oops, no content

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 Silly and trivial changes in v2: Fixed a typo and added 'colons and spaces'

 scripts/gen_initramfs_list.sh | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/scripts/gen_initramfs_list.sh b/scripts/gen_initramfs_list.sh
index 17fa901..8f4c145 100644
--- a/scripts/gen_initramfs_list.sh
+++ b/scripts/gen_initramfs_list.sh
@@ -171,6 +171,18 @@ dir_filelist() {
 	${dep_list}header "$1"
 
 	srcdir=$(echo "$1" | sed -e 's://*:/:g')
+
+	# Files and directories with spaces and colons are unsupported.
+	local unsupported=$(find "${srcdir}" -regex '.*\(:\| \).*')
+	if [ ! -z "${unsupported}" ]; then
+		printf "ERROR: Unable to handle files/directories with " >&2
+		printf "unsupported characters (spaces and colons).\n" >&2
+		printf "Please use other ways to generate a cpio-archive. " >&2
+		printf "Unsupported files and directories are:\n" >&2
+		printf "$unsupported\n" >&2
+		exit 1
+	fi
+
 	dirlist=$(find "${srcdir}" -printf "%p %m %U %G\n")
 
 	# If $dirlist is only one line, then the directory is empty
-- 
1.8.3.1

Re: [PATCH v2] initramfs: print error and shell out for unsupported content

[Alexander Holler]

Am 20.03.2014 23:00, schrieb Alexander Holler:
> The initramfs generation is broken for file and directory names which contain
> colons or spaces. Print an error and don't try to continue.

I will not send a v3, but I think this is a candidate for the stable 
kernels too, as the problem might drive people crazy. Therefor;

Cc: <stable@vger.kernel.org>

Regards,

Alexander Holler

Re: [PATCH v2] initramfs: print error and shell out for unsupported content

[Andrew Morton]

On Thu, 20 Mar 2014 23:00:45 +0100 Alexander Holler <holler@ahsoftware.de> wrote:

> The initramfs generation is broken for file and directory names which contain
> colons or spaces. Print an error and don't try to continue.
> 
> Tests:
> 
> cd linux
> make defconfig
> echo 'CONFIG_BLK_DEV_INITRD=y' >> .config
> echo 'CONFIG_INITRAMFS_ROOT_UID=0' >>.config
> echo 'CONFIG_INITRAMFS_ROOT_GID=0' >>.config
> echo 'CONFIG_INITRAMFS_COMPRESSION_NONE=y' >>.config
> echo 'CONFIG_INITRAMFS_SOURCE="/tmp/bugroot"' >>.config
> 
> Problem with colons:
> 
> mkdir -p /tmp/bugroot/a:b
> make -j4 bzImage # no error
> make bzImage # try again, oops
> 
> Problem with spaces:
> 
> mkdir -p /tmp/bugroot/a\ b
> make -j4 bzImage # no error
> zcat usr/initramfs_data.cpio.gz | cpio --extract --list # oops, no content
> 
> ...
>
> --- a/scripts/gen_initramfs_list.sh
> +++ b/scripts/gen_initramfs_list.sh
> @@ -171,6 +171,18 @@ dir_filelist() {
>  	${dep_list}header "$1"
>  
>  	srcdir=$(echo "$1" | sed -e 's://*:/:g')
> +
> +	# Files and directories with spaces and colons are unsupported.
> +	local unsupported=$(find "${srcdir}" -regex '.*\(:\| \).*')
> +	if [ ! -z "${unsupported}" ]; then
> +		printf "ERROR: Unable to handle files/directories with " >&2
> +		printf "unsupported characters (spaces and colons).\n" >&2
> +		printf "Please use other ways to generate a cpio-archive. " >&2
> +		printf "Unsupported files and directories are:\n" >&2
> +		printf "$unsupported\n" >&2
> +		exit 1
> +	fi
> +

It would be better to fix the it-doesnt-work-with-all-filenames bug. 
Any details on that?

Re: [PATCH v2] initramfs: print error and shell out for unsupported content

[Alexander Holler]

Am 21.03.2014 22:03, schrieb Andrew Morton:
> On Thu, 20 Mar 2014 23:00:45 +0100 Alexander Holler <holler@ahsoftware.de> wrote:
> 
>> The initramfs generation is broken for file and directory names which contain
>> colons or spaces. Print an error and don't try to continue.

> It would be better to fix the it-doesnt-work-with-all-filenames bug. 
> Any details on that?

IMHO not worth the time. The whole process which is curently used is
extremly fragile.

E.g it's almost guaranteed to fail trying to include arbitrary filenames
as dependencies in a Makefile. Besides the one problem I've discoverd
with colons, there could be much more things happen, e.g. with filenames
which do include other special Makefile characters you all would have to
escape correctly.

And the problem with spaces isn't as easy to fix as it first does look
like. I think it might be easier to write the whole stuff new instead of
trying to escape the spaces in various ways needed to end up correctly
in the cpio (it first goes through shell code and is then feeded as some
list to a C program).

And I think that just isn't worth the time. Using find | cpio works just
fine to generate a cpio archive and usually an initramfs just contains
some megabytes. So it isn't a problem at all to rebuild the complete
cpio archive with every call of make, it doesn't need much more than
about a second or similiar on almost any machine.

And for the records, I indeed had a deeper look, trying to fix it. But,
as said, quickly realized that it will need too much effort and doesn't
make sense, if it will be doable correctly at all.

Regards,

Alexander Holler

Re: [PATCH v2] initramfs: print error and shell out for unsupported content

[Andrew Morton]

On Fri, 21 Mar 2014 23:49:57 +0100 Alexander Holler <holler@ahsoftware.de> wrote:

> Am 21.03.2014 22:03, schrieb Andrew Morton:
> > On Thu, 20 Mar 2014 23:00:45 +0100 Alexander Holler <holler@ahsoftware.de> wrote:
> > 
> >> The initramfs generation is broken for file and directory names which contain
> >> colons or spaces. Print an error and don't try to continue.
> 
> > It would be better to fix the it-doesnt-work-with-all-filenames bug. 
> > Any details on that?
> 
> IMHO not worth the time. The whole process which is curently used is
> extremly fragile.
> 
> E.g it's almost guaranteed to fail trying to include arbitrary filenames
> as dependencies in a Makefile. Besides the one problem I've discoverd
> with colons, there could be much more things happen, e.g. with filenames
> which do include other special Makefile characters you all would have to
> escape correctly.
> 
> And the problem with spaces isn't as easy to fix as it first does look
> like. I think it might be easier to write the whole stuff new instead of
> trying to escape the spaces in various ways needed to end up correctly
> in the cpio (it first goes through shell code and is then feeded as some
> list to a C program).
> 
> And I think that just isn't worth the time. Using find | cpio works just
> fine to generate a cpio archive and usually an initramfs just contains
> some megabytes. So it isn't a problem at all to rebuild the complete
> cpio archive with every call of make, it doesn't need much more than
> about a second or similiar on almost any machine.
> 
> And for the records, I indeed had a deeper look, trying to fix it. But,
> as said, quickly realized that it will need too much effort and doesn't
> make sense, if it will be doable correctly at all.
> 

huh, OK.

Should we check for \t and \n as well?

Re: [PATCH v2] initramfs: print error and shell out for unsupported content

[Alexander Holler]

Am 21.03.2014 23:55, schrieb Andrew Morton:
> On Fri, 21 Mar 2014 23:49:57 +0100 Alexander Holler <holler@ahsoftware.de> wrote:
> 
>> Am 21.03.2014 22:03, schrieb Andrew Morton:
>>> On Thu, 20 Mar 2014 23:00:45 +0100 Alexander Holler <holler@ahsoftware.de> wrote:
>>>
>>>> The initramfs generation is broken for file and directory names which contain
>>>> colons or spaces. Print an error and don't try to continue.
>>
>>> It would be better to fix the it-doesnt-work-with-all-filenames bug. 
>>> Any details on that?
>>
>> IMHO not worth the time. The whole process which is curently used is
>> extremly fragile.
>>
>> E.g it's almost guaranteed to fail trying to include arbitrary filenames
>> as dependencies in a Makefile. Besides the one problem I've discoverd
>> with colons, there could be much more things happen, e.g. with filenames
>> which do include other special Makefile characters you all would have to
>> escape correctly.
>>
>> And the problem with spaces isn't as easy to fix as it first does look
>> like. I think it might be easier to write the whole stuff new instead of
>> trying to escape the spaces in various ways needed to end up correctly
>> in the cpio (it first goes through shell code and is then feeded as some
>> list to a C program).
>>
>> And I think that just isn't worth the time. Using find | cpio works just
>> fine to generate a cpio archive and usually an initramfs just contains
>> some megabytes. So it isn't a problem at all to rebuild the complete
>> cpio archive with every call of make, it doesn't need much more than
>> about a second or similiar on almost any machine.
>>
>> And for the records, I indeed had a deeper look, trying to fix it. But,
>> as said, quickly realized that it will need too much effort and doesn't
>> make sense, if it will be doable correctly at all.
>>
> 
> huh, OK.
> 
> Should we check for \t and \n as well?

Hmm, maybe. But usually there aren't filenames wich do contain those
characters, and if you want to break (or exploit) the kernel build
process, there are easier ways. But colons and spaces are more widely
used, e.g. the colons in my initramfs were generated by bluez (look at
/var/lib/bluetooth).

I think the current process is good enough for most stuff one wants to
put into an initramfs, and it has the great feature of the uid/guid
translation.
So just a quick check to avoid the most basic problems should be ok. And
I don't really see a need to check for \t and \n too, because nobody
sane uses them in filenames. But ok, that just would be a few chars more
in the regex for find. ;)

I leave that up to you.

Regards,

Alexander Holler

Re: [PATCH v2] initramfs: print error and shell out for unsupported content

[Alexander Holler]

Am 22.03.2014 00:07, schrieb Alexander Holler:
> Am 21.03.2014 23:55, schrieb Andrew Morton:

>> Should we check for \t and \n as well?
>
> Hmm, maybe. But usually there aren't filenames wich do contain those
> characters, and if you want to break (or exploit) the kernel build
> process, there are easier ways. But colons and spaces are more widely
> used, e.g. the colons in my initramfs were generated by bluez (look at
> /var/lib/bluetooth).
>
> I think the current process is good enough for most stuff one wants to
> put into an initramfs, and it has the great feature of the uid/guid
> translation.
> So just a quick check to avoid the most basic problems should be ok. And
> I don't really see a need to check for \t and \n too, because nobody
> sane uses them in filenames. But ok, that just would be a few chars more
> in the regex for find. ;)
>
> I leave that up to you.

Sorry for that answer, but I dislike the process which turns patch 
posters into remote keyboards quiet a lot.

When I post a patch I usually already have spend time to discover, 
investigate, describe, fix the problem (as good as I can or I'm 
willing), write a checkpatch conforming patch, and test that patch, 
which most of the time sums up to several hours (I didn't want to spend 
at all).

And then all kind of comments are arriving to change this and that and 
fix that typo or remove that space and the patch ping pong starts until 
an arbitrary maintainer or reviewer is happy.

I think the process should be more like that:

- patch posted
(- comments from maintainer)
- original patch commited
- patch from maintainer or reviewer on top of the original patch commited

That would give credits to both, the original patch poster and the 
maintainer or reviewer (for his changes).

Of course, that isn't an ideal solution, but I would like it a lot, if 
such a process would at least be seen as a possible and reasonable way, 
even if it means that two patches instead of one do end up in the 
repository.
To keep things together, maybe the second patch could be marked as 
[maintainer patch] or [reviewer patch], that would make it clear that 
both patches tie together and should be reverted both, if needed.

Anyway, I'm drifting offtopic, but that was something I always wanted to 
suggest.

But the real reason why I've started to write this mail, is that it 
might make sense to delete usr/.initramfs_data.cpio.d with a make 
(dist)clean. That file is the reason why the bug with colons might drive 
people crazy, because not even a make distclean (or git clean -df) will 
make make working again after a broken usr/.initramfs_data.cpio.d was 
build (with colons in filenames).

Maybe I will post a patch for that too, if I'm willing to do and test it 
(together with a v3 of the patch which checks for \t \n and \r too).

Regards,

Alexander Holler

[PATCH 1/2] initramfs: don't include filenames from the initramfs for make goals (dist)clean

[Alexander Holler]

Various unusual filenames might break the make process when they are included
as dependencies (e.g. filenames with a colon).

This has the disadvantage that not even a make (dist)clean does work afterwards.
What makes the matter worse is that git clean -df doesn't remove such a possible
broken dependency list too.

Avoid that by not including this dependency list for make goals (dist)clean.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
Cc: <stable@vger.kernel.org>
---
 usr/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/usr/Makefile b/usr/Makefile
index e767f01..5d5b6aa 100644
--- a/usr/Makefile
+++ b/usr/Makefile
@@ -49,9 +49,11 @@ ramfs-args  := \
 # in initramfs and to detect if any files are added/removed.
 # Removed files are identified by directory timestamp being updated
 # The dependency list is generated by gen_initramfs.sh -l
+ifndef clean
 ifneq ($(wildcard $(obj)/.initramfs_data.cpio.d),)
 	include $(obj)/.initramfs_data.cpio.d
 endif
+endif
 
 quiet_cmd_initfs = GEN     $@
       cmd_initfs = $(initramfs) -o $@ $(ramfs-args) $(ramfs-input)
-- 
1.8.3.1

[PATCH 2/2 v3] initramfs: print error and shell out for unsupported content

[Alexander Holler]

The initramfs generation is broken for file and directory names which contain
colons, spaces and other unusual characters. Print an error and don't try to
continue.

Some tests:

cd linux
make defconfig
echo 'CONFIG_BLK_DEV_INITRD=y' >> .config
echo 'CONFIG_INITRAMFS_ROOT_UID=0' >>.config
echo 'CONFIG_INITRAMFS_ROOT_GID=0' >>.config
echo 'CONFIG_INITRAMFS_COMPRESSION_NONE=y' >>.config
echo 'CONFIG_INITRAMFS_SOURCE="/tmp/bugroot"' >>.config

Problem with colons:

mkdir -p /tmp/bugroot/a:b
make -j4 bzImage # no error
make bzImage # try again, oops

Problem with spaces:

mkdir -p /tmp/bugroot/a\ b
make -j4 bzImage # no error
zcat usr/initramfs_data.cpio.gz | cpio --extract --list # oops, no content

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
Cc: <stable@vger.kernel.org>
---
 Changes in v2, v3: check for \n, \r and \t too, updated error msg.

 scripts/gen_initramfs_list.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/scripts/gen_initramfs_list.sh b/scripts/gen_initramfs_list.sh
index 17fa901..2c613e7 100644
--- a/scripts/gen_initramfs_list.sh
+++ b/scripts/gen_initramfs_list.sh
@@ -171,6 +171,19 @@ dir_filelist() {
 	${dep_list}header "$1"
 
 	srcdir=$(echo "$1" | sed -e 's://*:/:g')
+
+	# Files and directories with spaces and colons are unsupported.
+	local unsupported=$(find "${srcdir}" -regex '.*\(:\| \|\n\|\r\|\t\).*')
+	if [ ! -z "${unsupported}" ]; then
+		printf "ERROR: Unable to handle files/directories with " >&2
+		printf "unsupported characters (spaces, :, \\\n, \\\r, " >&2
+		printf "\\\t).\n Please use other ways to generate a " >&2
+		printf "cpio-archive with such names.\n" >&2
+		printf "Unsupported files and directories are:\n" >&2
+		printf "$unsupported\n" >&2
+		exit 1
+	fi
+
 	dirlist=$(find "${srcdir}" -printf "%p %m %U %G\n")
 
 	# If $dirlist is only one line, then the directory is empty
-- 
1.8.3.1

Re: [PATCH 2/2 v3] initramfs: print error and shell out for unsupported content

[Alexander Holler]

Am 22.03.2014 13:29, schrieb Alexander Holler:
> The initramfs generation is broken for file and directory names which contain
> colons, spaces and other unusual characters. Print an error and don't try to
> continue.

(...)
> +	# Files and directories with spaces and colons are unsupported.
> +	local unsupported=$(find "${srcdir}" -regex '.*\(:\| \|\n\|\r\|\t\).*')

I've just noted that -regex isn't POSIX. I don't know the kernel rules 
regarding this, and I don't care. But it might be a blocker for this patch.

Regards,

Alexander Holler

Re: [PATCH v2] initramfs: print error and shell out for unsupported content

[Alexander Holler]

Am 22.03.2014 00:07, schrieb Alexander Holler:
> Am 21.03.2014 23:55, schrieb Andrew Morton:
>> On Fri, 21 Mar 2014 23:49:57 +0100 Alexander Holler <holler@ahsoftware.de> wrote:
>>
>>> Am 21.03.2014 22:03, schrieb Andrew Morton:
>>>> On Thu, 20 Mar 2014 23:00:45 +0100 Alexander Holler <holler@ahsoftware.de> wrote:
>>>>
>>>>> The initramfs generation is broken for file and directory names which contain
>>>>> colons or spaces. Print an error and don't try to continue.
>>>
>>>> It would be better to fix the it-doesnt-work-with-all-filenames bug.
>>>> Any details on that?
>>>
>>> IMHO not worth the time. The whole process which is curently used is
>>> extremly fragile.
>>>
>>> E.g it's almost guaranteed to fail trying to include arbitrary filenames
>>> as dependencies in a Makefile. Besides the one problem I've discoverd
>>> with colons, there could be much more things happen, e.g. with filenames
>>> which do include other special Makefile characters you all would have to
>>> escape correctly.

To be a bit more verbose, (gnu)make doesn't support quoted filenames. 
That means one has to escape all kind of characters which might have a 
special meaning for make. Not really doable. Furthermore escaped 
characters are not very well supported by make and are usable only by a 
few rules.

>>> And the problem with spaces isn't as easy to fix as it first does look
>>> like. I think it might be easier to write the whole stuff new instead of
>>> trying to escape the spaces in various ways needed to end up correctly
>>> in the cpio (it first goes through shell code and is then feeded as some
>>> list to a C program).

To be a bit more verbose here, the shell script 
(scripts/gen_initramfs_list.sh) identifes filenames based on the 
position in a string which is delimited by the usual (shell) whitespace. 
E.g. $1 = filename, $2 = mode, ...
And this stuff is then feeded to usr/gen_init_cpio.c,which uses a 
similiar approach like

sscanf(line, "%" str(PATH_MAX) "s %" str(PATH_MAX) "s %o %d %d", name, 
target, ...

which obviously fails for filenames with whitspace too.

What I think might be reasonable is:

- get rid of the dependency list in form of a include into the Makefile 
and just generate the cpio-archive every time make is called. Common 
initramfs sizes are about a few megabytes and with today machines such a 
cpio-archive is build in about a second,

- get rid of gen_initramfs_list.sh and rewrite gen_init_cpio.c such that 
it reads the filenames, modes and similiar itself (e.g. by using stat(2)).


But I don't have any motivation to do so. Sorry.


Maybe Levente Kurusa is interested in doing so, he seems to have an 
interest in the topic as he was quiet fast to send an answer to my patch.


Regards,

Alexander Holler

Re: [PATCH v2] initramfs: print error and shell out for unsupported content

[Levente Kurusa]

Hi,

2014-03-26 22:16 GMT+01:00 Alexander Holler <holler@ahsoftware.de>:
> Am 22.03.2014 00:07, schrieb Alexander Holler:
>>
>> Am 21.03.2014 23:55, schrieb Andrew Morton:
>>
>>> On Fri, 21 Mar 2014 23:49:57 +0100 Alexander Holler
>>> <holler@ahsoftware.de> wrote:
>>>
>>>> Am 21.03.2014 22:03, schrieb Andrew Morton:
>>>>>
>>>>> On Thu, 20 Mar 2014 23:00:45 +0100 Alexander Holler
>>>>> <holler@ahsoftware.de> wrote:
>>>>>
>>>>>> The initramfs generation is broken for file and directory names which
>>>>>> contain
>>>>>> colons or spaces. Print an error and don't try to continue.
>>>>
>>>>
>>>>> It would be better to fix the it-doesnt-work-with-all-filenames bug.
>>>>> Any details on that?
>>>>
>>>>
>>>> IMHO not worth the time. The whole process which is curently used is
>>>> extremly fragile.
>>>>
>>>> E.g it's almost guaranteed to fail trying to include arbitrary filenames
>>>> as dependencies in a Makefile. Besides the one problem I've discoverd
>>>> with colons, there could be much more things happen, e.g. with filenames
>>>> which do include other special Makefile characters you all would have to
>>>> escape correctly.
>
>
> To be a bit more verbose, (gnu)make doesn't support quoted filenames. That
> means one has to escape all kind of characters which might have a special
> meaning for make. Not really doable. Furthermore escaped characters are not
> very well supported by make and are usable only by a few rules.
>
>
>>>> And the problem with spaces isn't as easy to fix as it first does look
>>>> like. I think it might be easier to write the whole stuff new instead of
>>>> trying to escape the spaces in various ways needed to end up correctly
>>>> in the cpio (it first goes through shell code and is then feeded as some
>>>> list to a C program).
>
>
> To be a bit more verbose here, the shell script
> (scripts/gen_initramfs_list.sh) identifes filenames based on the position in
> a string which is delimited by the usual (shell) whitespace. E.g. $1 =
> filename, $2 = mode, ...
> And this stuff is then feeded to usr/gen_init_cpio.c,which uses a similiar
> approach like
>
> sscanf(line, "%" str(PATH_MAX) "s %" str(PATH_MAX) "s %o %d %d", name,
> target, ...
>
> which obviously fails for filenames with whitspace too.
>
> What I think might be reasonable is:
>
> - get rid of the dependency list in form of a include into the Makefile and
> just generate the cpio-archive every time make is called. Common initramfs
> sizes are about a few megabytes and with today machines such a cpio-archive
> is build in about a second,
>

I don't understand what kind of include would you want.

> - get rid of gen_initramfs_list.sh and rewrite gen_init_cpio.c such that it
> reads the filenames, modes and similiar itself (e.g. by using stat(2)).

This is walkable but probably not worth the effort. Besides, why would
anyone want to put spaces, colons and arbitrary characters to filenames
in the initramfs?

>
>
> But I don't have any motivation to do so. Sorry.
>
> Maybe Levente Kurusa is interested in doing so, he seems to have an interest
> in the topic as he was quiet fast to send an answer to my patch.

All I wonder now is why are you being so arrogant. You are not a 'remote
keyboard' as you had called yourself, since when you send a patch
you not only have to send it but communicate WHY maintainers
should take that patch, and if somebody has a comment
spotting an obvious mistake you shouldn't become angry.
It was only a simple typo and a suggestion. Nobody told you
to blindly accept my suggestion. Everybody can have opinions.
It's open-source, the community suggests you new changes,
it's normal, get used to it. Writing up this nonsense sentence
(I feel the irony) probably took more time than actually resending
the patch did.

Just my two cents.

--
Regards,
Levente Kurusa

Re: [PATCH v2] initramfs: print error and shell out for unsupported content

[Alexander Holler]

Am 26.03.2014 22:38, schrieb Levente Kurusa:
> Hi,
>
> 2014-03-26 22:16 GMT+01:00 Alexander Holler <holler@ahsoftware.de>:
>> Am 22.03.2014 00:07, schrieb Alexander Holler:
>>>
>>> Am 21.03.2014 23:55, schrieb Andrew Morton:
>>>
>>>> On Fri, 21 Mar 2014 23:49:57 +0100 Alexander Holler
>>>> <holler@ahsoftware.de> wrote:
>>>>
>>>>> Am 21.03.2014 22:03, schrieb Andrew Morton:
>>>>>>
>>>>>> On Thu, 20 Mar 2014 23:00:45 +0100 Alexander Holler
>>>>>> <holler@ahsoftware.de> wrote:
>>>>>>
>>>>>>> The initramfs generation is broken for file and directory names which
>>>>>>> contain
>>>>>>> colons or spaces. Print an error and don't try to continue.
>>>>>
>>>>>
>>>>>> It would be better to fix the it-doesnt-work-with-all-filenames bug.
>>>>>> Any details on that?
>>>>>
>>>>>
>>>>> IMHO not worth the time. The whole process which is curently used is
>>>>> extremly fragile.
>>>>>
>>>>> E.g it's almost guaranteed to fail trying to include arbitrary filenames
>>>>> as dependencies in a Makefile. Besides the one problem I've discoverd
>>>>> with colons, there could be much more things happen, e.g. with filenames
>>>>> which do include other special Makefile characters you all would have to
>>>>> escape correctly.
>>
>>
>> To be a bit more verbose, (gnu)make doesn't support quoted filenames. That
>> means one has to escape all kind of characters which might have a special
>> meaning for make. Not really doable. Furthermore escaped characters are not
>> very well supported by make and are usable only by a few rules.
>>
>>
>>>>> And the problem with spaces isn't as easy to fix as it first does look
>>>>> like. I think it might be easier to write the whole stuff new instead of
>>>>> trying to escape the spaces in various ways needed to end up correctly
>>>>> in the cpio (it first goes through shell code and is then feeded as some
>>>>> list to a C program).
>>
>>
>> To be a bit more verbose here, the shell script
>> (scripts/gen_initramfs_list.sh) identifes filenames based on the position in
>> a string which is delimited by the usual (shell) whitespace. E.g. $1 =
>> filename, $2 = mode, ...
>> And this stuff is then feeded to usr/gen_init_cpio.c,which uses a similiar
>> approach like
>>
>> sscanf(line, "%" str(PATH_MAX) "s %" str(PATH_MAX) "s %o %d %d", name,
>> target, ...
>>
>> which obviously fails for filenames with whitspace too.
>>
>> What I think might be reasonable is:
>>
>> - get rid of the dependency list in form of a include into the Makefile and
>> just generate the cpio-archive every time make is called. Common initramfs
>> sizes are about a few megabytes and with today machines such a cpio-archive
>> is build in about a second,
>>
>
> I don't understand what kind of include would you want.
>
>> - get rid of gen_initramfs_list.sh and rewrite gen_init_cpio.c such that it
>> reads the filenames, modes and similiar itself (e.g. by using stat(2)).
>
> This is walkable but probably not worth the effort. Besides, why would
> anyone want to put spaces, colons and arbitrary characters to filenames
> in the initramfs?
>
>>
>>
>> But I don't have any motivation to do so. Sorry.
>>
>> Maybe Levente Kurusa is interested in doing so, he seems to have an interest
>> in the topic as he was quiet fast to send an answer to my patch.
>
> All I wonder now is why are you being so arrogant. You are not a 'remote
> keyboard' as you had called yourself, since when you send a patch
> you not only have to send it but communicate WHY maintainers
> should take that patch, and if somebody has a comment

The patch clearly explained what's broken.

> spotting an obvious mistake you shouldn't become angry.
> It was only a simple typo and a suggestion. Nobody told you
> to blindly accept my suggestion. Everybody can have opinions.
> It's open-source, the community suggests you new changes,
> it's normal, get used to it. Writing up this nonsense sentence
> (I feel the irony) probably took more time than actually resending
> the patch did.

So please accept my opinion that I'm not interested in fixing silly 
typos until I had at least some feedback with reasonable content. E.g. a 
Tested-By would have been nice.

And please forgive me that I'm unable to differentiate between feedback 
from "someone" or by a maintainer. Maybe I was fooled by that linux.com 
in your email, I had forgotten that this is a public available mail address.

So, Thanks for supporting the Linux Foundation.

Regards,

Alexander Holler

Re: [PATCH v2] initramfs: print error and shell out for unsupported content

[Alexander Holler]

Am 26.03.2014 22:55, schrieb Alexander Holler:
> Am 26.03.2014 22:38, schrieb Levente Kurusa:
>>> What I think might be reasonable is:
>>>
>>> - get rid of the dependency list in form of a include into the
>>> Makefile and
>>> just generate the cpio-archive every time make is called. Common
>>> initramfs
>>> sizes are about a few megabytes and with today machines such a
>>> cpio-archive
>>> is build in about a second,
>>>
>>
>> I don't understand what kind of include would you want.

Why do you try to discuss stuff you don't know about and haven't looked 
at? You could have taken the time to try out the description of my patch 
insgtead of just checking it for typos. Thats why I did spend the time 
to write that description.

The include is already there and is used. And that completly renders any 
further call of make broken, including make clean, distclean and 
mrproper. And is isn't obvious how to fix a once broken make. Even git 
clean -df doesn't help.

>>> - get rid of gen_initramfs_list.sh and rewrite gen_init_cpio.c such
>>> that it
>>> reads the filenames, modes and similiar itself (e.g. by using stat(2)).
>>
>> This is walkable but probably not worth the effort. Besides, why would
>> anyone want to put spaces, colons and arbitrary characters to filenames
>> in the initramfs?

I've already suggest an example for that. If you have a machine with 
bluetooth, look at /var/lib/bluetooth and you will discover directories 
with colons. So, guess what happens if you want to have (preset) 
link-keys in an initramfs to avoid an otherwise necessary pairing.

And spaces in filenames are used by a lot of people for various reasons. 
And you might wonder, but there exists software one might want to use in 
an initramfs which needs some file(s) with an hardcoded name wich 
contains spaces.


Alexander Holler

Re: [PATCH v2] initramfs: print error and shell out for unsupported content

[Alexander Holler]

Am 26.03.2014 23:37, schrieb Alexander Holler:
> Am 26.03.2014 22:55, schrieb Alexander Holler:
>> Am 26.03.2014 22:38, schrieb Levente Kurusa:

>>> This is walkable but probably not worth the effort. Besides, why would
>>> anyone want to put spaces, colons and arbitrary characters to filenames
>>> in the initramfs?
>
> I've already suggest an example for that. If you have a machine with
> bluetooth, look at /var/lib/bluetooth and you will discover directories
> with colons. So, guess what happens if you want to have (preset)
> link-keys in an initramfs to avoid an otherwise necessary pairing.
>
> And spaces in filenames are used by a lot of people for various reasons.
> And you might wonder, but there exists software one might want to use in
> an initramfs which needs some file(s) with an hardcoded name wich
> contains spaces.

Just that this problem exists at least since the dawn of git doesn't 
mean nobody has suffert through it.

E.g. I know the bug with colons since several years, but just feared to 
post a simple patch (for legitimate reasons as this thread shows).

Alexander Holler

Re: [PATCH 2/2 v3] initramfs: print error and shell out for unsupported content

[Michal Marek]

Dne 22.3.2014 19:22, Alexander Holler napsal(a):
> Am 22.03.2014 13:29, schrieb Alexander Holler:
>> The initramfs generation is broken for file and directory names which
>> contain
>> colons, spaces and other unusual characters. Print an error and don't
>> try to
>> continue.
> 
> (...)
>> +    # Files and directories with spaces and colons are unsupported.
>> +    local unsupported=$(find "${srcdir}" -regex '.*\(:\|
>> \|\n\|\r\|\t\).*')
> 
> I've just noted that -regex isn't POSIX. I don't know the kernel rules
> regarding this, and I don't care. But it might be a blocker for this patch.

The bigger problem is that there is no C-style quoting in regexps or
character classes matches any file with 'n', 'r' or 't' in its name. How
about

  -name '*[:[:space:]]*'

?

Michal

Re: [PATCH 2/2 v3] initramfs: print error and shell out for unsupported content

[Alexander Holler]

Am 31.03.2014 22:31, schrieb Michal Marek:
> Dne 22.3.2014 19:22, Alexander Holler napsal(a):
>> Am 22.03.2014 13:29, schrieb Alexander Holler:
>>> The initramfs generation is broken for file and directory names which
>>> contain
>>> colons, spaces and other unusual characters. Print an error and don't
>>> try to
>>> continue.
>>
>> (...)
>>> +    # Files and directories with spaces and colons are unsupported.
>>> +    local unsupported=$(find "${srcdir}" -regex '.*\(:\|
>>> \|\n\|\r\|\t\).*')
>>
>> I've just noted that -regex isn't POSIX. I don't know the kernel rules
>> regarding this, and I don't care. But it might be a blocker for this patch.
>
> The bigger problem is that there is no C-style quoting in regexps or
> character classes matches any file with 'n', 'r' or 't' in its name. How
> about
>
>    -name '*[:[:space:]]*'
>
> ?

Hmm, I wasn't aware that find supports such expressions, and I don't 
know how compatible that is. It would do the trick too.

But as I already said, trying to use arbitrary filenames in a Makefile 
doesn't really work. There are still other problems, e.g. filenames with 
a / (directory separator), *, or some of the special make variables like $<.

So the really working solution would be to get rid of that generated 
(and hidden) include. Modifying gen_init_cpio.c to browse and collect 
all filenames and types itself doesn't look like much work (an evening 
should be enough).

The drawback is that such a solution would build the initramfs every 
time make is called (while CONFIG_INITRAMFS_SOURCE points to a 
directory), but I think that time is negligible.
Another drawback is that it wouldn't be a simple patch anymore. Thus it 
wouldn't qualify for any stable tree (and nobody promises such a patch 
would be accepted at all, so it might be a waste of time at all).

Regards,

Alexander Holler

Re: [PATCH 2/2 v3] initramfs: print error and shell out for unsupported content

[Michal Marek]

On 2014-04-01 13:23, Alexander Holler wrote:
> But as I already said, trying to use arbitrary filenames in a Makefile 
> doesn't really work. There are still other problems, e.g. filenames with 
> a / (directory separator), *, or some of the special make variables like $<.
> 
> So the really working solution would be to get rid of that generated 
> (and hidden) include. Modifying gen_init_cpio.c to browse and collect 
> all filenames and types itself doesn't look like much work (an evening 
> should be enough).
> 
> The drawback is that such a solution would build the initramfs every 
> time make is called (while CONFIG_INITRAMFS_SOURCE points to a 
> directory), but I think that time is negligible.

Well, the cpio is embedded in the kernel image, so a rebuild of the cpio
means a relink of the kernel. One option would be to implement the
timestamp checking in gen_init_cpio.c, but I'm not sure if that's worth it.

Michal

Re: [PATCH 2/2 v3] initramfs: print error and shell out for unsupported content

[Alexander Holler]

Am 01.04.2014 14:23, schrieb Michal Marek:
> On 2014-04-01 13:23, Alexander Holler wrote:
>> But as I already said, trying to use arbitrary filenames in a Makefile
>> doesn't really work. There are still other problems, e.g. filenames with
>> a / (directory separator), *, or some of the special make variables like $<.
>>
>> So the really working solution would be to get rid of that generated
>> (and hidden) include. Modifying gen_init_cpio.c to browse and collect
>> all filenames and types itself doesn't look like much work (an evening
>> should be enough).
>>
>> The drawback is that such a solution would build the initramfs every
>> time make is called (while CONFIG_INITRAMFS_SOURCE points to a
>> directory), but I think that time is negligible.
>
> Well, the cpio is embedded in the kernel image, so a rebuild of the cpio
> means a relink of the kernel. One option would be to implement the
> timestamp checking in gen_init_cpio.c, but I'm not sure if that's worth it.

I can't answer that too, but if someone spends the time implement the 
directory traversing and collecting stats, it would just be one stat 
more (to get the timestamp of an maybe already built cpio archive).

But I'm fine with just avoiding the most common problems (in my case 
colons and spaces). And the little patch which makes sure that make 
clean, distclean or mrproper will work always, avoids the need to search 
how to fix the problem (deleting that hidden include) once a broken 
include was generated.

Regards,

Alexander Holler

[PATCH 1/2 v4] initramfs: don't include filenames from the initramfs for make goals (dist)clean

[Alexander Holler]

Various unusual filenames might break the make process when they are included
as dependencies (e.g. filenames with a colon).

This has the disadvantage that not even a make (dist)clean does work afterwards.
What makes the matter worse is that git clean -df doesn't remove such a possible
broken dependency list too.

Avoid that by not including this dependency list for make goals (dist)clean.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
Cc: <stable@vger.kernel.org>
---

 Changes to v3: none

 usr/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/usr/Makefile b/usr/Makefile
index e767f01..5d5b6aa 100644
--- a/usr/Makefile
+++ b/usr/Makefile
@@ -49,9 +49,11 @@ ramfs-args  := \
 # in initramfs and to detect if any files are added/removed.
 # Removed files are identified by directory timestamp being updated
 # The dependency list is generated by gen_initramfs.sh -l
+ifndef clean
 ifneq ($(wildcard $(obj)/.initramfs_data.cpio.d),)
 	include $(obj)/.initramfs_data.cpio.d
 endif
+endif
 
 quiet_cmd_initfs = GEN     $@
       cmd_initfs = $(initramfs) -o $@ $(ramfs-args) $(ramfs-input)
-- 
1.8.3.1

[PATCH 2/2 v4] initramfs: print error and shell out for unsupported content

[Alexander Holler]

The initramfs generation is broken for file and directory names which contain
colons, spaces and other unusual characters which can't be included in names
for dependencies in traditional Makefiles (which gen_initramfs_list.sh
generates). Print an error for the most common unsupported characters (colons
and whitespace) and don't try to continue if these are discovered.

Some tests:

cd linux
make defconfig
echo 'CONFIG_BLK_DEV_INITRD=y' >> .config
echo 'CONFIG_INITRAMFS_ROOT_UID=0' >>.config
echo 'CONFIG_INITRAMFS_ROOT_GID=0' >>.config
echo 'CONFIG_INITRAMFS_COMPRESSION_NONE=y' >>.config
echo 'CONFIG_INITRAMFS_SOURCE="/tmp/bugroot"' >>.config

Problem with colons:

mkdir -p /tmp/bugroot/a:b
make -j4 bzImage # no error
make bzImage # try again, oops

Problem with spaces:

mkdir -p /tmp/bugroot/a\ b
make -j4 bzImage # no error
zcat usr/initramfs_data.cpio.gz | cpio --extract --list # oops, no content

Cc: <stable@vger.kernel.org>
---

 Changes to v3:
   - Uses find with -name and an expression suggested by Michael Marek
     instead of find -regex. Therefor I've added his Signed-off-by.
   - Detail the unsupported characters which which will be discovered in
     the commit message.

 scripts/gen_initramfs_list.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/scripts/gen_initramfs_list.sh b/scripts/gen_initramfs_list.sh
index 17fa901..53398d5 100755
--- a/scripts/gen_initramfs_list.sh
+++ b/scripts/gen_initramfs_list.sh
@@ -171,6 +171,19 @@ dir_filelist() {
 	${dep_list}header "$1"
 
 	srcdir=$(echo "$1" | sed -e 's://*:/:g')
+
+	# Files and directories with spaces and colons are unsupported.
+	local unsupported=$(find "${srcdir}" -name '*[:[:space:]]*')
+	if [ ! -z "${unsupported}" ]; then
+		printf "ERROR: Unable to handle files/directories with " >&2
+		printf "unsupported characters (spaces, :, \\\n, \\\r, " >&2
+		printf "\\\t).\n Please use other ways to generate a " >&2
+		printf "cpio-archive with such names.\n" >&2
+		printf "Unsupported files and directories are:\n" >&2
+		printf "$unsupported\n" >&2
+		exit 1
+	fi
+
 	dirlist=$(find "${srcdir}" -printf "%p %m %U %G\n")
 
 	# If $dirlist is only one line, then the directory is empty
-- 
1.8.3.1