9front - general discussion about 9front
 help / color / mirror / Atom feed
From: ori@eigenstate.org
To: 9front@9front.org
Subject: Re: [9front] sysproc: raise limit on shebang lines, handle quoting.
Date: Sat, 16 Jul 2022 13:45:29 -0400	[thread overview]
Message-ID: <5A0F4F2779A4FE204FBC0B799A5AEC44@eigenstate.org> (raw)
In-Reply-To: <E0A23896B1E68D2E16F7F196364F17C7@eigenstate.org>

Quoth ori@eigenstate.org:
> Quoth ori@eigenstate.org:
> > Quoth cinap_lenrek@felloff.net:
> > > > -	s += 2;
> > > > -	n -= 2;		/* skip #! */
> > > 
> > > i'd keep these lines, avoiding the multiple s+2 expressions
> > > down the road.
> > > 
> > > my main concern is making sure we'r not blowing the stack
> > > with this change on archs with small 4k stacks like 32bit arm.
> > > 
> > > just need to checks...
> > > 
> > > also:
> > > 
> > > > +			ehdr = (void*)buf;
> > > 
> > > not sure if that will yield properly aligned ehdr pointer as
> > > buf is just char[]. i think that was the reason it was a union
> > > before.
> > > 
> > > otherwise looks ok.
> > 
> > Thanks, and updated. Changes from last round:
> > 
> > - Use s+=2 as before
> > - Add stack size assert (can be removed before committing?)
> > - Bump buffer to 256, since it turned out we have more than
> >   enough space. On 386, we have 0xbb0 stack left by the time
> >   we call sysexec. 128 is workable, but I actually ran into
> >   it with a particularly insane auth/box call...
> > - Use a union instead of casting; it doesn't matter much,
> >   but I didn't like copying the exec header around. :)
> 

And, one more iteration, with an update to docs,
as well as a more general stackremain() function.

diff 8f1954c0dd4ca231d6904fbbd788dbe40ef284e3 uncommitted
--- a//sys/man/2/exec
+++ b//sys/man/2/exec
@@ -67,6 +67,11 @@
 ls | mc
 .EE
 .PP
+There may be up to 256 bytes of arguments passed to the interpreter.
+These are tokenized into up to 64 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
--- a/sys/src/9/port/sysproc.c
+++ b/sys/src/9/port/sysproc.c
@@ -243,8 +243,9 @@
 }
 
 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] != '!')
@@ -251,27 +252,10 @@
 		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;
 }
@@ -297,15 +281,33 @@
 				  | (uvlong)p[7];
 }
 
+/*
+ * Returns an estimate of the amount of stack space we have left.
+ * Note, this assumes that stacks grow down. We currently have
+ * no working architectures where the stack grows up.
+ */
+static intptr
+stackremain(void)
+{
+	int sval;
+	if(up != nil)
+		return (intptr)&sval - (intptr)up->kstack;
+	else
+		return (intptr)&sval - (intptr)MACHP(m->machno)->stack;
+}
+
 uintptr
 sysexec(va_list list)
 {
-	struct {
-		Exec;
-		uvlong	hdr[1];
+	union {
+		struct {
+			Exec;
+			uvlong	hdr[1];
+		};
+		char buf[256];
 	} ehdr;
-	char line[sizeof(ehdr)];
-	char *progarg[sizeof(line)/2+1];
+	char line[256];
+	char *progarg[64+1];
 	volatile char *args, *elem, *file0;
 	char **argv, **argp, **argp0;
 	char *a, *e, *charp, *file;
@@ -318,6 +320,8 @@
 	Chan *tc;
 	Fgrp *f;
 
+	/* We put big uffers on the stack, make sure we have room to keep going */
+	assert(stackspace() > 512);
 	args = elem = nil;
 	file0 = va_arg(list, char*);
 	validaddr((uintptr)file0, 1, 0);
@@ -353,7 +357,7 @@
 		if(!indir)
 			kstrdup(&elem, up->genbuf);
 
-		n = devtab[tc->type]->read(tc, &ehdr, sizeof(ehdr), 0);
+		n = devtab[tc->type]->read(tc, ehdr.buf, sizeof(ehdr.buf), 0);
 		if(n >= sizeof(Exec)) {
 			magic = beswal(ehdr.magic);
 			if(magic == AOUT_MAGIC) {
@@ -393,8 +397,8 @@
 		/*
 		 * Process #! /bin/sh args ...
 		 */
-		memmove(line, &ehdr, n);
-		n = shargs(line, n, progarg);
+		memmove(line, ehdr.buf, n);
+		n = shargs(line, n, progarg, nelem(progarg));
 		if(n < 1)
 			error(Ebadexec);
 		/*


      reply	other threads:[~2022-07-16 17:46 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-10 15:38 ori
2022-07-10 18:29 ` cinap_lenrek
2022-07-16 17:20   ` ori
2022-07-16 17:29     ` ori
2022-07-16 17:45       ` ori [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5A0F4F2779A4FE204FBC0B799A5AEC44@eigenstate.org \
    --to=ori@eigenstate.org \
    --cc=9front@9front.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).