[Dazuko-devel] Bug in mask_proc()

[Lino Sanfilippo]

Hi,

there is a bug in the mask_proc() function in event.c:
A dazukofs_proc structure is allocated on the stack and put into a 
global list.
This might lead to corrupted data since the stack location of the struct 
might
be overwritten in future.
This patch uses heap memory instead of stack memory. Since the proc struct
is allocated and freed frequently,  a memory cache is used.

This patch also adds some missing includes in file.c and ign_dev.c.

@John
I also saw that the event allocation and releasing is unnecessary 
complicate
(see release_event()). Should we not use a real ref counting mechanism that
ensures memory freeing when the last reference to the event is released?


Regards,
Lino

Geschäftsführender Gesellschafter: Tjark Auerbach
Sitz der Gesellschaft: Tettnang
Handelsregister: Amtsgericht Ulm, HRB 630992
ALLGEMEINE GESCHÄFTSBEDINGUNGEN
Es gelten unsere Allgemeinen Geschäftsbedingungen
(AGB). Sie finden sie in der jeweils gültigen Fassung
im Internet unter http://www.avira.de/agb
***************************************************

diff -rup dazukofs-3.1.0-rc2/event.c dazukofs-3.1.0-rc2-Patched/event.c
--- dazukofs-3.1.0-rc2/event.c  2009-07-02 21:15:05.000000000 +0200
+++ dazukofs-3.1.0-rc2-Patched/event.c  2009-07-09 17:57:01.000000000 +0200
@@ -87,6 +87,7 @@ static struct dazukofs_proc proc_list;
 static struct kmem_cache *dazukofs_group_cachep;
 static struct kmem_cache *dazukofs_event_container_cachep;
 static struct kmem_cache *dazukofs_event_cachep;
+static struct kmem_cache *dazukofs_proc_cachep;
 
 static int last_event_id;
 
@@ -128,6 +129,13 @@ int dazukofs_init_events(void)
        if (!dazukofs_event_cachep)
                goto error_out;
 
+       dazukofs_proc_cachep = 
+               kmem_cache_create("dazukofs_proc_cache",
+                                 sizeof(struct dazukofs_proc), 0,
+                                 SLAB_HWCACHE_ALIGN, NULL);
+       if (!dazukofs_proc_cachep)
+               goto error_out;
+
        return 0;
 
 error_out:
@@ -137,6 +145,9 @@ error_out:
                kmem_cache_destroy(dazukofs_event_container_cachep);
        if (dazukofs_event_cachep)
                kmem_cache_destroy(dazukofs_event_cachep);
+       if (dazukofs_proc_cachep)
+               kmem_cache_destroy(dazukofs_proc_cachep);
+
        return -ENOMEM;
 }
 
@@ -288,6 +299,7 @@ void dazukofs_destroy_events(void)
        kmem_cache_destroy(dazukofs_group_cachep);
        kmem_cache_destroy(dazukofs_event_container_cachep);
        kmem_cache_destroy(dazukofs_event_cachep);
+       kmem_cache_destroy(dazukofs_proc_cachep);
 }
 
 /**
@@ -576,6 +588,7 @@ static int check_recursion(void)
                        found = 1;
                        put_pid(proc->proc_id);
                        list_del(pos);
+                       kmem_cache_free(dazukofs_proc_cachep, proc);
                        break;
                }
        }
@@ -944,19 +957,25 @@ static struct dazukofs_event_container *
 
 /**
  * mask_proc - mask the current process
- * @proc: process structure to use for the list
  *
  * Description: Assign the current process to the provided proc structure
  * and add the structure to the list. The list is used to prevent
  * generating recursive file access events. The process is removed from
  * the list with the check_recursion() function.
  */
-static void mask_proc(struct dazukofs_proc *proc)
+static int mask_proc(void)
 {
+       struct dazukofs_proc *proc;
+
+       proc = kmem_cache_zalloc(dazukofs_proc_cachep, GFP_KERNEL);
+       if (!proc)
+               return -ENOMEM;
        proc->proc_id = get_pid(task_pid(current));
        mutex_lock(&proc_mutex);
        list_add(&proc->list, &proc_list.list);
        mutex_unlock(&proc_mutex);
+
+       return 0;
 }
 
 /**
@@ -972,7 +991,6 @@ static void mask_proc(struct dazukofs_pr
 static int open_file(struct dazukofs_event_container *ec)
 {
        struct dazukofs_event *evt = ec->event;
-       struct dazukofs_proc proc;
        int ret;
 
        /* open the file read-only */
@@ -984,7 +1002,9 @@ static int open_file(struct dazukofs_eve
        }
 
        /* add myself to be ignored on file open (to avoid recursion) */
-       mask_proc(&proc);
+       ret = mask_proc();
+       if (ret) 
+               goto error_out2;
 
        ec->file = dentry_open(dget(evt->dentry), mntget(evt->mnt),
                               O_RDONLY | O_LARGEFILE, current_cred());
diff -rup dazukofs-3.1.0-rc2/file.c dazukofs-3.1.0-rc2-Patched/file.c
--- dazukofs-3.1.0-rc2/file.c   2009-07-02 21:12:56.000000000 +0200
+++ dazukofs-3.1.0-rc2-Patched/file.c   2009-07-09 17:54:22.000000000 +0200
@@ -27,6 +27,7 @@
 #include <linux/file.h>
 #include <linux/fs_stack.h>
 #include <linux/cred.h>
+#include <linux/sched.h>
 
 #include "dazukofs_fs.h"
 #include "event.h"
diff -rup dazukofs-3.1.0-rc2/ign_dev.c dazukofs-3.1.0-rc2-Patched/ign_dev.c
--- dazukofs-3.1.0-rc2/ign_dev.c        2009-06-28 22:16:23.000000000 +0200
+++ dazukofs-3.1.0-rc2-Patched/ign_dev.c        2009-07-09 17:55:16.000000000 
+0200
@@ -23,6 +23,7 @@
 #include <linux/cdev.h>
 #include <linux/uaccess.h>
 #include <linux/pid.h>
+#include <linux/sched.h>
 
 #include "dazukofs_fs.h"
 #include "dev.h"