[PATCH 1/3] [RESEND] parse_shell_args.c: shell-like argument list parser

[PATCH 1/3] [RESEND] parse_shell_args.c: shell-like argument list parser

[Kirill A. Shutemov]

It's implementation of argument list parser using shell rules.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 include/linux/kernel.h |    2 +
 lib/Makefile           |    2 +-
 lib/parse_shell_args.c |  238 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 241 insertions(+), 1 deletions(-)
 create mode 100644 lib/parse_shell_args.c

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 7fa3718..b68d1ba 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -208,6 +208,8 @@ extern int func_ptr_is_kernel_text(void *ptr);
 struct pid;
 extern struct pid *session_of_pgrp(struct pid *pgrp);
 
+extern int parse_shell_args(const char *in, char *out, size_t out_size);
+
 /*
  * FW_BUG
  * Add this to a message where you are sure the firmware is buggy or behaves
diff --git a/lib/Makefile b/lib/Makefile
index 32b0e64..3aaf2f3 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -20,7 +20,7 @@ lib-y	+= kobject.o kref.o klist.o
 
 obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
 	 bust_spinlocks.o hexdump.o kasprintf.o bitmap.o scatterlist.o \
-	 string_helpers.o
+	 string_helpers.o parse_shell_args.o
 
 ifeq ($(CONFIG_DEBUG_KOBJECT),y)
 CFLAGS_kobject.o += -DDEBUG
diff --git a/lib/parse_shell_args.c b/lib/parse_shell_args.c
new file mode 100644
index 0000000..2b32cab
--- /dev/null
+++ b/lib/parse_shell_args.c
@@ -0,0 +1,238 @@
+/*
+ * lib/parse_shell_args.c
+ *
+ * Copiright (C) 2009 Kirill A. Shutemov
+ *
+ * Shell-like argument list parser
+ */
+
+#ifdef __KERNEL__
+#include <linux/module.h>
+#include <linux/types.h>
+#else
+#include <sys/types.h>
+#endif
+
+#define S_START		0
+#define S_DONE		1
+#define S_NORMAL	2
+#define S_QUOTE		3
+#define S_DQUOTE	4
+
+/**
+ * parse_shell_args - parse shell-like argument list
+ *
+ * @in:		null-terminated input string
+ * @out:	output buffer
+ * @out_size:	output buffer size in byte
+ *
+ * Returns number of arguments or 0 on error.
+ * Parsed arguments store in @out separated by '\0'
+ */
+int parse_shell_args(const char *in, char *out, size_t out_size)
+{
+	int state = S_START;
+	const char *in_p = in;
+	char *out_p = out;
+	int argc = 0;
+	int escape = 0;
+
+	while (state != S_DONE) {
+		if (out_p - out >= out_size)
+			return 0;
+
+		if (escape) {
+			*out_p++ = *in_p++;
+			escape = 0;
+			continue;
+		}
+
+		switch (state) {
+			case S_START:
+				switch (*in_p) {
+					case '\n':
+					case '\0':
+						state = S_DONE;
+						*out_p = '\0';
+						break;
+					case ' ':
+					case '\t':
+						in_p++;
+						break;
+					case '\'':
+						state = S_QUOTE;
+						in_p++;
+						argc++;
+						break;
+					case '"':
+						state = S_DQUOTE;
+						in_p++;
+						argc++;
+						break;
+					default:
+						state = S_NORMAL;
+						argc++;
+						break;
+				}
+				break;
+
+			case S_NORMAL:
+				switch (*in_p) {
+					case '\n':
+					case '\0':
+						state = S_DONE;
+						*out_p = '\0';
+						break;
+					case ' ':
+					case '\t':
+						state = S_START;
+						*out_p++ = '\0';
+						break;
+					case '\'':
+						state = S_QUOTE;
+						in_p++;
+						break;
+					case '\"':
+						state = S_DQUOTE;
+						in_p++;
+						break;
+					case '\\':
+						in_p++;
+						escape = 1;
+						break;
+					default:
+						*out_p++ = *in_p++;
+						break;
+				}
+				break;
+
+			case S_QUOTE:
+				switch (*in_p) {
+					case '\'':
+						state = S_NORMAL;
+						in_p++;
+						break;
+					case '\n':
+					case '\0':
+						return 0;
+						break;
+					default:
+						*out_p++ = *in_p++;
+						break;
+				}
+				break;
+			case S_DQUOTE:
+				switch (*in_p) {
+					case '\"':
+						state = S_NORMAL;
+						in_p++;
+						break;
+					case '\n':
+					case '\0':
+						return 0;
+						break;
+					case '\\':
+						if (*(in_p+1) == '$' ||
+								*(in_p+1) == '`' ||
+								*(in_p+1) == '"' ||
+								*(in_p+1) == '\\' ||
+								*(in_p+1) == '\n') {
+							in_p++;
+							escape = 1;
+						} else
+							*out_p++ = *in_p++;
+						break;
+					default:
+						*out_p++ = *in_p++;
+						break;
+				}
+				break;
+			default:
+				return 0;
+				break;
+		}
+	}
+
+	return argc;
+}
+
+#ifdef __KERNEL__
+EXPORT_SYMBOL(parse_shell_args);
+#else
+/* Unit tests */
+
+#include <limits.h>
+#include <stdio.h>
+#include <string.h>
+
+static void test_parse(const char *in, const char *expected_out, int expected_argc)
+{
+	char out[PATH_MAX];
+	char *out_p = out;
+	int i;
+	int argc;
+
+	argc = parse_shell_args(in, out, PATH_MAX);
+
+	if (argc != expected_argc) {
+		printf("%s: wrong argc: %d, but expected %d\n", in, argc, expected_argc);
+		return;
+	}
+
+	for(i=0; i < argc; i++) {
+		int len;
+
+		if (strcmp(expected_out, out_p)) {
+			printf("%s: %s, but expected %s\n", in , out_p, expected_out);
+			return;
+		}
+
+		len = strlen(expected_out);
+		expected_out += len + 1;
+		out_p += len + 1;
+	}
+
+	printf("%s:\tOK\n", in);
+}
+
+int main(void)
+{
+	test_parse("", "", 0);
+
+	test_parse("/bin/ls", "/bin/ls", 1);
+	test_parse("/bin\\ ls", "/bin ls", 1);
+	test_parse("/bin/ls\n", "/bin/ls", 1);
+	test_parse("/bin/ls\\\n", "/bin/ls\n", 1);
+	test_parse(" /bin/ls", "/bin/ls", 1);
+	test_parse("\t/bin/ls", "/bin/ls", 1);
+	test_parse("/bin/ls ", "/bin/ls", 1);
+	test_parse("/bin/ls\t", "/bin/ls", 1);
+	test_parse("\\'/bin/ls", "\'/bin/ls", 1);
+	test_parse("/bin\\'/ls", "/bin\'/ls", 1);
+	test_parse("/bin'/ls", "", 0);
+	test_parse("/bin\"/ls", "", 0);
+
+	test_parse("'/bin/ls'", "/bin/ls", 1);
+	test_parse("'/bin ls'", "/bin ls", 1);
+	test_parse("'/bin\tls'", "/bin\tls", 1);
+	test_parse("'/bin''/ls'", "/bin/ls", 1);
+	test_parse("/bin'/ls'", "/bin/ls", 1);
+	test_parse("'/bin'/ls", "/bin/ls", 1);
+	test_parse("'/bin \t\"%$#@/ls'", "/bin \t\"%$#@/ls", 1);
+
+	test_parse("\"/bin/ls\"", "/bin/ls", 1);
+	test_parse("\"/bin\"\"/ls\"", "/bin/ls", 1);
+	test_parse("/bin\"/ls\"", "/bin/ls", 1);
+	test_parse("\"/bin\"/ls", "/bin/ls", 1);
+	test_parse("\"\\$\\`\\\"\\\\\\\n\\\\!\\@\\#\\%\\a\"", "$`\"\\\n\\!\\@\\#\\%\\a", 1);
+	test_parse("\"/bin/ls\\\"", "", 0);
+
+	test_parse("/bin/ls -a", "/bin/ls\0-a", 2);
+	test_parse("/bin/ls\t-a", "/bin/ls\0-a", 2);
+	test_parse("/bin/ls  \t \t-a", "/bin/ls\0-a", 2);
+	test_parse("/bin/ls -a -l", "/bin/ls\0-a\0-l", 3);
+	test_parse("/bin/ls '-a -l'", "/bin/ls\0-a -l", 2);
+
+	return 0;
+}
+#endif
-- 
1.6.1.3.GIT

[PATCH 2/3] [RESEND] binfmt_script: rewrite using parse_shell_args()

[Kirill A. Shutemov]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 3921 bytes --]

parse_shell_args() allows to use more than one argument in shebang.

This patch allows to write something like

 #!/usr/bin/env python -c

and it will be parsed according shell rules.

Currently, it interpretes as /usr/bin/env "python -c" and we gets
ENOENT.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 fs/binfmt_script.c |   88 +++++++++++++++++++---------------------------------
 1 files changed, 32 insertions(+), 56 deletions(-)

diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 0834350..ab6a6b2 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -1,93 +1,69 @@
 /*
  *  linux/fs/binfmt_script.c
  *
+ *  Copyright (C) 2009  Kirill A. Shutemov
  *  Copyright (C) 1996  Martin von Löwis
  *  original #!-checking implemented by tytso.
  */
 
 #include <linux/module.h>
-#include <linux/string.h>
-#include <linux/stat.h>
-#include <linux/slab.h>
 #include <linux/binfmts.h>
-#include <linux/init.h>
 #include <linux/file.h>
-#include <linux/err.h>
 #include <linux/fs.h>
 
 static int load_script(struct linux_binprm *bprm,struct pt_regs *regs)
 {
-	char *cp, *i_name, *i_arg;
 	struct file *file;
-	char interp[BINPRM_BUF_SIZE];
-	int retval;
+	char out[BINPRM_BUF_SIZE];
+	int argc, retval;
 
 	if ((bprm->buf[0] != '#') || (bprm->buf[1] != '!') ||
 	    (bprm->recursion_depth > BINPRM_MAX_RECURSION))
 		return -ENOEXEC;
-	/*
-	 * This section does the #! interpretation.
-	 * Sorta complicated, but hopefully it will work.  -TYT
-	 */
 
 	bprm->recursion_depth++;
 	allow_write_access(bprm->file);
 	fput(bprm->file);
 	bprm->file = NULL;
 
-	bprm->buf[BINPRM_BUF_SIZE - 1] = '\0';
-	if ((cp = strchr(bprm->buf, '\n')) == NULL)
-		cp = bprm->buf+BINPRM_BUF_SIZE-1;
-	*cp = '\0';
-	while (cp > bprm->buf) {
-		cp--;
-		if ((*cp == ' ') || (*cp == '\t'))
-			*cp = '\0';
-		else
-			break;
-	}
-	for (cp = bprm->buf+2; (*cp == ' ') || (*cp == '\t'); cp++);
-	if (*cp == '\0') 
-		return -ENOEXEC; /* No interpreter name found */
-	i_name = cp;
-	i_arg = NULL;
-	for ( ; *cp && (*cp != ' ') && (*cp != '\t'); cp++)
-		/* nothing */ ;
-	while ((*cp == ' ') || (*cp == '\t'))
-		*cp++ = '\0';
-	if (*cp)
-		i_arg = cp;
-	strcpy (interp, i_name);
-	/*
-	 * OK, we've parsed out the interpreter name and
-	 * (optional) argument.
-	 * Splice in (1) the interpreter's name for argv[0]
-	 *           (2) (optional) argument to interpreter
-	 *           (3) filename of shell script (replace argv[0])
-	 *
-	 * This is done in reverse order, because of how the
-	 * user environment and arguments are stored.
-	 */
 	retval = remove_arg_zero(bprm);
 	if (retval)
 		return retval;
+
 	retval = copy_strings_kernel(1, &bprm->interp, bprm);
-	if (retval < 0) return retval; 
+	if (retval < 0)
+		return retval;
 	bprm->argc++;
-	if (i_arg) {
-		retval = copy_strings_kernel(1, &i_arg, bprm);
-		if (retval < 0) return retval; 
-		bprm->argc++;
+
+	bprm->buf[BINPRM_BUF_SIZE - 1] = '\0';
+
+	argc = parse_shell_args(bprm->buf+2, out, BINPRM_BUF_SIZE);
+
+	if (!argc)
+		return -ENOEXEC;
+
+	{
+		char *argv[argc];
+		char *out_p = out;
+		int i;
+
+		for(i=0; i < argc; i++) {
+			argv[i] = out_p;
+			out_p += strlen(out_p) + 1;
+		}
+
+		retval = copy_strings_kernel(argc, argv, bprm);
+		if (retval)
+			return retval;
+		bprm->argc += argc;
 	}
-	retval = copy_strings_kernel(1, &i_name, bprm);
-	if (retval) return retval; 
-	bprm->argc++;
-	bprm->interp = interp;
+
+	bprm->interp = out;
 
 	/*
 	 * OK, now restart the process with the interpreter's dentry.
 	 */
-	file = open_exec(interp);
+	file = open_exec(out);
 	if (IS_ERR(file))
 		return PTR_ERR(file);
 
@@ -95,7 +71,7 @@ static int load_script(struct linux_binprm *bprm,struct pt_regs *regs)
 	retval = prepare_binprm(bprm);
 	if (retval < 0)
 		return retval;
-	return search_binary_handler(bprm,regs);
+	return search_binary_handler(bprm, regs);
 }
 
 static struct linux_binfmt script_format = {
-- 
1.6.1.3.GIT

[PATCH 3/3] [RESEND] binfmt_misc.c: Allow arguments in interpreter string

[Kirill A. Shutemov]

Use parse_shell_args() to parse interpreter string. It allows to pass
arguments to interpreter.

Now we can drop dummy wrappers around interpreters.

Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
---
 fs/binfmt_misc.c |   46 +++++++++++++++++++++++++++++-----------------
 1 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index c4e8353..fadb29d 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -14,6 +14,7 @@
  *  1997-06-30 minor cleanup
  *  1997-08-09 removed extension stripping, locking cleanup
  *  2001-02-28 AV: rewritten into something that resembles C. Original didn't.
+ *  2009-02-03 Allow arguments in interpreter string
  */
 
 #include <linux/module.h>
@@ -50,7 +51,7 @@ typedef struct {
 	int size;			/* size of magic/mask */
 	char *magic;			/* magic or filename extension */
 	char *mask;			/* mask, NULL for exact match */
-	char *interpreter;		/* filename of interpreter */
+	char *command;			/* command to interprete the file */
 	char *name;
 	struct dentry *dentry;
 } Node;
@@ -107,8 +108,8 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
 {
 	Node *fmt;
 	struct file * interp_file = NULL;
-	char iname[BINPRM_BUF_SIZE];
-	char *iname_addr = iname;
+	char out[BINPRM_BUF_SIZE];
+	int argc = 0;
 	int retval;
 	int fd_binary = -1;
 
@@ -120,13 +121,14 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
 	if (bprm->recursion_depth > BINPRM_MAX_RECURSION)
 		goto _ret;
 
-	/* to keep locking time low, we copy the interpreter string */
+	/* to keep locking time low, we copy the command string */
 	read_lock(&entries_lock);
 	fmt = check_file(bprm);
 	if (fmt)
-		strlcpy(iname, fmt->interpreter, BINPRM_BUF_SIZE);
+		argc = parse_shell_args(fmt->command, out, BINPRM_BUF_SIZE);
 	read_unlock(&entries_lock);
-	if (!fmt)
+
+	if (!argc)
 		goto _ret;
 
 	if (!(fmt->flags & MISC_FMT_PRESERVE_ARGV0)) {
@@ -170,15 +172,25 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
 		goto _error;
 	bprm->argc++;
 
-	/* add the interp as argv[0] */
-	retval = copy_strings_kernel (1, &iname_addr, bprm);
-	if (retval < 0)
-		goto _error;
-	bprm->argc ++;
+	{
+		char *argv[argc];
+		char *out_p = out;
+		int i;
+
+		for(i=0; i < argc; i++) {
+			argv[i] = out_p;
+			out_p += strlen(out_p) + 1;
+		}
+
+		retval = copy_strings_kernel(argc, argv, bprm);
+		if (retval)
+			return retval;
+		bprm->argc += argc;
+	}
 
-	bprm->interp = iname;	/* for binfmt_script */
+	bprm->interp = out;	/* for binfmt_script */
 
-	interp_file = open_exec (iname);
+	interp_file = open_exec (out);
 	retval = PTR_ERR (interp_file);
 	if (IS_ERR (interp_file))
 		goto _error;
@@ -287,7 +299,7 @@ static char * check_special_flags (char * sfs, Node * e)
 }
 /*
  * This registers a new binary format, it recognises the syntax
- * ':name:type:offset:magic:mask:interpreter:flags'
+ * ':name:type:offset:magic:mask:command:flags'
  * where the ':' is the IFS, that can be chosen with the first char
  */
 static Node *create_entry(const char __user *buffer, size_t count)
@@ -379,12 +391,12 @@ static Node *create_entry(const char __user *buffer, size_t count)
 			goto Einval;
 		*p++ = '\0';
 	}
-	e->interpreter = p;
+	e->command = p;
 	p = strchr(p, del);
 	if (!p)
 		goto Einval;
 	*p++ = '\0';
-	if (!e->interpreter[0])
+	if (!e->command[0])
 		goto Einval;
 
 
@@ -448,7 +460,7 @@ static void entry_status(Node *e, char *page)
 		return;
 	}
 
-	sprintf(page, "%s\ninterpreter %s\n", status, e->interpreter);
+	sprintf(page, "%s\ninterpreter %s\n", status, e->command);
 	dp = page + strlen(page);
 
 	/* print the special flags */
-- 
1.6.1.3.GIT

Re: [PATCH 2/3] [RESEND] binfmt_script: rewrite using parse_shell_args()

[Matthew Garrett]

On Wed, Feb 18, 2009 at 05:16:01PM +0200, Kirill A. Shutemov wrote:
> parse_shell_args() allows to use more than one argument in shebang.

As mentioned before, this breaks userspace ABI.
-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/3] [RESEND] binfmt_script: rewrite using parse_shell_args()

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 413 bytes --]

On Wed, Feb 18, 2009 at 03:16:16PM +0000, Matthew Garrett wrote:
> On Wed, Feb 18, 2009 at 05:16:01PM +0200, Kirill A. Shutemov wrote:
> > parse_shell_args() allows to use more than one argument in shebang.
> 
> As mentioned before, this breaks userspace ABI.

Do you have real world example of breakage?

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.org/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/3] [RESEND] binfmt_script: rewrite using parse_shell_args()

[Matthew Garrett]

On Wed, Feb 18, 2009 at 05:22:52PM +0200, Kirill A. Shutemov wrote:
> On Wed, Feb 18, 2009 at 03:16:16PM +0000, Matthew Garrett wrote:
> > On Wed, Feb 18, 2009 at 05:16:01PM +0200, Kirill A. Shutemov wrote:
> > > parse_shell_args() allows to use more than one argument in shebang.
> > 
> > As mentioned before, this breaks userspace ABI.
> 
> Do you have real world example of breakage?

Have you checked every script on every Linux system?
-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/3] [RESEND] binfmt_script: rewrite using parse_shell_args()

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 861 bytes --]

On Wed, Feb 18, 2009 at 03:29:56PM +0000, Matthew Garrett wrote:
> On Wed, Feb 18, 2009 at 05:22:52PM +0200, Kirill A. Shutemov wrote:
> > On Wed, Feb 18, 2009 at 03:16:16PM +0000, Matthew Garrett wrote:
> > > On Wed, Feb 18, 2009 at 05:16:01PM +0200, Kirill A. Shutemov wrote:
> > > > parse_shell_args() allows to use more than one argument in shebang.
> > > 
> > > As mentioned before, this breaks userspace ABI.
> > 
> > Do you have real world example of breakage?
> 
> Have you checked every script on every Linux system?

Ok, I've looked into other kernels(*BSD and OpenSolaris). None of them
parse arguments in shebang. Before 6.x FreeBSD parsed arguments, but then
they changed behaviour.

So, this patch is wrong. What about patch #3?

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + ALT Linux Team, http://www.altlinux.org/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 2/3] [RESEND] binfmt_script: rewrite using parse_shell_args()

[Alan Cox]

> > > As mentioned before, this breaks userspace ABI.
> > 
> > Do you have real world example of breakage?
> 
> Have you checked every script on every Linux system?

And even if he did its an ABI and an important one. It is fixed and
defined. If you want a non standard behaviour please implement it via
binfmt_misc and a helper.