Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932724AbbDODcy (ORCPT ); Tue, 14 Apr 2015 23:32:54 -0400 Received: from ozlabs.org ([103.22.144.67]:49251 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932621AbbDODcs (ORCPT ); Tue, 14 Apr 2015 23:32:48 -0400 From: Rusty Russell To: Arthur Gautier Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH] param: fixup quote parsing of kernel arguments In-Reply-To: <20150408094401.GA10714@khany.gandi.net> References: <1428405635-8231-1-git-send-email-baloo@gandi.net> <87twwr17ow.fsf@rustcorp.com.au> <20150408094401.GA10714@khany.gandi.net> User-Agent: Notmuch/0.17 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-pc-linux-gnu) Date: Wed, 15 Apr 2015 10:24:24 +0930 Message-ID: <87pp76rz27.fsf@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3244 Lines: 118 Arthur Gautier writes: > On Wed, Apr 08, 2015 at 03:29:43PM +0930, Rusty Russell wrote: >> Arthur Gautier 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 >> >> 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/