From 520a39efcd135556b21e196bfd64802fd3236ce1 Mon Sep 17 00:00:00 2001 From: Ori Bernstein Date: Mon, 25 Jul 2022 04:48:44 +0000 Subject: [PATCH 1/2] sysproc: raise limit on #! lines, and allow quoted args MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Our #! line length is very short, and the naïve quoting makes it difficult to pass more complicated arguments to the programs being run. This is fine for simple interpreters, but it's often useful to pass arguments to more complicated interpreters like auth/box or awk. This change raises the limit, but also switches to tokenizing via tokenize(2), rather than hand rolled whitespace splitting. The limits chosen are arbitrary, but they leave approximately 3 KiB of stack space on 386, and 13k on amd64. This is a lot of stack used, but it should leave enough for fairly deep devtab chan stacks. --- sys/man/2/exec | 5 ++++ sys/src/9/port/sysproc.c | 65 ++++++++++++++++------------------------ 2 files changed, 31 insertions(+), 39 deletions(-) diff --git a/sys/man/2/exec b/sys/man/2/exec index 9c9d4134b..dc7551d02 100644 --- a/sys/man/2/exec +++ b/sys/man/2/exec @@ -67,6 +67,11 @@ and any initial arguments to that program, for example ls | mc .EE .PP +There may be up to 256 bytes of arguments passed to the interpreter. +These are tokenized into up to 32 arguments by +.IR tokenize (2) +before being passed as the interpreters argument vector. +.PP When a C program is executed, it is called as follows: .IP diff --git a/sys/src/9/port/sysproc.c b/sys/src/9/port/sysproc.c index bb25b2b4b..cd1e641cb 100644 --- a/sys/src/9/port/sysproc.c +++ b/sys/src/9/port/sysproc.c @@ -243,35 +243,19 @@ sysrfork(va_list list) } static int -shargs(char *s, int n, char **ap) +shargs(char *s, int n, char **ap, int nap) { + char *p; int i; if(n <= 2 || s[0] != '#' || s[1] != '!') return -1; s += 2; n -= 2; /* skip #! */ - for(i=0;; i++){ - if(i >= n) - return 0; - if(s[i]=='\n') - break; - } - s[i] = 0; - - i = 0; - for(;;) { - while(*s==' ' || *s=='\t') - s++; - if(*s == 0) - break; - ap[i++] = s++; - while(*s && *s!=' ' && *s!='\t') - s++; - if(*s == 0) - break; - *s++ = 0; - } + if((p = memchr(s, '\n', n)) == nil) + return 0; + *p = 0; + i = tokenize(s, ap, nap-1); ap[i] = nil; return i; } @@ -300,12 +284,15 @@ beswav(uvlong v) uintptr sysexec(va_list list) { - struct { - Exec; - uvlong hdr[1]; - } ehdr; - char line[sizeof(ehdr)]; - char *progarg[sizeof(line)/2+1]; + union { + struct { + Exec; + uvlong hdr[1]; + } ehdr; + char buf[256]; + } u; + char line[256]; + char *progarg[32+1]; volatile char *args, *elem, *file0; char **argv, **argp, **argp0; char *a, *e, *charp, *file; @@ -353,22 +340,22 @@ sysexec(va_list list) if(!indir) kstrdup(&elem, up->genbuf); - n = devtab[tc->type]->read(tc, &ehdr, sizeof(ehdr), 0); + n = devtab[tc->type]->read(tc, u.buf, sizeof(u.buf), 0); if(n >= sizeof(Exec)) { - magic = beswal(ehdr.magic); + magic = beswal(u.ehdr.magic); if(magic == AOUT_MAGIC) { if(magic & HDR_MAGIC) { - if(n < sizeof(ehdr)) + if(n < sizeof(u.ehdr)) error(Ebadexec); - entry = beswav(ehdr.hdr[0]); - text = UTZERO+sizeof(ehdr); + entry = beswav(u.ehdr.hdr[0]); + text = UTZERO+sizeof(u.ehdr); } else { - entry = beswal(ehdr.entry); + entry = beswal(u.ehdr.entry); text = UTZERO+sizeof(Exec); } if(entry < text) error(Ebadexec); - text += beswal(ehdr.text); + text += beswal(u.ehdr.text); if(text <= entry || text >= (USTKTOP-USTKSIZE)) error(Ebadexec); @@ -393,8 +380,8 @@ sysexec(va_list list) /* * Process #! /bin/sh args ... */ - memmove(line, &ehdr, n); - n = shargs(line, n, progarg); + memmove(line, u.buf, n); + n = shargs(line, n, progarg, nelem(progarg)); if(n < 1) error(Ebadexec); /* @@ -411,8 +398,8 @@ sysexec(va_list list) t = (text+align) & ~align; text -= UTZERO; - data = beswal(ehdr.data); - bss = beswal(ehdr.bss); + data = beswal(u.ehdr.data); + bss = beswal(u.ehdr.bss); align = BY2PG-1; d = (t + data + align) & ~align; bssend = t + data + bss; From bc64cc50ac468ea8d1017fc8d3a761b7926d9fe0 Mon Sep 17 00:00:00 2001 From: Ori Bernstein Date: Tue, 26 Jul 2022 04:57:40 +0000 Subject: [PATCH 2/2] auth/box: preserve cwd name, but clear it out Auth/box previously switched to /, rather than preserving the cwd. This would break relative paths to items that would get pulled into the namespace. This change removes the '-.' flag, and causes auth/box to keep the current working directory, making it more usable for scripting. --- sys/man/8/auth | 7 ++--- sys/src/cmd/auth/box.c | 62 +++++++++++++++++++----------------------- 2 files changed, 31 insertions(+), 38 deletions(-) diff --git a/sys/man/8/auth b/sys/man/8/auth index 9c80885b9..f0c4cd2ac 100644 --- a/sys/man/8/auth +++ b/sys/man/8/auth @@ -62,7 +62,7 @@ changeuser, convkeys, printnetkey, status, enable, disable, authsrv, guard.srv, .PP .B auth/box [ -.B -d +.B -s ] [ .B -rc .I file @@ -298,9 +298,8 @@ the child namespace; the flag specifies a string of driver characters to keep. The .B -s -flag gives a base set of namespace -components, ones expected by rc, then passes -the first argument as a script file to rc. +flag initializes the namespace to what rc expects, +and passes its arguments unmodified to /bin/rc. .PP .I As executes diff --git a/sys/src/cmd/auth/box.c b/sys/src/cmd/auth/box.c index 60d842b90..0709f2e18 100644 --- a/sys/src/cmd/auth/box.c +++ b/sys/src/cmd/auth/box.c @@ -2,7 +2,11 @@ #include #include -static int debug; +static int debug; +static char cwd[8192]; +static char *parts[256]; +static int mflags[nelem(parts)]; +static int nparts; static void binderr(char *new, char *old, int flag) @@ -32,20 +36,14 @@ binderr(char *new, char *old, int flag) fprint(2, "bind %s %s %s\n", dash, new, old); } if(bind(new, old, flag) < 0) - sysfatal("bind: %r"); + sysfatal("bind %s: %r", new); } static void resolvenames(char **names, int nname) { int i; - char buf[8192]; - int fd; - fd = open(".", OREAD|OCEXEC); - if(fd < 0) - sysfatal("could not open .: %r"); - fd2path(fd, buf, sizeof buf); for(i = 0; i < nname; i++){ if(names[i] == nil) continue; @@ -55,10 +53,9 @@ resolvenames(char **names, int nname) case '/': break; default: - names[i] = cleanname(smprint("%s/%s", buf, names[i])); - } + names[i] = cleanname(smprint("%s/%s", cwd, names[i])); + } } - close(fd); } static void @@ -103,7 +100,8 @@ sandbox(char **names, int *flags, int nname) free(d); binderr(skel, dir, MBEFORE); } - binderr(names[j], targ, flags[j]); + if(flags[j] != -1) + binderr(names[j], targ, flags[j]); } binderr(newroot, "/", MREPL); } @@ -133,16 +131,11 @@ skelfs(void) sysfatal("/mnt/d mount setup: %r"); } -static char *parts[256]; -static int mflags[nelem(parts)]; -static int nparts; -static char *rc[] = { "/bin/rc", nil , nil}; - static void push(char *path, int flag) { if(nparts == nelem(parts)) - sysfatal("component overflow"); + sysfatal("too many bound paths"); parts[nparts] = path; mflags[nparts++] = flag; } @@ -150,23 +143,23 @@ push(char *path, int flag) void usage(void) { - fprint(2, "usage %s: [ -d ] [ -r file ] [ -c dir ] [ -e devs ] [ -. path ] cmd args...\n", argv0); + fprint(2, "usage %s: [ -r file ] [ -c dir ] [ -e devs ] cmd args...\n", argv0); exits("usage"); } void main(int argc, char **argv) { - char devs[1024]; - int dfd; - char *path; + char **argp, devs[128]; + int i, narg, dfd; char *a; int sflag; nparts = 0; - path = "/"; + narg = 0; memset(devs, 0, sizeof devs); sflag = 0; + argp = argv; ARGBEGIN{ case 'D': debug++; @@ -184,9 +177,6 @@ main(int argc, char **argv) case 'e': snprint(devs, sizeof devs, "%s%s", devs, EARGF(usage())); break; - case '.': - path = EARGF(usage()); - break; case 's': sflag = 1; break; @@ -195,18 +185,19 @@ main(int argc, char **argv) break; }ARGEND - if(argc == 0) + if(argc == 0 && !sflag) usage(); + if(getwd(cwd, sizeof(cwd)) == nil) + sysfatal("getwd: %r"); + push(cwd, -1); if(sflag){ snprint(devs, sizeof devs, "%s%s", devs, "|d"); push("/srv", MREPL|MCREATE); push("/env", MREPL|MCREATE); push("/rc", MREPL); push("/bin", MREPL); - push(argv[0], MREPL); - rc[1] = argv[0]; - argv = rc; + argp[narg++] = "/bin/rc"; } else { if(access(argv[0], AEXIST) == -1){ if((argv[0] = smprint("/bin/%s", argv[0])) == nil) @@ -216,6 +207,9 @@ main(int argc, char **argv) } push(argv[0], MREPL); } + for(i = 0; i < argc; i++) + argp[narg++] = argv[i]; + argp[narg] = nil; rfork(RFNAMEG|RFFDG); skelfs(); @@ -225,7 +219,7 @@ main(int argc, char **argv) resolvenames(parts, nparts); sandbox(parts, mflags, nparts); - + if(debug) fprint(2, "chdev %s\n", devs); @@ -238,8 +232,8 @@ main(int argc, char **argv) } close(dfd); - if(chdir(path) < 0) - sysfatal("can not cd to %s", path); - exec(argv[0], argv); + if(chdir(cwd) < 0) + sysfatal("chdir %s: %r", cwd); + exec(argp[0], argp); sysfatal("exec: %r"); }