Project

General

Profile

Actions

Bug #1387

open

zero-size malloc and ps: kvm_getprocs: Bad address

Added by qhwt+dfly almost 15 years ago. Updated almost 3 years ago.

Status:
Feedback
Priority:
Normal
Assignee:
-
Category:
Userland
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

Hello.
On recent -DEVELOPMENT ps command displays a non-intuitive error message
on a non-existent pid:
ps: kvm_getprocs: Bad address

Apparently the recent malloc reimplementation has changed malloc(3) family
so as malloc(0) now returns a pointer such that useracc() returns EFAULT
for it. kvm_getprocs() is one the functions affected by the new behavior
(no, I don't have the list of affected functions other than this
yet :). It used to return a non-NULL pointer and a 0 to *cnt, but now
it returns just NULL without affecting *cnt, so ps command displays the
error message. I think one way to fix is something like below (the fix
to the callers is taken from FreeBSD), but if we can restore the old
behavior of malloc(0), we don't need such fix.

%%%
diff --git a/bin/ps/ps.c b/bin/ps/ps.c
index 82e3e63..b66988a 100644
--- a/bin/ps/ps.c
+++ b/bin/ps/ps.c
@ -385,7 +385,9 @ main(int argc, char *argv)
/
* select procs
/
- if ((kp = kvm_getprocs(kd, what, flag, &nentries)) == NULL)
+ nentries = 1;
+ kp = kvm_getprocs(kd, what, flag, &nentries);
+ if ((kp == NULL && nentries > 0) || (kp != NULL && nentries < 0))
errx(1, "%s", kvm_geterr(kd));
if ((kinfo = malloc(nentries * sizeof(*kinfo))) NULL)
err(1, NULL);
diff --git a/lib/libkvm/kvm_proc.c b/lib/libkvm/kvm_proc.c
index 1c39636..d0e388d 100644
--
a/lib/libkvm/kvm_proc.c
++ b/lib/libkvm/kvm_proc.c
@ -479,6 +479,10 @ kvm_getprocs(kvm_t *kd, int op, int arg, int *cnt)
_kvm_syserr(kd, kd->program, "kvm_getprocs");
return (0);
}
if (size 0) {
+ *cnt = 0;
+ return (0);
+ }
do {
size = size / 10;
kd->procbase = (struct kinfo_proc *)
diff --git a/usr.bin/fstat/fstat.c b/usr.bin/fstat/fstat.c
index 133fff7..7a8545d 100644
--- a/usr.bin/fstat/fstat.c
++ b/usr.bin/fstat/fstat.c
@ -241,7 +241,9 @ main(int argc, char **argv)
if (kvm_nlist(kd, nl) != 0)
errx(1, "no namelist: s", kvm_geterr(kd));
#endif
- if ((p = kvm_getprocs(kd, what, arg, &cnt)) == NULL)
+ cnt = 1;
+ p = kvm_getprocs(kd, what, arg, &cnt);
+ if ((p == NULL &x%x
cnt > 0) || (p != NULL && cnt < 0))
errx(1, "%s", kvm_geterr(kd));
if (nflg)
printf("USER
.*s %.*s FD DEV %.*s MODE SZ|DV R/W",
%%

Actions #1

Updated by corecode almost 15 years ago

YONETANI Tomokazu wrote:

Hello.
On recent -DEVELOPMENT ps command displays a non-intuitive error message
on a non-existent pid:
ps: kvm_getprocs: Bad address

Apparently the recent malloc reimplementation has changed malloc(3) family
so as malloc(0) now returns a pointer such that useracc() returns EFAULT
for it.

I think we should make malloc(0) return a valid pointer. Whether to
return NULL or a pointer is implementation specific, and I believe our
previous malloc, and most other mallocs out there indeed return a pointer
and don't return NULL.

cheers
simon

Actions #2

Updated by joerg almost 15 years ago

On Sun, May 24, 2009 at 10:26:24AM +0200, Simon 'corecode' Schubert wrote:

I think we should make malloc(0) return a valid pointer.

For consistency, just round up malloc(0) to the minimal allocation size.
That avoids most problems with existing programs and keeps the spirit of
the standard.

Joerg

Actions #3

Updated by corecode almost 15 years ago

Matthew Dillon wrote:

We currently return an invalid non-NULL pointer, ZERO_LENGTH_PTR,
which I have set to ((void *)-8).

We can change ZERO_LENGTH_PTR to be whatever we want, including
making it a pointer to valid memory like the address of a dummy
global.

The standard however demands a unique pointer to be returned - we can't
return a constant pointer. I think rounding up to the minimal size sounds
good.

cheers
simon

Actions #4

Updated by dillon over 14 years ago

We currently return an invalid non-NULL pointer, ZERO_LENGTH_PTR,
which I have set to ((void *)-8).

We can change ZERO_LENGTH_PTR to be whatever we want, including
making it a pointer to valid memory like the address of a dummy
global.
It might be better to fix kvm_getprocs, though, to not try to do a
useracc test on a zero-length allocation. But I leave it up to you
guys.
-Matt
Matthew Dillon
&lt;&gt;

diff --git a/lib/libc/stdlib/nmalloc.c b/lib/libc/stdlib/nmalloc.c
index faa1e5c..29c1c4f 100644
--- a/lib/libc/stdlib/nmalloc.c
++ b/lib/libc/stdlib/nmalloc.c
@ -183,7 +183,7 @ typedef struct slglobaldata {
*/
#define WEIRD_ADDR 0xdeadc0de
#define MAX_COPY sizeof(weirdary)
-#define ZERO_LENGTH_PTR ((void *)-8)
#define ZERO_LENGTH_PTR ((void *)&nmalloc_dummy_storage)

#define BIGHSHIFT    10            /* bigalloc hash table */
#define BIGHSIZE (1 << BIGHSHIFT)
@ -219,6 +219,7 @ static struct slglobaldata SLGlobalData[SLGD_MAX];
static bigalloc_t bigalloc_array[BIGHSIZE];
static spinlock_t bigspin_array[BIGXSIZE];
static int malloc_panic;
+static int nmalloc_dummy_storage;
static const int32_t weirdary[16] = {
WEIRD_ADDR, WEIRD_ADDR, WEIRD_ADDR, WEIRD_ADDR,
Actions #5

Updated by dillon over 14 years ago

:The standard however demands a unique pointer to be returned - we can't=20
:return a constant pointer. I think rounding up to the minimal size sound=
:s=20
:good.
:
:cheers
: simon

Sure, go ahead and make the change.  I think that will actually reduce
the size of the code slightly since we would not longer have to deal
with the special pointer. Remember to deal with realloc properly as
well.
-Matt
Actions #6

Updated by tuxillo about 9 years ago

  • Description updated (diff)
  • Category set to Userland
  • Status changed from New to Feedback
  • Assignee deleted (0)
  • Target version set to 4.2

Hi guys,

So what do we do here? Change malloc(0) behavior to return a minimum-sized allocation ptr? Fix ps? Both?

Cheers,
Antonio Huete

Actions #7

Updated by tuxillo almost 3 years ago

  • Target version changed from 4.2 to 6.0
Actions

Also available in: Atom PDF