On 09/01/15 03:49, Theo de Raadt wrote:
Martijn van Duren wrote:
Hello tech@,

I took a quick glance at ksh and one of the first things I noticed was
that it uses some sanatizing code on argv. When looking at execve(2) I
see that EINVAL or EFAULT are returned when argv isn't properly
formatted. I've also verified this quickly by a small PoC and in
sys/kern/kern_exec.c.

Would it make sense to remove the check all together?

I think this is ok. You used to have to worry about it, because the kernel let
you exec something with empty argv. And there's still perhaps a portability
concern. But old workarounds need to die sometime. I support removing this,
but I'd like some one else to comment.

with fire

(or else .... i wonder if doas needs this checking...)


Looking forward to the smoke signals.

Here's another small patch that removes the INT32 define, since it was already statically defined. Although I was doubting whether or not it should be changed to int32_t since that way it conforms gives more guarantees towards the original comment and sys/types.h was already included in sh.h.
Index: sh.h
===================================================================
RCS file: /cvs/src/bin/ksh/sh.h,v
retrieving revision 1.33
diff -u -p -r1.33 sh.h
--- sh.h	18 Dec 2013 13:53:12 -0000	1.33
+++ sh.h	1 Sep 2015 17:23:13 -0000
@@ -28,12 +28,6 @@
 
 #include <paths.h>
 
-/* Find a integer type that is at least 32 bits (or die) - SIZEOF_* defined
- * by autoconf (assumes an 8 bit byte, but I'm not concerned).
- * NOTE: INT32 may end up being more than 32 bits.
- */
-# define INT32	int
-
 /* end of common headers */
 
 /* some useful #defines */
@@ -53,7 +47,7 @@
 #define	BIT(i)	(1<<(i))	/* define bit in flag */
 
 /* Table flag type - needs > 16 and < 32 bits */
-typedef INT32 Tflag;
+typedef int Tflag;
 
 #define	NUFILE	32		/* Number of user-accessible files */
 #define	FDBASE	10		/* First file usable by Shell */
@@ -353,7 +347,7 @@ EXTERN Getopt user_opt;		/* parsing stat
 
 /* This for co-processes */
 
-typedef INT32 Coproc_id; /* something that won't (realisticly) wrap */
+typedef int Coproc_id; /* something that won't (realisticly) wrap */
 struct coproc {
 	int	read;		/* pipe from co-process's stdout */
 	int	readw;		/* other side of read (saved temporarily) */
Index: jobs.c
===================================================================
RCS file: /cvs/src/bin/ksh/jobs.c,v
retrieving revision 1.41
diff -u -p -r1.41 jobs.c
--- jobs.c	18 Apr 2015 18:28:36 -0000	1.41
+++ jobs.c	1 Sep 2015 17:23:13 -0000
@@ -71,7 +71,7 @@ struct job {
 	int	status;		/* exit status of last process */
 	pid_t	pgrp;		/* process group of job */
 	pid_t	ppid;		/* pid of process that forked job */
-	INT32	age;		/* number of jobs started */
+	int	age;		/* number of jobs started */
 	struct timeval systime;	/* system time used by job */
 	struct timeval usrtime;	/* user time used by job */
 	Proc	*proc_list;	/* process list */
@@ -111,7 +111,7 @@ static Job		*async_job;
 static pid_t		async_pid;
 
 static int		nzombie;	/* # of zombies owned by this process */
-INT32			njobs;		/* # of jobs started */
+int			njobs;		/* # of jobs started */
 static int		child_max;	/* CHILD_MAX */
 
 

Reply via email to