Subject: Re: copyinstr() with a zero-length buffer
To: None <tech-kern@netbsd.org>
From: Chuck Silvers <chuq@chuq.com>
List: tech-kern
Date: 02/06/2000 22:44:11
(oops, lost track of this for a while...)
On Tue, Nov 02, 1999 at 12:13:58PM -0500, Charles M. Hannum wrote:
>
> I observe the following additional problems:
...
> * powerpc doesn't check whether the saved length pointer is non-null.
> It also uses EACCES rather then EFAULT.
could someone please test this patch for the ppc:
Index: arch/powerpc/powerpc/copyinstr.c
===================================================================
RCS file: /cvsroot/syssrc/sys/arch/powerpc/powerpc/copyinstr.c,v
retrieving revision 1.2
diff -u -r1.2 copyinstr.c
--- copyinstr.c 1999/01/10 10:24:16 1.2
+++ copyinstr.c 1999/11/07 17:36:17
@@ -44,21 +44,25 @@
size_t len;
size_t *done;
{
- int c;
const u_char *up = udaddr;
u_char *kp = kaddr;
- int l;
-
+ size_t l;
+ int c, rv;
+
+ rv = ENAMETOOLONG;
for (l = 0; len-- > 0; l++) {
if ((c = fubyte(up++)) < 0) {
- *done = l;
- return EACCES;
+ rv = EFAULT;
+ break;
}
if (!(*kp++ = c)) {
- *done = l + 1;
- return 0;
+ l++;
+ rv = 0;
+ break;
}
}
- *done = l;
- return ENAMETOOLONG;
+ if (done != NULL) {
+ *done = l;
+ }
+ return rv;
}
Index: arch/powerpc/powerpc/copyoutstr.c
===================================================================
RCS file: /cvsroot/syssrc/sys/arch/powerpc/powerpc/copyoutstr.c,v
retrieving revision 1.2
diff -u -r1.2 copyoutstr.c
--- copyoutstr.c 1999/01/10 10:24:16 1.2
+++ copyoutstr.c 1999/11/07 17:36:17
@@ -46,18 +46,23 @@
{
const u_char *kp = kaddr;
u_char *up = udaddr;
- int l;
-
+ size_t l;
+ int rv;
+
+ rv = ENAMETOOLONG;
for (l = 0; len-- > 0; l++) {
if (subyte(up++, *kp) < 0) {
- *done = l;
- return EACCES;
+ rv = EFAULT;
+ break;
}
if (!*kp++) {
- *done = l + 1;
- return 0;
+ l++;
+ rv = 0;
+ break;
}
}
- *done = l;
- return ENAMETOOLONG;
+ if (done != NULL) {
+ *done = l;
+ }
+ return rv;
}
Index: arch/powerpc/powerpc/copystr.c
===================================================================
RCS file: /cvsroot/syssrc/sys/arch/powerpc/powerpc/copystr.c,v
retrieving revision 1.1
diff -u -r1.1 copystr.c
--- copystr.c 1996/09/30 16:34:43 1.1
+++ copystr.c 1999/11/07 17:36:17
@@ -46,13 +46,18 @@
u_char *kfp = kfaddr;
u_char *kdp = kdaddr;
size_t l;
+ int rv;
+ rv = ENAMETOOLONG;
for (l = 0; len-- > 0; l++) {
if (!(*kdp++ = *kfp++)) {
- *done = l + 1;
- return 0;
+ l++;
+ rv = 0;
+ break;
}
}
- *done = l;
- return ENAMETOOLONG;
+ if (done != NULL) {
+ *done = l;
+ }
+ return rv;
}
here's the test program I've been using:
#include <string.h>
#include <stdlib.h>
#include <sys/param.h>
#include <unistd.h>
#include <stdio.h>
#include <errno.h>
#define CHECK_LEN (NCARGS+2)
static char **array = NULL;
int
main(argc, argv, envp)
int argc;
char *argv[];
char *envp[];
{
int i;
array = (char **)malloc(CHECK_LEN * sizeof(char *));
for (i = 0; i < (CHECK_LEN - 1); i++)
array[i] = "/bin/sh";
array[CHECK_LEN - 1] = NULL;
errno = 0;
execve("/bin/sh", array, envp);
printf("errno %d\n", errno);
array[0] = "z";
errno = 0;
execve("/bin/sh", array, envp);
printf("errno %d\n", errno);
array[0] = (char *)-1L;
errno = 0;
execve("/bin/sh", array, envp);
printf("errno %d\n", errno);
array[0] = (char *)1L;
errno = 0;
execve("/bin/sh", array, envp);
printf("errno %d\n", errno);
array[0] = (char *)0x7f000000;
errno = 0;
execve("/bin/sh", array, envp);
printf("errno %d\n", errno);
array[0] = (char *)0x1f000000;
errno = 0;
execve("/bin/sh", array, envp);
printf("errno %d\n", errno);
printf("yay!\n");
exit(0);
}