[arch-projects] [mkinitcpio] [RFC] Rewrite parse_cmdline (again)

Thomas Bächler

14 Apr 2011 14 Apr '11

10:43 p.m.

This should properly handle all ugly characters in values, omit settings any forbidden variable names and take care of double-quoted values with spaces in them. This should finally fix FS#23467, FS#22080 and FS#13900. --- I screwed up the last fix for FS#23467 when trying to fix the other two. This should now handle all cases just fine, although it duplicates some of the work the kernel already does. I tested it better than 0.6.10 and it seems okay. Can someone please proof-read this and tell me if it is good? init_functions | 58 +++++++++++++++++++++++++++++++++++-------------------- 1 files changed, 37 insertions(+), 21 deletions(-) diff --git a/init_functions b/init_functions index fd5bbb6..e29d696 100644 --- a/init_functions +++ b/init_functions @@ -31,27 +31,43 @@ launch_interactive_shell() { } parse_cmdline() { - eval set -- $(cat /proc/cmdline) - for cmd in "$@"; do - case "${cmd}" in - \#*) break ;; # ignore everything after a # in the commandline - # The kernel passes those to the kernel on its own - [0123456Ss]) ;; - [0-9]*) ;; - single) ;; - rw) readwrite="yes" ;; - ro) readwrite="no" ;; - # only export stuff that does work with ash :) - *=*) rhs="$(echo "${cmd}" | cut -d= -f2-)" - cmd="$(echo "${cmd}" | cut -d= -f1 | sed 's|\.|_|g')" - cmd="$(echo "${cmd}" | sed 's|-|_|g')=\"${rhs}\"" - (echo "${cmd}" | grep -qe '^[0-9]') || eval "${cmd}" - ;; - *) cmd="$(echo "${cmd}" | sed 's|\.|_|g')" - cmd="$(echo "${cmd}" | sed 's|-|_|g')" - (echo "${cmd}" | grep -qe '^[0-9]') || eval "${cmd}=y" - ;; - esac + local w in_quotes lhs rhs + in_quotes=0 + for w in $(cat /proc/cmdline); do + if [ ${in_quotes} -eq 0 ]; then + case "${w}" in + \#*) break ;; # ignore everything after a # in the commandline + # The kernel passes those to init on its own + [0123456Ss]) ;; + single) ;; + rw) readwrite="yes" ;; + ro) readwrite="no" ;; + # only export stuff that does work with ash :) + *=*) rhs="$(echo "${w}" | cut -d= -f2-)" + lhs="$(echo "${w}" | cut -d= -f1 | sed 's|\.|_|g;s|-|_|g;')" + if [ "${rhs:0:1}" = "\"" ]; then + if [ "${rhs:$((${#rhs}-1))}" = "\"" ]; then + rhs="${rhs:1:$((${#rhs}-2))}" + else + in_quotes=1 + continue + fi + fi + (echo "${lhs}" | grep -qe '^[0-9]' -e '[^a-zA-Z0-9_]') || eval ${lhs}=\${rhs} + ;; + *) lhs="$(echo "${w}" | sed 's|\.|_|g;s|-|_|g;')" + (echo "${lhs}" | grep -qe '^[0-9]' -e '[^a-zA-Z0-9_]') || eval ${lhs}=y + ;; + esac + else + if [ "${w:$((${#w}-1))}" = "\"" ]; then + rhs="${rhs} ${w:0:$((${#w}-1))}" + in_quotes=0 + (echo "${lhs}" | grep -qe '^[0-9]' -e '[^a-zA-Z0-9_]') || eval ${lhs}=\${rhs} + else + rhs="${rhs} ${w}" + fi + fi done } -- 1.7.4.4

Show replies by date

Dave Reisner

14 Apr 14 Apr

11:09 p.m.

On Thu, Apr 14, 2011 at 10:43:31PM +0200, Thomas Bächler wrote:

...

This should properly handle all ugly characters in values, omit settings any forbidden variable names and take care of double-quoted values with spaces in them.

This should finally fix FS#23467, FS#22080 and FS#13900. ---

I screwed up the last fix for FS#23467 when trying to fix the other two. This should now handle all cases just fine, although it duplicates some of the work the kernel already does.

I tested it better than 0.6.10 and it seems okay. Can someone please proof-read this and tell me if it is good?

Sandbox testing says this doesn't work. I feed in a file containing: root=/dev/sda video="din:10 foo bar baz" quiet ro and after parsing, i echo "root=$root" and "video=$video". i get... video="din:10 foo bar baz root=/dev/sda I posted what I thought was a valid solution on FS#23467 from stack overflow that seems to be a lot more sane and _much_ more maintainable. For those who weren't following the report, I suggested that we use the same fallback as /etc/fstab, which says to encode spaces with octal sequences. These can then be decoded using printf's %b flag. It requires only a simple change to the current parse_cmdline function. I also never got a response as to where you're seeing pollution from using export over eval. Sorry, but eval sucks hard and I've put a lot of effort into _removing_ it from wherever I can in Arch code. It's _rarely_ used properly. On top of it all, I've never seen any variables declared on the command line populated into userspace. Am I insane here? d

...

init_functions | 58 +++++++++++++++++++++++++++++++++++-------------------- 1 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/init_functions b/init_functions index fd5bbb6..e29d696 100644 --- a/init_functions +++ b/init_functions @@ -31,27 +31,43 @@ launch_interactive_shell() { }

parse_cmdline() { - eval set -- $(cat /proc/cmdline) - for cmd in "$@"; do - case "${cmd}" in - \#*) break ;; # ignore everything after a # in the commandline - # The kernel passes those to the kernel on its own - [0123456Ss]) ;; - [0-9]*) ;; - single) ;; - rw) readwrite="yes" ;; - ro) readwrite="no" ;; - # only export stuff that does work with ash :) - *=*) rhs="$(echo "${cmd}" | cut -d= -f2-)" - cmd="$(echo "${cmd}" | cut -d= -f1 | sed 's|\.|_|g')" - cmd="$(echo "${cmd}" | sed 's|-|_|g')=\"${rhs}\"" - (echo "${cmd}" | grep -qe '^[0-9]') || eval "${cmd}" - ;; - *) cmd="$(echo "${cmd}" | sed 's|\.|_|g')" - cmd="$(echo "${cmd}" | sed 's|-|_|g')" - (echo "${cmd}" | grep -qe '^[0-9]') || eval "${cmd}=y" - ;; - esac + local w in_quotes lhs rhs + in_quotes=0 + for w in $(cat /proc/cmdline); do + if [ ${in_quotes} -eq 0 ]; then + case "${w}" in + \#*) break ;; # ignore everything after a # in the commandline + # The kernel passes those to init on its own + [0123456Ss]) ;; + single) ;; + rw) readwrite="yes" ;; + ro) readwrite="no" ;; + # only export stuff that does work with ash :) + *=*) rhs="$(echo "${w}" | cut -d= -f2-)" + lhs="$(echo "${w}" | cut -d= -f1 | sed 's|\.|_|g;s|-|_|g;')" + if [ "${rhs:0:1}" = "\"" ]; then + if [ "${rhs:$((${#rhs}-1))}" = "\"" ]; then + rhs="${rhs:1:$((${#rhs}-2))}" + else + in_quotes=1 + continue + fi + fi + (echo "${lhs}" | grep -qe '^[0-9]' -e '[^a-zA-Z0-9_]') || eval ${lhs}=\${rhs} + ;; + *) lhs="$(echo "${w}" | sed 's|\.|_|g;s|-|_|g;')" + (echo "${lhs}" | grep -qe '^[0-9]' -e '[^a-zA-Z0-9_]') || eval ${lhs}=y + ;; + esac + else + if [ "${w:$((${#w}-1))}" = "\"" ]; then + rhs="${rhs} ${w:0:$((${#w}-1))}" + in_quotes=0 + (echo "${lhs}" | grep -qe '^[0-9]' -e '[^a-zA-Z0-9_]') || eval ${lhs}=\${rhs} + else + rhs="${rhs} ${w}" + fi + fi done }

-- 1.7.4.4

Thomas Bächler

11:28 p.m.

Am 14.04.2011 23:09, schrieb Dave Reisner:

...

...
I tested it better than 0.6.10 and it seems okay. Can someone please proof-read this and tell me if it is good?

Sandbox testing says this doesn't work. I feed in a file containing:

root=/dev/sda video="din:10 foo bar baz" quiet ro

and after parsing, i echo "root=$root" and "video=$video". i get...

video="din:10 foo bar baz root=/dev/sda

Oh yeah, I forgot one line, fixed it (I'll send a v2 in a second).

...

I posted what I thought was a valid solution on FS#23467 from stack overflow that seems to be a lot more sane and _much_ more maintainable. For those who weren't following the report, I suggested that we use the same fallback as /etc/fstab, which says to encode spaces with octal sequences. These can then be decoded using printf's %b flag. It requires only a simple change to the current parse_cmdline function.

I really don't understand what you mean (or how it would help). What you mentioned doesn't solve the initial problem of finding out which spaces separate arguments and which spaces are inside quoted strings.

...

I also never got a response as to where you're seeing pollution from using export over eval.

'export' exports variables to the environment and they are passed on when forking. These variables are for internal purposes in mkinitcpio and don't belong into other applications. As a simple example, I had 'udevd_running=1' set in my environment during initscripts, which certainly shouldn't be there. (This wasn't on the command line, but in another place in mkinitcpio where things where exported when they shouldn't be). The right way would be something like bash's 'declare', but ash doesn't have that.

...

Sorry, but eval sucks hard and I've put a lot of effort into _removing_ it from wherever I can in Arch code. It's _rarely_ used properly.

If you check carefully, I use eval right here. There is nothing in the eval'ed strings that should cause trouble.

Thomas Bächler

11:28 p.m.

New subject: [arch-projects] [mkinitcpio] [RFCv2] Rewrite parse_cmdline (again)

This should properly handle all ugly characters in values, omit settings any forbidden variable names and take care of double-quoted values with spaces in them. This should finally fix FS#23467, FS#22080 and FS#13900. --- init_functions | 59 ++++++++++++++++++++++++++++++++++++------------------- 1 files changed, 38 insertions(+), 21 deletions(-) diff --git a/init_functions b/init_functions index fd5bbb6..be3599e 100644 --- a/init_functions +++ b/init_functions @@ -31,27 +31,44 @@ launch_interactive_shell() { } parse_cmdline() { - eval set -- $(cat /proc/cmdline) - for cmd in "$@"; do - case "${cmd}" in - \#*) break ;; # ignore everything after a # in the commandline - # The kernel passes those to the kernel on its own - [0123456Ss]) ;; - [0-9]*) ;; - single) ;; - rw) readwrite="yes" ;; - ro) readwrite="no" ;; - # only export stuff that does work with ash :) - *=*) rhs="$(echo "${cmd}" | cut -d= -f2-)" - cmd="$(echo "${cmd}" | cut -d= -f1 | sed 's|\.|_|g')" - cmd="$(echo "${cmd}" | sed 's|-|_|g')=\"${rhs}\"" - (echo "${cmd}" | grep -qe '^[0-9]') || eval "${cmd}" - ;; - *) cmd="$(echo "${cmd}" | sed 's|\.|_|g')" - cmd="$(echo "${cmd}" | sed 's|-|_|g')" - (echo "${cmd}" | grep -qe '^[0-9]') || eval "${cmd}=y" - ;; - esac + local w in_quotes lhs rhs + in_quotes=0 + for w in $(cat /proc/cmdline); do + if [ ${in_quotes} -eq 0 ]; then + case "${w}" in + \#*) break ;; # ignore everything after a # in the commandline + # The kernel passes those to init on its own + [0123456Ss]) ;; + single) ;; + rw) readwrite="yes" ;; + ro) readwrite="no" ;; + # only export stuff that does work with ash :) + *=*) rhs="$(echo "${w}" | cut -d= -f2-)" + lhs="$(echo "${w}" | cut -d= -f1 | sed 's|\.|_|g;s|-|_|g;')" + if [ "${rhs:0:1}" = "\"" ]; then + if [ "${rhs:$((${#rhs}-1))}" = "\"" ]; then + rhs="${rhs:1:$((${#rhs}-2))}" + else + rhs="${rhs:1}" + in_quotes=1 + continue + fi + fi + (echo "${lhs}" | grep -qe '^[0-9]' -e '[^a-zA-Z0-9_]') || eval ${lhs}=\${rhs} + ;; + *) lhs="$(echo "${w}" | sed 's|\.|_|g;s|-|_|g;')" + (echo "${lhs}" | grep -qe '^[0-9]' -e '[^a-zA-Z0-9_]') || eval ${lhs}=y + ;; + esac + else + if [ "${w:$((${#w}-1))}" = "\"" ]; then + rhs="${rhs} ${w:0:$((${#w}-1))}" + in_quotes=0 + (echo "${lhs}" | grep -qe '^[0-9]' -e '[^a-zA-Z0-9_]') || eval ${lhs}=\${rhs} + else + rhs="${rhs} ${w}" + fi + fi done } -- 1.7.4.4

dave reisner

11:44 p.m.

On Apr 14, 2011 5:28 PM, "Thomas Bächler" wrote:

...

Am 14.04.2011 23:09, schrieb Dave Reisner:

...
...
I tested it better than 0.6.10 and it seems okay. Can someone please proof-read this and tell me if it is good?

Sandbox testing says this doesn't work. I feed in a file containing:

root=/dev/sda video="din:10 foo bar baz" quiet ro

and after parsing, i echo "root=$root" and "video=$video". i get...

video="din:10 foo bar baz root=/dev/sda

Oh yeah, I forgot one line, fixed it (I'll send a v2 in a second).

...
I posted what I thought was a valid solution on FS#23467 from stack overflow that seems to be a lot more sane and _much_ more maintainable. For those who weren't following the report, I suggested that we use the same fallback as /etc/fstab, which says to encode spaces with octal sequences. These can then be decoded using printf's %b flag. It requires only a simple change to the current parse_cmdline function.

I really don't understand what you mean (or how it would help). What you mentioned doesn't solve the initial problem of finding out which spaces separate arguments and which spaces are inside quoted strings.

It absolutely does because it eliminates spaces within variables, e.g. video=foo\040bar root=… The whole point is that quotes aren't used. fstab sets precedent here so its not some wild and whacky new thing being introduced.

...

...
I also never got a response as to where you're seeing pollution from using export over eval.

'export' exports variables to the environment and they are passed on when forking. These variables are for internal purposes in mkinitcpio and don't belong into other applications. As a simple example, I had 'udevd_running=1' set in my environment during initscripts, which certainly shouldn't be there. (This wasn't on the command line, but in another place in mkinitcpio where things where exported when they shouldn't be).

The right way would be something like bash's 'declare', but ash doesn't have that.

Right. I understand what export does. It also wouldn't be hard to cleanse the environment since we know exactly what we're parsing. Having those cars during sysinit? Not the worst thing in the world. Exported to a user's shell? That's a real issue.

...

...
Sorry, but eval sucks hard and I've put a lot of effort into _removing_ it from wherever I can in Arch code. It's _rarely_ used properly.

If you check carefully, I use eval right here. There is nothing in the eval'ed strings that should cause trouble.

We'll have to agree to disagree here. The problem is that you can't know what eval is executing in this case because its unbounded and unchecked input. d

Rémy Oudompheng

15 Apr 15 Apr

12:13 a.m.

On Thu 14 April 2011 at 17:44 -0400, dave reisner wrote:

...

On Apr 14, 2011 5:28 PM, "Thomas Bächler" wrote:

...
...
I posted what I thought was a valid solution on FS#23467 from stack overflow that seems to be a lot more sane and _much_ more maintainable. For those who weren't following the report, I suggested that we use the same fallback as /etc/fstab, which says to encode spaces with octal sequences. These can then be decoded using printf's %b flag. It requires only a simple change to the current parse_cmdline function.

I really don't understand what you mean (or how it would help). What you mentioned doesn't solve the initial problem of finding out which spaces separate arguments and which spaces are inside quoted strings.

It absolutely does because it eliminates spaces within variables, e.g. video=foo\040bar root=…

The whole point is that quotes aren't used. fstab sets precedent here so its not some wild and whacky new thing being introduced.

This looks very nice, but how do we get a space-escaped kernel command line? Is the user expected to encode his command line at boot time? And do we really have to filter out invalid variable names? Isn't there some documentation that says the kernel isn't supposed to have absurd command line parameters (like unescaped >, <, &, |) Because I think that (minus eval statements), the function would look better like this : local w lhs rhs eval set -- $(cat test) for w; do case "${w}" in \#*) break ;; # ignore everything after a # in the commandline # The kernel passes those to init on its own [0123456Ss]) ;; single) ;; rw) readwrite="yes" ;; ro) readwrite="no" ;; # variables don't begin with a digit [0-9]*) ;; # only export stuff that does work with ash :) *=*) lhs=${w%%=*} rhs=${w#*=} # replace forbidden characters lhs=${lhs/./_} lhs=${lhs/-/_} eval "${lhs}='${rhs}'" ;; *) lhs=${w/./_} lhs=${w/-/_} eval "${lhs}=y" ;; esac done -- Rémy.

Thomas Bächler

12:38 a.m.

Am 15.04.2011 00:13, schrieb Rémy Oudompheng:

...

And do we really have to filter out invalid variable names? Isn't there some documentation that says the kernel isn't supposed to have absurd command line parameters (like unescaped >, <, &, |)

The kernel allows all kind of crappy stuff. It even exports almost everything with a = in it to the environment (but not everything, it strips root= for example). If you put something like (/%$=abc on the command line, it will export a variable (%$ into the environment. The situation is pretty annoying, as I don't know for sure which variables it exports and which it doesn't, so we still try to parse everything.

Dave Reisner

2:24 a.m.

On Fri, Apr 15, 2011 at 12:13:29AM +0200, Rémy Oudompheng wrote:

...

On Thu 14 April 2011 at 17:44 -0400, dave reisner wrote:

...
On Apr 14, 2011 5:28 PM, "Thomas Bächler" wrote:

...
...
I posted what I thought was a valid solution on FS#23467 from stack overflow that seems to be a lot more sane and _much_ more maintainable. For those who weren't following the report, I suggested that we use the same fallback as /etc/fstab, which says to encode spaces with octal sequences. These can then be decoded using printf's %b flag. It requires only a simple change to the current parse_cmdline function.

I really don't understand what you mean (or how it would help). What you mentioned doesn't solve the initial problem of finding out which spaces separate arguments and which spaces are inside quoted strings.

It absolutely does because it eliminates spaces within variables, e.g. video=foo\040bar root=…

The whole point is that quotes aren't used. fstab sets precedent here so its not some wild and whacky new thing being introduced.

This looks very nice, but how do we get a space-escaped kernel command line? Is the user expected to encode his command line at boot time?

And do we really have to filter out invalid variable names? Isn't there some documentation that says the kernel isn't supposed to have absurd command line parameters (like unescaped >, <, &, |)

Because I think that (minus eval statements), the function would look better like this :

local w lhs rhs eval set -- $(cat test) for w; do case "${w}" in \#*) break ;; # ignore everything after a # in the commandline # The kernel passes those to init on its own [0123456Ss]) ;; single) ;; rw) readwrite="yes" ;; ro) readwrite="no" ;; # variables don't begin with a digit [0-9]*) ;; # only export stuff that does work with ash :) *=*) lhs=${w%%=*} rhs=${w#*=} # replace forbidden characters lhs=${lhs/./_} lhs=${lhs/-/_} eval "${lhs}='${rhs}'" ;; *) lhs=${w/./_} lhs=${w/-/_} eval "${lhs}=y" ;; esac done

-- Rémy.

Yes, the idea is that the user is expected to escape their own whitespace, just as they are expected to do so in /etc/fstab. I tested the following across the open bug reports and can report great success on all fronts: parse_cmdline() { for cmd in $(cat /proc/cmdline); do case $cmd in \#*) break ;; # ignore everything after a # in the commandline # The kernel passes those to the kernel on its own [0123456Ss]) ;; [0-9]*) ;; single) ;; rw) readwrite="yes" ;; ro) readwrite="no" ;; # only export stuff that does work with ash :) *=*) lhs=${cmd%%=*} lhs=${lhs//[-.]/_} rhs=$(printf '%b' "${cmd#*=}") export "$lhs"="$rhs" ;; *) cmd=${cmd//[-.]/_} export "$cmd"="y" ;; esac done } Yes, I've also removed the grep checks prior to each export. They're redundant because of the 3rd case down, [0-9]*, already checking for a variable starting with a leading digit. Thomas mentioned in IRC that there was a "huge problem" with this, but had to go to sleep. I'm interested in what he has to say. dave

Thomas Bächler

10:06 a.m.

Am 15.04.2011 02:24, schrieb Dave Reisner:

...

Yes, the idea is that the user is expected to escape their own whitespace, just as they are expected to do so in /etc/fstab.

You don't understand the problem at all: We cannot simply change the format of the kernel command line as we wish. I'll explain it to you as clearly as possible: The kernel interprets A=B pairs on the command line, where B can be double-quoted - as far as I am aware, the kernel does not interpret any kind of escapes here, solely double quotes. This is what the kernel does with them (as far as I figured it out so far): (1) If the option A is recognized by any part of the kernel, B is passed as the value of A to that part of the kernel. If we were to escape characters inside it, it wouldn't understand the value anymore. (2) If the option A is not recognized by the kernel, it is exported into the initial environment (which is passed on to PID 1). (3) If an option is not of the form A=B, but does not contain a =, the option is parsed as an argument to init. Sadly, some of the variables we need in early userspace are not exported - this at least applies to root=, but there might be more (root= is traditionally used by the kernel, so (1) applies to it). Therefore, we need to parse the whole command line again. Most of what we parse there is actually redundant - like the acpi_osi= or video= options, which are never used in early userspace. However, even if we omit setting those variables, we still need to parse them to know where they end, in order to get all options from the command line. What we MUST do is deal with the format the kernel command line has. I didn't even understand your comments until now, because I would never have thought your suggestion was that ridiculous.

Thomas Bächler

10:35 a.m.

Am 15.04.2011 10:06, schrieb Thomas Bächler:

...

(3) If an option is not of the form A=B, but does not contain a =, the option is parsed as an argument to init.

Heh, writing is hard. This is what I wanted to write: (3) If an option is not of the form A=B, i.e. does not contain a =, the option is passed as an argument to init.

Dave Reisner

12:22 p.m.

On Fri, Apr 15, 2011 at 10:06:55AM +0200, Thomas Bächler wrote:

...

Am 15.04.2011 02:24, schrieb Dave Reisner:

...
Yes, the idea is that the user is expected to escape their own whitespace, just as they are expected to do so in /etc/fstab.

You don't understand the problem at all: We cannot simply change the format of the kernel command line as we wish.

I'll explain it to you as clearly as possible:

The kernel interprets A=B pairs on the command line, where B can be double-quoted - as far as I am aware, the kernel does not interpret any kind of escapes here, solely double quotes.

This is what the kernel does with them (as far as I figured it out so far): (1) If the option A is recognized by any part of the kernel, B is passed as the value of A to that part of the kernel. If we were to escape characters inside it, it wouldn't understand the value anymore. (2) If the option A is not recognized by the kernel, it is exported into the initial environment (which is passed on to PID 1). (3) If an option is not of the form A=B, but does not contain a =, the option is parsed as an argument to init.

Sadly, some of the variables we need in early userspace are not exported - this at least applies to root=, but there might be more (root= is traditionally used by the kernel, so (1) applies to it). Therefore, we need to parse the whole command line again.

Most of what we parse there is actually redundant - like the acpi_osi= or video= options, which are never used in early userspace. However, even if we omit setting those variables, we still need to parse them to know where they end, in order to get all options from the command line.

What we MUST do is deal with the format the kernel command line has. I didn't even understand your comments until now, because I would never have thought your suggestion was that ridiculous.

Cool, good to see a breakdown of how the cmdline parameters are passed. I didn't come across any of this in my googling adventures. I do notice, however, that only simple words (not variable decls) at the _end_ of the cmdline are passed on as positional parameters to init. Anything else seems to be ignored. d

Thomas Bächler

1:16 p.m.

Am 15.04.2011 12:22, schrieb Dave Reisner:

...

Cool, good to see a breakdown of how the cmdline parameters are passed. I didn't come across any of this in my googling adventures.

Part of it is from the kernel documentation. Part of it is guesses based on what I've seen.

...

I do notice, however, that only simple words (not variable decls) at the _end_ of the cmdline are passed on as positional parameters to init. Anything else seems to be ignored.

I think everything after init= or rdinit= is passed. If neither init or rdinit is specified, you might be right with your assumption. I didn't investigate this too far, as we simply pass on those arguments with "$@" to init. (Btw, init= is another variable that (like root=) isn't passed by the kernel.)

Thomas Bächler

12:21 a.m.

Am 14.04.2011 23:44, schrieb dave reisner:

...

...
...
I posted what I thought was a valid solution on FS#23467 from stack overflow that seems to be a lot more sane and _much_ more maintainable. For those who weren't following the report, I suggested that we use the same fallback as /etc/fstab, which says to encode spaces with octal sequences. These can then be decoded using printf's %b flag. It requires only a simple change to the current parse_cmdline function.

I really don't understand what you mean (or how it would help). What you mentioned doesn't solve the initial problem of finding out which spaces separate arguments and which spaces are inside quoted strings.

It absolutely does because it eliminates spaces within variables, e.g. video=foo\040bar root=…

The whole point is that quotes aren't used. fstab sets precedent here so its not some wild and whacky new thing being introduced.

I'll have to recheck this.

...

...
If you check carefully, I use eval right here. There is nothing in the eval'ed strings that should cause trouble.

We'll have to agree to disagree here. The problem is that you can't know what eval is executing in this case because its unbounded and unchecked input.

The only thing I do is: eval ${lhs}=\${rhs} ${lhs} is only characters, numbers and underscores, and \${rhs} only evaluates to ${rhs}. I don't see a problem here.

4769

Age (days ago)

4770

Last active (days ago)

List overview

Download

12 comments

4 participants

participants (4)

Dave Reisner
dave reisner
Rémy Oudompheng
Thomas Bächler

[arch-projects] [mkinitcpio] [RFC] Rewrite parse_cmdline (again)

tags

participants (4)