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 */