When starting kernel with arguments like:
init=/bin/sh -c "echo arguments"
the trailing double quote is not removed which results in following command
being executed:
/bin/sh -c 'echo arguments"'
This commit removes the trailing double quote.
Signed-off-by: Arthur Gautier <[email protected]>
---
kernel/params.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/params.c b/kernel/params.c
index 728e05b..2118546 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -156,8 +156,11 @@ static char *next_arg(char *args, char **param, char **val)
if (args[i] == '=')
equals = i;
}
- if (args[i] == '"')
+ if (args[i] == '"') {
+ if (!equals)
+ args[i] = '\0';
in_quote = !in_quote;
+ }
}
*param = args;
--
2.1.4
Arthur Gautier <[email protected]> writes:
> When starting kernel with arguments like:
> init=/bin/sh -c "echo arguments"
> the trailing double quote is not removed which results in following command
> being executed:
> /bin/sh -c 'echo arguments"'
>
> This commit removes the trailing double quote.
>
> Signed-off-by: Arthur Gautier <[email protected]>
Hi Arthur,
Thanks, I'd not considered quotes outside '='. But this
fixes it in a weird way: we handle quotes below, we just don't do
anything for the "raw value" case:
for (i = 0; args[i]; i++) {
if (isspace(args[i]) && !in_quote)
break;
if (equals == 0) {
if (args[i] == '=')
equals = i;
}
if (args[i] == '"')
in_quote = !in_quote;
}
*param = args;
if (!equals)
*val = NULL;
else {
args[equals] = '\0';
*val = args + equals + 1;
/* Don't include quotes in value. */
if (**val == '"') {
(*val)++;
if (args[i-1] == '"')
args[i-1] = '\0';
}
if (quoted && args[i-1] == '"')
args[i-1] = '\0';
}
The logical fix is to just always remove the close quotes in both
cases:
diff --git a/kernel/params.c b/kernel/params.c
index 728e05b167de..a22d6a759b1a 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -173,9 +173,9 @@ static char *next_arg(char *args, char **param, char **val)
if (args[i-1] == '"')
args[i-1] = '\0';
}
- if (quoted && args[i-1] == '"')
- args[i-1] = '\0';
}
+ if (quoted && args[i-1] == '"')
+ args[i-1] = '\0';
if (args[i]) {
args[i] = '\0';
Does this work for you?
Thanks,
Rusty.
> ---
> kernel/params.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/params.c b/kernel/params.c
> index 728e05b..2118546 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -156,8 +156,11 @@ static char *next_arg(char *args, char **param, char **val)
> if (args[i] == '=')
> equals = i;
> }
> - if (args[i] == '"')
> + if (args[i] == '"') {
> + if (!equals)
> + args[i] = '\0';
> in_quote = !in_quote;
> + }
> }
>
> *param = args;
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Wed, Apr 08, 2015 at 03:29:43PM +0930, Rusty Russell wrote:
> Arthur Gautier <[email protected]> writes:
> > When starting kernel with arguments like:
> > init=/bin/sh -c "echo arguments"
> > the trailing double quote is not removed which results in following command
> > being executed:
> > /bin/sh -c 'echo arguments"'
> >
> > This commit removes the trailing double quote.
> >
> > Signed-off-by: Arthur Gautier <[email protected]>
>
> Hi Arthur,
>
> Thanks, I'd not considered quotes outside '='. But this
> fixes it in a weird way: we handle quotes below, we just don't do
> anything for the "raw value" case:
>
> for (i = 0; args[i]; i++) {
> if (isspace(args[i]) && !in_quote)
> break;
> if (equals == 0) {
> if (args[i] == '=')
> equals = i;
> }
> if (args[i] == '"')
> in_quote = !in_quote;
> }
>
> *param = args;
> if (!equals)
> *val = NULL;
> else {
> args[equals] = '\0';
> *val = args + equals + 1;
>
> /* Don't include quotes in value. */
> if (**val == '"') {
> (*val)++;
> if (args[i-1] == '"')
> args[i-1] = '\0';
> }
> if (quoted && args[i-1] == '"')
> args[i-1] = '\0';
> }
>
> The logical fix is to just always remove the close quotes in both
> cases:
>
> diff --git a/kernel/params.c b/kernel/params.c
> index 728e05b167de..a22d6a759b1a 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -173,9 +173,9 @@ static char *next_arg(char *args, char **param, char **val)
> if (args[i-1] == '"')
> args[i-1] = '\0';
> }
> - if (quoted && args[i-1] == '"')
> - args[i-1] = '\0';
> }
> + if (quoted && args[i-1] == '"')
> + args[i-1] = '\0';
>
> if (args[i]) {
> args[i] = '\0';
>
> Does this work for you?
>
Hi Rusty,
This does indeed fixes my issue and I agree with the fix but I've also
noticed a problem when parsing commands like:
char * input = "var0=\"val=ue\" \"var1\"=value \"var2=value\" \"echo foo\"";
char buf[255];
char *args = buf;
char * param, *val;
memcpy(buf, input, strlen(input)+1);
while(*args)
{
args = next_arg(args, ¶m, &val);
printf("%s=%s\n", param, val);
}
This parses commandline like:
var0=val=ue
var1"=value
var2=value
echo foo=(null)
As you may notice when using doublequote for keys, the final doublequote
is not removed. I'm not sure this should be considered as a problem nor
this should be expected but my patch was fixing this issue as well.
--
\o/ Arthur
G Gandi.net
Arthur Gautier <[email protected]> writes:
> On Wed, Apr 08, 2015 at 03:29:43PM +0930, Rusty Russell wrote:
>> Arthur Gautier <[email protected]> writes:
>> > When starting kernel with arguments like:
>> > init=/bin/sh -c "echo arguments"
>> > the trailing double quote is not removed which results in following command
>> > being executed:
>> > /bin/sh -c 'echo arguments"'
>> >
>> > This commit removes the trailing double quote.
>> >
>> > Signed-off-by: Arthur Gautier <[email protected]>
>>
>> Hi Arthur,
>>
>> Thanks, I'd not considered quotes outside '='. But this
>> fixes it in a weird way: we handle quotes below, we just don't do
>> anything for the "raw value" case:
>>
>> for (i = 0; args[i]; i++) {
>> if (isspace(args[i]) && !in_quote)
>> break;
>> if (equals == 0) {
>> if (args[i] == '=')
>> equals = i;
>> }
>> if (args[i] == '"')
>> in_quote = !in_quote;
>> }
>>
>> *param = args;
>> if (!equals)
>> *val = NULL;
>> else {
>> args[equals] = '\0';
>> *val = args + equals + 1;
>>
>> /* Don't include quotes in value. */
>> if (**val == '"') {
>> (*val)++;
>> if (args[i-1] == '"')
>> args[i-1] = '\0';
>> }
>> if (quoted && args[i-1] == '"')
>> args[i-1] = '\0';
>> }
>>
>> The logical fix is to just always remove the close quotes in both
>> cases:
>>
>> diff --git a/kernel/params.c b/kernel/params.c
>> index 728e05b167de..a22d6a759b1a 100644
>> --- a/kernel/params.c
>> +++ b/kernel/params.c
>> @@ -173,9 +173,9 @@ static char *next_arg(char *args, char **param, char **val)
>> if (args[i-1] == '"')
>> args[i-1] = '\0';
>> }
>> - if (quoted && args[i-1] == '"')
>> - args[i-1] = '\0';
>> }
>> + if (quoted && args[i-1] == '"')
>> + args[i-1] = '\0';
>>
>> if (args[i]) {
>> args[i] = '\0';
>>
>> Does this work for you?
>>
>
> Hi Rusty,
>
> This does indeed fixes my issue and I agree with the fix but I've also
> noticed a problem when parsing commands like:
>
> char * input = "var0=\"val=ue\" \"var1\"=value \"var2=value\" \"echo foo\"";
> char buf[255];
> char *args = buf;
> char * param, *val;
>
> memcpy(buf, input, strlen(input)+1);
>
> while(*args)
> {
> args = next_arg(args, ¶m, &val);
> printf("%s=%s\n", param, val);
> }
>
> This parses commandline like:
>
> var0=val=ue
> var1"=value
> var2=value
> echo foo=(null)
>
> As you may notice when using doublequote for keys, the final doublequote
> is not removed. I'm not sure this should be considered as a problem nor
> this should be expected but my patch was fixing this issue as well.
Indeed. And the following isn't parse correctly either:
foo="one ""two"
Though it *looks* like the code handles this, it doesn't: you'll get
'one ""two'.
Your first case was interesting because it was a more practical example.
I've applied my simple fix for now; if you want to do more thorough quote
handling (ie. use memmove), I'd love to see patches.
Thanks!
Rusty.