2000-11-16 17:19:42

by Richard Jerrell

Subject: Bug in 2.4.0-test9 and test10 with sys_shmat()

Sending -1 as the shmid to shmat will cause an oops. 2.2.16 caught this
with simple boundry checking, so replace the lines

if (!shm_sb || (shmid % SEQ_MULTIPLIER) == zero_id)
return -EINVAL;

with

if (!shm_sb || shmid < 0 || (shmid % SEQ_MULTIPLIER) == zero_id)
return -EINVAL;

Simple program to demonstrate the bug...

#include <sys/ipc.h>
#include <sys/shm.h>

int main(void) {
shmat(-1,0,0);
return 0;
}

Rich
[email protected]

2000-11-17 08:46:51

by aprasad

Subject: Re: Bug in 2.4.0-test9 and test10 with sys_shmat()

>Sending -1 as the shmid to shmat will cause an oops. 2.2.16 caught this
>with simple boundry checking, so replace the lines

>if (!shm_sb || (shmid % SEQ_MULTIPLIER) == zero_id)
return -EINVAL;

>with

>if (!shm_sb || shmid < 0 || (shmid % SEQ_MULTIPLIER) == zero_id)
return -EINVAL;

-1 shmid is causing oops only when used with superuser privileges,
otherwise it returns -EINVAL.
regards
Anil

2000-11-17 15:10:11

by Christoph Rohland

Subject: [Patch] Re: Bug in 2.4.0-test9 and test10 with sys_shmat()

Hi Linus,

The attached patch fixes two things:

1) shmat should not oops on shmid < 0
2) I think the shm tables should be allocated with GFP_USER instead of
GFP_KERNEL since these are user requests.

Greetings
Christoph

--- 4-11-6/ipc/shm.c Wed Oct 4 15:58:02 2000
+++ linux/ipc/shm.c Fri Nov 17 13:47:29 2000
@@ -572,13 +572,13 @@
if (pages == 0)
return NULL;

- ret = kmalloc ((dir+1) * sizeof(pte_t *), GFP_KERNEL);
+ ret = kmalloc ((dir+1) * sizeof(pte_t *), GFP_USER);
if (!ret)
goto nomem;

for (ptr = ret; ptr < ret+dir ; ptr++)
{
- *ptr = (pte_t *)__get_free_page (GFP_KERNEL);
+ *ptr = (pte_t *)__get_free_page (GFP_USER);
if (!*ptr)
goto free;
init_ptes (*ptr, PTES_PER_PAGE);
@@ -586,7 +586,7 @@

/* The last one is probably not of PAGE_SIZE: we use kmalloc */
if (last) {
- *ptr = kmalloc (last*sizeof(pte_t), GFP_KERNEL);
+ *ptr = kmalloc (last*sizeof(pte_t), GFP_USER);
if (!*ptr)
goto free;
init_ptes (*ptr, last);
@@ -724,7 +724,7 @@
struct shmid_kernel *shp;
pte_t **dir;

- shp = (struct shmid_kernel *) kmalloc (sizeof (*shp) + namelen, GFP_KERNEL);
+ shp = (struct shmid_kernel *) kmalloc (sizeof (*shp) + namelen, GFP_USER);
if (!shp)
return ERR_PTR(-ENOMEM);

@@ -1202,7 +1202,7 @@
char name[SHM_FMT_LEN+1];
void *user_addr;

- if (!shm_sb || (shmid % SEQ_MULTIPLIER) == zero_id)
+ if (!shm_sb || shmid < 0 || (shmid % SEQ_MULTIPLIER) == zero_id)
return -EINVAL;

if ((addr = (ulong)shmaddr)) {